Problem/Motivation
- Create an AJAX enabled view with exposed filters (preferably a couple, at least one being text/string filter), enable the reset button
- Load the view page - no reset button
- click filter submit button
- Reset button appears
- Click Reset button
- View is refreshed but reset button never disappears and the views query is not the same as the initial load
- Do the same with no AJAX for the view and it works as expected
Proposed resolution
Overwrite the views exposed input during exposed form submission, otherwise exposed input is rebuilt from the current (AJAX POST) request data with contains the exposed values again.
Remaining tasks
Tests?
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#30 | 2732111-30.patch | 2.33 KB | Lendude |
#28 | 2732111-28.patch | 2.34 KB | Lendude |
#28 | interdiff-2732111-19-28.txt | 623 bytes | Lendude |
#19 | interdiff-2732111-19.txt | 1.66 KB | damiankloip |
#19 | 2732111-19.patch | 2.32 KB | damiankloip |
Comments
Comment #2
damiankloip CreditAttribution: damiankloip commentedComment #3
dawehnerNice!
Comment #4
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #5
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedThis should do it.
Comment #7
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedThis goes a bit deeper than just the reset button, the actual query is not the same when reset is hit when ajax is enabled. More tests on the way, but short story is that #2024695: The "Reset" button ignores the "Use AJAX" setting (always behaves in a non-AJAX way). broke it.
Comment #8
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedSo here is the failing test and the fix to revert the AJAX behaviour on the Reset button.
Comment #9
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #10
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #11
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #13
jibranLGTM
Comment #14
alexpottI don't think a "lgtm" rtbc is valid here. We're changing the fix from #2024695: The "Reset" button ignores the "Use AJAX" setting (always behaves in a non-AJAX way). and there's no discussion of whether the bug that reported is fixed. Plus given this is a front end affecting change I'd expect a screenshot or two proving you've tested it and thought about the possible impacts of the change.
Comment #15
jibranI thought JS test was clear enough but +1 to more discussion and screenshots.
Comment #16
alexpotta code comment about the
.not
might help too.Comment #17
dawehnerLet's leverage
$this->assertSession()->pageTextContains('Page One')
Comment #18
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedA screenshot of a page reload? :) I mean there can be a screenshot of the reloaded page but it won't show you much that the actual view page that is originally loaded wouldn't show you.
Also, with regard the the front end change its also worth noting that the original issue that changed this behaviour was committed with almost no discussion, or any input from a views maintainer :) Also, that issue was not a fix as nothing was broken from 7.x. this was always the intended way for it to work. That issue actually broke things with resetting pretty badly.
This definitely fixes the submit logic and also renders #2732103: Boolean exposed filter default values do not work correctly with AJAX enabled views unnecessary. This now essentially covers that fix too by the coverage here.
Comment #19
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment added to the JS. @dawehner, do you mean change the test like this?
Comment #20
dawehnerYou can use
$assert->buttonExists()
hereComment #21
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedI was going to use that but it throws an exception when it doesn't exist. We want it not to exist.
Comment #22
dawehnerAh nevermind then
Comment #27
dawehnerAll have been random failures.
Comment #28
LendudeThis method is scheduled for removal in #2734091: Remove tests specific waitForAjaxToFinish methods, replace with assertSession()->assertWaitOnAjaxRequest., and
$this->assertSession()->assertWaitOnAjaxRequest();
is now available as a replacement, so lets just make this future proof by using that.Cosmetic change only so leaving at RTBC
Comment #30
Lendudereroll
Comment #31
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedThanks lendude.
Comment #32
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!
Comment #36
catchThis was cherry picked to 8.1.x so moving back.
Comment #37
jagermonster CreditAttribution: jagermonster commentedI think when this was merged it ended up overwriting https://www.drupal.org/files/issues/2024695-6.patch
The .not('[data-drupal-selector=edit-reset]') in the attachExposedFormAjax should be removed
The reset button still does not seem to function correctly
Comment #38
pyxio CreditAttribution: pyxio commentedwhy is this marked fixed??? i still have the same issue and it is already drupal 8.6.7 ... is there any fix for this currently? or any clever workaround? not being able to use ajax and/or reset is not very user-friendly search page. cheers kevin
Comment #39
jenniferhoude CreditAttribution: jenniferhoude commentedIm still having this same issue and im using drupal 8.8.2. View is using AJAX when clicking reset button acts like it works but just refreshes the view, with filters still in place and reset button doesnt disappear.
Comment #40
stefan.kornI am also still seeing this issue with reset in views using ajax.
You even can see this in the views preview. Reset button has no effect there, just like @jenniferaube describes.
Seems to me that the fix from here only prevent some "bad" behaviour for reset button in ajax, but does not make reset button work with ajax.
Comment #41
VishalKumarSahu CreditAttribution: VishalKumarSahu as a volunteer commentedAjax Reset button is still a issue. It should have been working as expected. Actually submit button is bound by the event listener but not the reset button. Any workaround?