Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Similar to #2658678: Checkbox widget does not enable multiple facet's values at a time we can also transform the dropdown widget into a js-driven widget which does a GET request instead of a POST for performance reasons. This means we remove the actual form, so the redirect doesn't have to happen anymore either. A win-win situation.
Comment | File | Size | Author |
---|---|---|---|
#25 | fixes.patch | 2.73 KB | borisson_ |
#21 | use_get_request_for-2713875-21.patch | 21.16 KB | borisson_ |
Comments
Comment #2
borisson_Comment #3
stijn26 CreditAttribution: stijn26 as a volunteer commentedComment #4
stijn26 CreditAttribution: stijn26 as a volunteer commentedTo-do: ensure that there is only one active item. Configurable placeholder text?
Comment #7
borisson_Thanks for that patch Stijn!
We already have a processor that ensures only 1 item can be active, we should probably make sure that processor is enabled by default for this widget. I attached a patch that removes a useless unit test and updates a failing one. The integration test'll still fail.
I think the easiest way forward here is implementing #2611198: Give widgets the ability to require settings and it's inverse.
Comment #10
borisson_This makes the integration test pass, we could commit this, but I wouldn't feel very confident right now, without any test coverage. So I'd love to get #2719065: Javascript test for widget in first, so we can test the JS that makes this into a dropdown.
So, let's keep this patch as-is for now, and we'll come back here soon.
Comment #11
borisson_I broke the js, I think. But I don't know why.
Comment #14
borisson_Comment #15
borisson_Reverted #11.
Comment #16
borisson_Ran eslint over the new js file.
Comment #17
StryKaizerShould be working now.
Since we can not access facet configuration from a widget plugin at this moment (this is the only plugin which requires this), I added a visible warning instead notifying the end user the "Make sure only one result can be shown." configuration is required to behave as a standard dropdown.
Comment #20
StryKaizerComment #21
borisson_Eslint + phpcs fixes, changed 1 doc comment in js. RTBC'd the patch because I think it looks great. We can add test coverage next in #2719065: Javascript test for widget
Comment #22
claudiu.cristeaNits.
Needs a newline at the end of file.
Why is this
private function
? Probablyprotected function
?Comment #23
borisson_Committed, see you in #2719065: Javascript test for widget to add test coverage.
Comment #25
borisson_Oh, sorry about #22, didn't see that before I committed, I've fixed those in the attached patch, you agree with those changes?
Comment #26
claudiu.cristeaComment #28
borisson_Committed that followup patch.