This patch allow for buttons (with type=submit) to trigger an ajax enabled exposed form, and is similar to the change in #1123300: Image button in block views with exposed filter breaks AJAX routine.

Release notes snippet

Submit buttons themed as <button type="submit"> elements now trigger AJAX within Views exposed forms, just like they do when themed as <input type="submit"> elements. This now matches the behavior of Drupal 7 Views, fixing a regression from Drupal 8 and earlier versions of Drupal 9.

CommentFileSizeAuthor
#88 1551534-80.patch4.36 KBeffulgentsia
#88 1551534-80-test-only.patch2.9 KBeffulgentsia
#80 interdiff_78-80.txt717 bytesvsujeetkumar
#80 1551534-80.patch4.36 KBvsujeetkumar
#78 interdiff_75-78.txt1.01 KBvsujeetkumar
#78 1551534-78.patch4.33 KBvsujeetkumar
#75 1551534-75.patch4.86 KBLendude
#75 interdiff-1551534-73-75.txt3.15 KBLendude
#73 1551534-73.89.patch0 bytesneclimdul
#73 1551534-73.interdiff.txt2.24 KBneclimdul
#73 1551534-73.patch4.75 KBneclimdul
#70 1551534-70.patch3.76 KBcasey
#52 1551534-52.patch4.67 KBLendude
#52 interdiff-1551534-49-52.txt752 bytesLendude
#49 1551534-49.patch4.69 KBLendude
#49 1551534-49-TEST_ONLY.patch3.32 KBLendude
#44 views-ajax-filter-causes-page-refresh-1551534-44.patch1.37 KBhenkholtrop
#41 1551534-8.6.patch1.56 KBmishac
#36 1551534-8.5-36.patch1.47 KBseanB
#32 1551534-8.4-32.patch891 bytesb_sharpe
#32 1551534-32.patch801 bytesb_sharpe
#24 1551534-23.patch795 bytesdevert
#21 1551534-21.patch672 bytesheykarthikwithu
#16 allow_a_button_in_an-1551534-16.patch606 bytesalansaviolobo
#15 drupal8.views-module.1551534-15.patch638 bytesrichmac
allow-buttons-to-trigger-ajax.patch562 bytesbcn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bcn’s picture

Status: Active » Needs review

Status change since there is a patch to review.

dawehner’s picture

What is the difference between button in input, or why do you need this feature?

bcn’s picture

The general difference is a button can have html inside.
<button><h1>Click</h1></button>
instead of:
<input type="button" />
--http://stackoverflow.com/questions/469059/button-vs-input-type-button-which-to-use

I needed this because I was using a theme which used a element in the exposed filter of a view.

natted’s picture

I've tested this patch and use it also for the same reason as poster #3

JuliaKoelsch’s picture

I also tested it, because I ran into the issue described. Fixes issue for me as well.

zmove’s picture

+1 for this, especially with the HTML5 that is coming a support is very important.

cpliakas’s picture

Status: Needs review » Reviewed & tested by the community

+1, and I also confirm that the patch fixes the issue I ran into as well. This seems to have multiple testers with positive experiences and no discernable drawbacks, so marking as RTBC.

Jibus’s picture

+1, confirm that the patch works for me. Thanks !

dnewkerk’s picture

+1 also. Tested and confirmed this works perfectly. I ran into this issue when using the Twitter Bootstrap theme (which is HTML5-based), though it likely affects most/all of the various HTML5-based themes.

muka’s picture

+1 same issue on bootstrap theme, works for me

Lucasljj’s picture

dawehner’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Code » views.module
Status: Reviewed & tested by the community » Patch (to be ported)

Great. Thanks for providing and testing the patch.

Committed to 7.x-3.x and marked as to be ported to 8.x

batabata’s picture

WELL,HELPsFUL

richmac’s picture

Assigned: Unassigned » richmac
richmac’s picture

Assigned: richmac » Unassigned
Status: Patch (to be ported) » Needs review
FileSize
638 bytes

The attached patch has been tested against a view created in 8.x-dev.

alansaviolobo’s picture

Issue summary: View changes
FileSize
606 bytes

reroll

DamienMcKenna’s picture

Does this patch need tests? It makes complete sense to me and still applies against HEAD. RTBC?

Lucasljj’s picture

Issue tags: +markup
mikefyfer’s picture

.

mikefyfer’s picture

Just to update this real quick, as it's still an issue. In a hurry, so no patch here, just the code.

as of Drupal 8.0.4

In core/modules/views/js/ajax_view.js - change

line 117
$('input[type=submit], input[type=image]', this.$exposed_form).each(function (index) {
to
$('input[type=submit], button[type=submit], input[type=image]', this.$exposed_form).each(function (index) {
heykarthikwithu’s picture

re-roll

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

devert’s picture

This patch no longer seems to be able to be applied to latest Drupal 8.1.8.

devert’s picture

Version: 8.1.x-dev » 8.1.8
FileSize
795 bytes

Re-roll so patch will work on Drupal 8.1.8

devert’s picture

Version: 8.1.8 » 8.1.x-dev

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nerdacus’s picture

Applied #24 to Drupal 8.2.1 and it works as promised.

dawehner’s picture

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

I think the steps forward would be to add some test coverage.

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mvwensen’s picture

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

b_sharpe’s picture

Re-roll

droplet’s picture

IMO, we should make it new .class rather than tag[attr] if it's a feature request. and mark existing way @deprecated.

nst37’s picture

Are there any plans to bring back reset button and make it work with AJAX?

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

seanB’s picture

Alexandre360’s picture

the patch is quite simple and probably have no drawback. Any plan to commit it soon ?

b_sharpe’s picture

Status: Needs work » Reviewed & tested by the community

Works great, can see no way for this to cause issue/conflict

HongPong’s picture

+1 looks good to me, this would help with certain things.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

In order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

  1. https://www.drupal.org/docs/8/testing
  2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.5.x
mishac’s picture

Patch didn't apply for me on 8.6-rc2. rerolling.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

grantkruger’s picture

#41 worked for me on 8.6 after #36 stopped working. Thanks to you both.

henkholtrop’s picture

Redid the patch. Now on the 8.6.x branch.

henkholtrop’s picture

Status: Needs work » Needs review
sanderwind’s picture

Status: Needs review » Reviewed & tested by the community

Tested #44 and works.

alexpott’s picture

Version: 8.6.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Needs work

See #40 we still need automated test coverage. Also as a new feature we need to target the next minor release branch.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests, -JavaScriptTest
FileSize
3.32 KB
4.69 KB

Here is a test for this, test_only file is also the interdiff.

The last submitted patch, 49: 1551534-49-TEST_ONLY.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

idebr’s picture

+++ b/core/modules/views/tests/themes/views_test_theme/views_test_theme.theme
@@ -0,0 +1,34 @@
+  $attributes = array('id', 'value', 'type');

This should be short array syntax. Test looks good!

Lendude’s picture

idebr’s picture

Status: Needs review » Reviewed & tested by the community

The feedback in #40 has been addressed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 52: 1551534-52.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Reviewed & tested by the community

Unrelated fail

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 52: 1551534-52.patch, failed testing. View results

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 52: 1551534-52.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Reviewed & tested by the community

Media lib fail

heykarthikwithu’s picture

+1 RTBC

lauriii’s picture

Is there a specific reason why we wouldn't allow <button type="button"> to trigger ajax?

lauriii’s picture

Status: Reviewed & tested by the community » Needs review

Moving to needs review for #62.

idebr’s picture

Status: Needs review » Reviewed & tested by the community

#62 Drupal has historically only used input elements, so many components do not support <button type="button"> simply because the markup was not available. There is no technical reason why a <button> should not be able to trigger ajax.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 52: 1551534-52.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Reviewed & tested by the community
lauriii’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/js/ajax_view.es6.js
@@ -139,7 +139,7 @@
+    $('input[type=submit], button[type=submit], input[type=image]', this.$exposed_form)

I would prefer allowing button[type=button] to trigger ajax too since it seems like there isn't any particular reason to not do that.

idebr’s picture

Status: Needs work » Needs review

#67 Not sure, a button[type="button"] should not trigger a form submit:

type
The default behavior of the button. Possible values are:

  • submit: The button submits the form data to the server. This is the default if the attribute is not specified for buttons associated with a
    , or if the attribute is an empty or invalid value.
  • reset: The button resets all the controls to their initial values, like . (This behavior tends to annoy users.)
  • button: The button has no default behavior, and does nothing when pressed by default. It can have client-side scripts listen to the element's events, which are triggered when the events occur.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

casey’s picture

Status: Needs review » Needs work

The last submitted patch, 70: 1551534-70.patch, failed testing. View results

ugintl’s picture

#52 did not apply to 8.9. Should I try #70?

neclimdul’s picture

Status: Needs work » Needs review
FileSize
4.75 KB
2.24 KB
0 bytes

I don't think 70 would apply to 8.9. I've made a quick convenience patch though for 8.9 even though I don't expect this will be back ported to 8 at this point.

70 failed because it dropped the .theme file so the test failed. Here's basically the same thing but it should pass.

This interdiff is against 70 to show what I fixed but this is the same patch as 52 rerolled onto 9.1.x. There would be no interdiff to 52 because there was just some whitespace conflicts from a es6 transpiler changes and some other patch offset conflicts.

Reroll is not a review, I haven't actually looked at this in a long time just passing through and helping out.

The last submitted patch, 73: 1551534-73.patch, failed testing. View results

Lendude’s picture

Lets see if this gets it back to green on 9.1.x

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

idebr’s picture

Status: Needs review » Needs work

The patch in #75 contains an unrelated file `./core/.phpunit.result.cache`

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
4.33 KB
1.01 KB

Re-roll patch created for 9.2.x, Also removed unreated file './core/.phpunit.result.cache' from the patch, Please have a look and advise.

idebr’s picture

Status: Needs review » Needs work

core/modules/views/js/ajax_view.es6.js does not pass eslint validation

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
4.36 KB
717 bytes

Fixed cs issues.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

b_sharpe’s picture

Status: Needs review » Reviewed & tested by the community

Works as intended and has test coverage, lets get this in!

BramDriesen’s picture

+1 also tested this and it indeed does what it needs to do :)

choudouhu’s picture

for version 9.1.9, 1551534-75.patch not working

choudouhu’s picture

For version 9.1.9, 1551534-75.patch not working

BramDriesen’s picture

@choudouhu Use the latest one (#80) instead of a year old patch :-)

effulgentsia’s picture

Title: Allow a button in an exposed forms to trigger ajax. » Submit buttons themed as <button> element fail to trigger ajax in Views exposed forms
Category: Feature request » Bug report
FileSize
2.9 KB
4.36 KB

Adding issue credits for reviewers and testers, and for @rfbrandsma from #2897276: Allow the attachment of AJAX exposed form behavior to a button of type submit .

Also, reclassifying this as a bug and retitling accordingly.

Also, reuploading #80 to get a new test run on Drupal 9.3, as well as a test-only patch to make sure that it fails as expected without the JS fix.

effulgentsia’s picture

Responding to reviews that didn't get incorporated into the patch:

@lauriii in #67:

I would prefer allowing button[type=button] to trigger ajax too since it seems like there isn't any particular reason to not do that.

I agree with #68's answer to this that the ajax that we're triggering is effectively a form submit so it should only apply to type="submit" buttons. Also, this way, we match the behavior that's in Views for Drupal 7 in https://git.drupalcode.org/project/views/-/blob/7.x-3.x/js/ajax_view.js#L90, per the commit that happened in #12. If we discover a need to also trigger this for other types of buttons, we can have a followup issue to add that.

@droplet in #33:

IMO, we should make it new .class rather than tag[attr] if it's a feature request. and mark existing way @deprecated.

If we want to do this, let's do it in a followup. The existing code in HEAD targets input[type=submit], and button[type=submit] has essentially the same semantics, so refactoring from tag[attr] is out of scope for this issue.

effulgentsia’s picture

effulgentsia credited Dom..

effulgentsia’s picture

Missed some in #96.

The last submitted patch, 88: 1551534-80-test-only.patch, failed testing. View results

  • effulgentsia committed 5478184 on 9.3.x
    Issue #1551534 by Lendude, vsujeetkumar, neclimdul, b_sharpe, devert,...
effulgentsia’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
Issue tags: +9.3.0 release notes

Pushed to 9.3.x. Tagging for a release notes mention, and added the release notes snippet to the issue summary.

Thank you to the many people who tested, reviewed, and rerolled this over the years.

effulgentsia’s picture

Issue summary: View changes
effulgentsia’s picture

By the way, there are several other JS files where input[type=submit] is in a selector and button[type=submit] isn't. And there might also be PHP code that treats them differently too. From #2998908-10: Views exposed form Ajax selector doesn't work on buttons:

I'm setting up a Search API page, and whether I have AJAX enabled or not, if I turn the submit button into a button of type submit, the full text search exposed filter has no effect.

Breaking AJAX seems to be part of a larger problem.

#1671190: Use <button /> form element type instead of <input type="submit" /> is probably the best place to discuss that for now. We may want to open new child issues of that one to fix specific bugs ahead of actually changing core to use <button> elements itself.

Status: Fixed » Closed (fixed)

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