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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip created an issue. See original summary.

damiankloip’s picture

Status: Active » Needs review
dawehner’s picture

Issue tags: +JavaScriptTest
+++ b/core/modules/views/src/Tests/Plugin/ExposedFormTest.php
@@ -168,6 +168,9 @@ public function testResetButton() {
 
+    // Test the button is hidden after reset.
+    $this->assertNoField('edit-reset');

Nice!

damiankloip’s picture

damiankloip’s picture

The last submitted patch, 5: 2732111-5-tests-only-FAIL.patch, failed testing.

damiankloip’s picture

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

damiankloip’s picture

So here is the failing test and the fix to revert the AJAX behaviour on the Reset button.

damiankloip’s picture

Priority: Normal » Major
damiankloip’s picture

Title: Reset button never gets removed on AJAX enabled views » Reset button doesn't work and never gets removed on AJAX enabled views
damiankloip’s picture

Issue summary: View changes

The last submitted patch, 8: 2732111-8-tests-only-FAIL.patch, failed testing.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

LGTM

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

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

jibran’s picture

I thought JS test was clear enough but +1 to more discussion and screenshots.

alexpott’s picture

+++ b/core/modules/views/js/ajax_view.js
@@ -114,7 +114,7 @@
-    $('input[type=submit], input[type=image]', this.$exposed_form).each(function (index) {
+    $('input[type=submit], input[type=image]', this.$exposed_form).not('[data-drupal-selector=edit-reset]').each(function (index) {

a code comment about the .not might help too.

dawehner’s picture

+++ b/core/modules/views/tests/src/FunctionalJavascript/ExposedFilterAJAXTest.php
@@ -70,6 +70,16 @@ public function testExposedFiltering() {
+    $page = $session->getPage();
+    $html = $page->getHtml();
+    $this->assertContains('Page One', $html);
+    $this->assertContains('Page Two', $html);

Let's leverage $this->assertSession()->pageTextContains('Page One')

[10:42:32] alexpott: so basically resets conceptually requires you to fallback to the default values of a form, as part of the submit logic
[10:42:46] alexpott: without JS we are simply doing that by reloading the page
[10:43:33] with JS you would need to do that in the submit somehow

damiankloip’s picture

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

damiankloip’s picture

Status: Needs work » Needs review
FileSize
2.32 KB
1.66 KB

Comment added to the JS. @dawehner, do you mean change the test like this?

dawehner’s picture

+++ b/core/modules/views/tests/src/FunctionalJavascript/ExposedFilterAJAXTest.php
@@ -74,12 +74,10 @@ public function testExposedFiltering() {
+    $this->assertFalse($session->getPage()->hasButton('Reset'));

You can use $assert->buttonExists() here

damiankloip’s picture

I was going to use that but it throws an exception when it doesn't exist. We want it not to exist.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Ah nevermind then

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: 2732111-19.patch, failed testing.

The last submitted patch, 19: 2732111-19.patch, failed testing.

The last submitted patch, 19: 2732111-19.patch, failed testing.

The last submitted patch, 19: 2732111-19.patch, failed testing.

dawehner’s picture

Status: Needs work » Reviewed & tested by the community

All have been random failures.

Lendude’s picture

+++ b/core/modules/views/tests/src/FunctionalJavascript/ExposedFilterAJAXTest.php
@@ -70,6 +70,14 @@ public function testExposedFiltering() {
+    $this->waitForAjaxToFinish();

This 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

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 28: 2732111-28.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

reroll

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

Thanks lendude.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

  • catch committed cd248df on 8.2.x
    Issue #2732111 by damiankloip, Lendude, dawehner: Reset button doesn't...

  • catch committed f7fed13 on 8.1.x
    Issue #2732111 by damiankloip, Lendude, dawehner: Reset button doesn't...

Status: Fixed » Closed (fixed)

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

catch’s picture

Version: 8.2.x-dev » 8.1.x-dev

This was cherry picked to 8.1.x so moving back.

jagermonster’s picture

I 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

pyxio’s picture

why 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

jenniferhoude’s picture

Version: 8.1.x-dev » 8.8.x-dev

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

stefan.korn’s picture

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

VishalKumarSahu’s picture

Version: 8.8.x-dev » 8.9.x-dev

Ajax 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?