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).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mtodor created an issue. See original summary.

mtodor’s picture

Issue summary: View changes
mtodor’s picture

Status: Active » Needs review
FileSize
2.13 KB

Here 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).

borisson_’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

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.

+++ b/js/base-widget.js
@@ -0,0 +1,29 @@
+  Drupal.behaviors.facetsViewsAjax = {
+    attach: function () {
+      $('.js-facets-widget').once('js-facet-filter').on('facets_filter', function (event, url) {
+        window.location = url;
+      });
+    }
+  };

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.

mtodor’s picture

@borisson_ thank you for feedback. And here we go! ;)

I have provided a few diff files so that it's easier to understand changes.

  1. 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.
  2. 3031581_5_link_widget_diff.txtis implementation of Facets JS API for Link widget.
  3. 3031581_5_checkbox_dropdown_widgets_diff.txtare changes for Checkbox and Dropdown widgets to use Facets JS API

There 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:

  1. AJAX: Link, Checkbox and Dropdown widgets
  2. No-AJAX: Link, Checkbox and Dropdown widgets
  3. No-JavaScript: Link, Checkbox and Dropdown widgets should be available as links
mtodor’s picture

Here 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.

giorgio79’s picture

Nice 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

chr.fritsch’s picture

@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 and Drupal\Tests\facets\FunctionalJavascript\WidgetJSTest.

Do you have also other things in mind that should be covered?

mtodor’s picture

Adjusted link widget implementation so that it doesn't provide classes for the child class.

chr.fritsch’s picture

Here are some tests for the ajax dropdown and checkbox widget.

borisson_’s picture

Issue tags: -Needs tests

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.

chr.fritsch’s picture

I 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.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

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.

  • borisson_ committed 02f0e68 on 8.x-1.x authored by chr.fritsch
    Issue #3031581 by mtodor, chr.fritsch, borisson_: Provide JavaScript API...
borisson_’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

askibinski’s picture

Just 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 using once().

borisson_’s picture

Nevermind, it is in the patch but not in the commit apparantly.

Huh, did I commit only a part of this patch?

mtodor’s picture

@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 use once().