Closed (fixed)
Project:
Facets
Version:
8.x-1.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
12 May 2016 at 21:03 UTC
Updated:
28 May 2016 at 19:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
claudiu.cristeaPatch.
Comment #5
claudiu.cristeaFix.
Comment #8
claudiu.cristeaSorry, the patch from #5 was malformed. interdiff from #5 is OK.
Comment #9
claudiu.cristeaComment #10
strykaizerIs this only empty when soft_limit is enabled?
No need to add the js library if its disabled I suppose
Comment #11
claudiu.cristea@StryKaizer, yes this is what I'm doing here. I'm adding the library only when soft_limit is not 0 (aka "No limit").
Comment #12
strykaizerOkay, ignore my comment
I never use the empty command obviously ;)
Patch looks good.
I used data-attribute instead in the checkbox/dropdown widgets for passing on config to js, but we prolly should use drupal.settings as you did in this patch for more consistency. I'll create an issue for that.
Code is rtbc for me, but still needs manual testing.
Comment #13
claudiu.cristeaI changed the your checkbox data-facet-id > data-drupal-facet-item-id because it's about items. The I added a new data-drupal-facet-id to the list of facets (the UL list). Probably we should keep them.
Comment #14
claudiu.cristeaProbably I would move the
data-drupal-facet-item-idfrom checkbox to its parent (Links) to have it in all situations.Comment #15
strykaizerpatch from #8 was not working with me (both for lists and checkbox widget)
It looks like this is a js selector issue, as you already mentioned in #14
data-drupal-facet-id is on the parent div in my case, instead of the ul
Comment #16
claudiu.cristeaWorks for me. I tried Links, Checkbox and a custom widget that is extending Checkbox. But my site the heavily themed so, probably, this affects where the selector is placed?
Comment #17
claudiu.cristeaCan you investigate why this happens? I cannot reproduce that situation
Comment #18
strykaizerSure, doing right now.
Vanilla drupal with standard profile (so bartik) here.
Drupal 8.2.x
Comment #19
claudiu.cristeaOK, we are fixing also a bug here (see the interdiff.txt). The block build should be an array of items. Otherwise
\Drupal\block\BlockViewBuilder::preRender()is moving all#attributeson the block level.Comment #22
claudiu.cristeaYeah, forgot to treat also the empty scenario.
Comment #23
strykaizerGreat work, nice clean code!
Tested, works as expected.
Extremely minor nitpick, but this can also be fixxed while commiting I suppose.
RTBC for me
Extremely minor nitpick: Typo in todo ;) (not(h)ing)
Created an issue for this #2725453: Refactor widget plugins by adding interface, base class, schema
Comment #25
strykaizerComment #26
strykaizerDropdown got broken by this, fix coming up