Problem/Motivation
We have several selection widget modules like Chosen, Sumoselect, .. name it.
Everyone has its own favorite selection widget and it would be nice if they could use them for Facets.
That's why it would be nice to provide some API solution inside Facets so that every selection widget solution can make easy integration.
Proposed resolution
My proposal would be some kind of JavaScript API so that other widgets can use it.
Here are is proposal:
1. In order to discover a widget by Facets module, we should agree on CSS selector that should be used. For example: js-facets-widget
2. Facets module should provide handler on elements with that selector. We should agree on the handler name. For example: facets_filter. That handler should accept URL for the filter because URL is always used (in case of AJAX and NO-AJAX).
3. Every widget (also ones provided by Facets module) should use that trigger to provide filtering URL. In that case, we have one uniform way to provide information to Facets what should be used for filtering.
With these changes, selection widgets will just provide URL for filtering and they can react on any type of events provided by used libraries and their implementations. In some case it will be simple change
or click
event on base HTML elements, but sometimes it will be event provided by used JS library (like in case of select2, Chosen with multi-select option).
In all cases, it will trigger facets_filter
on widget and Facets will act accordingly in case of AJAX or NO-AJAX.
Related to
This issue is partially related to #2957442: Trigger JS behaviors for JS-generated Dropdown (for Chosen and others).
Comment | File | Size | Author |
---|---|---|---|
#10 | 3031581-10.patch | 18.31 KB | chr.fritsch |
Comments
Comment #2
mtodor CreditAttribution: mtodor at Thunder commentedComment #3
mtodor CreditAttribution: mtodor at Thunder commentedHere is a patch with minimal changes, just to demonstrate how it should look like.
Next step would be changing of select widgets provided by Facets (dropdown, checkbox and links) to use the same API (only call provided handler with correct values).
Comment #4
borisson_Like I mentioned earlier on slack to @chr.fritsch, we will need to cover this with tests from every possible angle, since this is a new api and we don't really have a lot of coverage related to the javascript side of our module yet. The last big JS-issue we committed got a lot of issues posted about it afterwards, that (ajax) issue was a lot bigger, but still it's something I would love to avoid having to deal with again.
This looks like it is a big part of the proposed change and it feels very readable and really coherent. Nice!
This should just be rewrapped under 80 cols :)
All in all, I'm very much in favor of this, we've had this requested a couple of times in the past - this looks like probably the best idea I've seen to resolve this issue.
Big +1 on this idea, and sorry for waiting so long before coming back with feedback - I've been (and still am) really busy.
Comment #5
mtodor CreditAttribution: mtodor at Thunder commented@borisson_ thank you for feedback. And here we go! ;)
I have provided a few diff files so that it's easier to understand changes.
3031581_5_base_JS_API_diff.txt
is base Facets JS API. There is a bit bigger comment there. That comment is Facets JS API documentation.3031581_5_link_widget_diff.txt
is implementation of Facets JS API for Link widget.3031581_5_checkbox_dropdown_widgets_diff.txt
are changes for Checkbox and Dropdown widgets to use Facets JS APIThere is already an example of integration for this Facets JS API for Select2 module and you can look at that here: #3031587: Support for AJAX Facets.
TODO - Tests:
Comment #6
mtodor CreditAttribution: mtodor at Thunder commentedHere is a feature that uses new JS API events and disabling widgets when filtering is triggered by any widget.
The issue can be found here: #3034776: Disable facet widgets until result is available.
Comment #7
giorgio79 CreditAttribution: giorgio79 commentedNice approach! Just recommended it in a bigger picture thread for handling Views WHERE modders (filters,arguments,facets etc) #1668024: Provide a unified user interface for static filters, exposed filters, contextual filters (arguments) and facets
Comment #8
chr.fritsch@borisson_: Regarding the tests, I think we should extend the coverage for the checkbox and dropdown widget in an ajax view. The other cases @mtodor listed are already covered by
Drupal\Tests\facets\Functional\WidgetIntegrationTest
andDrupal\Tests\facets\FunctionalJavascript\WidgetJSTest
.Do you have also other things in mind that should be covered?
Comment #9
mtodor CreditAttribution: mtodor at Thunder commentedAdjusted link widget implementation so that it doesn't provide classes for the child class.
Comment #10
chr.fritschHere are some tests for the ajax dropdown and checkbox widget.
Comment #11
borisson_No, this looks like it's covered really, really well. I would love to get manual validation from someone else that this works but otherwise this looks great.
Comment #12
chr.fritschI would really like to see this issue landed. How can we proceed?
We are using this patch in a Thunder with SearchApi/Facets branch for a while and it works fine for us.
Comment #13
borisson_That's enough validation for me. Setting this to rtbc, will go trough the queue over the weekend and commit this (and other open issues) so we can tag a new release soon-ish.
Comment #15
borisson_Committed.
Comment #17
askibinski CreditAttribution: askibinski as a volunteer and at ezCompany commentedJust wanted to note here that somewhere alongside this issue https://www.drupal.org/commitlog/commit/70809/8d7b1c954f6eccd323237f8d77... was commited with a refactoring of the JS libraries, altough I can't find it anywhere in an issue review?Nevermind, it is in the patch but not in the commit apparantly.
Each js widget now calls
Drupal.attachbehaviours()
which can potentially cause problems on sites implementing scripts without properly usingonce()
.Comment #18
borisson_Huh, did I commit only a part of this patch?
Comment #19
mtodor CreditAttribution: mtodor at Thunder commented@borisson_ as I see, everything is committed but real commit is different then credited one.
Credited: https://git.drupalcode.org/project/facets/commit/02f0e68
Code: https://git.drupalcode.org/project/facets/commit/8d7b1c9
@askibinski Unfortunately, behaviours are the way how to attach some JS functionality to elements in Drupal. And we need that in order to make some parts generic. In general
once()
should be always used in behaviour attach handlers, because you never know who and when will trigger additional attach behaviours. It could be from some additional AJAX requests, it could be from new JS functionality (like in this case). It's not nice, but only way to stay protected is to useonce()
.