Use <button /> form element type instead of <input type="submit" />

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Tagging for accessibility in case this has unforeseen consequences

#92 includes stable which was removed.but skimming comments #93-#97 seems there is still work to be done.

Was also previously tagged for screenshots which would need to happen.

Thanks.

Added patch against #92 in 10.1 version

Hiding patch #103 as it is missing files.
/core/modules/system/templates/button.html.twig
/core/themes/stable/templates/form/button.html.twig
/core/themes/stable9/templates/form/button.html.twig

Please include interdiffs in your re-rolls. I also don't think a re-roll was needed as it was confirmed to apply to 10.1 in #100

Per sthomen's comment on comment #97 → I have adapted the patch to only add the new #theme_wrapper if one doesn't already exist. This allows the patch to work with paragraphs enabled.

I guess it needs review again.

The last submitted patch, 105: core-use-button-instead-of-input-1671190-105.patch, failed testing. View results →

Just ran into this again when preparing the Drupal 10 Version (3.0x.) of the Homebox module. The issue clearly still exists and for theming it's really horrible that Drupal FAPI's '#type' => 'button' forces an <input type="submit" .../> and there's no real button FAPI element.

Are there any plans or initiatives planning to work on this area?

moving to MR to get all the gitlab CI goodness.

Here's what I did in Homebox → now to have buttons instead of input submit's for #ajax actions with the goal to use icons:
https://git.drupalcode.org/project/homebox/-/blob/3.0.x/src/Element/Home...

'portlet_add_ajax' => [
          '#type' => 'homebox_button',
          '#value' => $this->t('Add Portlet'),
          '#name' => 'addPortlet',
          '#attributes' => [
            'title' => $this->t('Add Portlet'),
          ],
          '#ajax' => [
            'callback' => '::updateFormAjaxCallback',
            'wrapper' => 'layout-wrapper',
            'event' => 'click',
          ],
]

If it helps anyone :) It works, now we have a real button type.

Idea: We could provide this additional FAPI element in contrib first, in a similar way!

Been looking at this at Contrib Day after Lille Drupalcon but haven't been able to make any progress as it looks like the MR hasn't been rebased, and when manually rebased to drupal-1671190/11.x ended up with this error on /core/install.php.

Fatal error: Cannot redeclare format_size() (previously declared in /var/www/html/repos/drupal/core/includes/common.inc:141) in /var/www/html/core/includes/common.inc on line 143

Looks like a manual rebase of this branch is required to bring everything up to date?

Taran2L → changed the visibility of the branch 1671190-add-button-tag to hidden.

Taran2L → changed the visibility of the branch 11.x to hidden.

hi all,

I've created an alternative PR which follows recommendations from #58 📌 Use form element type instead of Needs work by @dawenher

Fix the bugs we have in core which disallow the usages of buttons
Expose buttons, make it opt in
Discuss later, whether we want it to be buttons by default. Changing the HTML structure there is IMHO a big BC break when you actually would have to deal with CSS and more.

The main difference is that we are not introducing any new form element properties but rather allowing usage of button whenever needed (#uses_button_tag feels weird and unnecessary imo)

The question is how far should we go with that approach as clearly <button> element can do more, and atm it's not possible to achieve that, for example

• Button can be one of 3 types: button/reset/submit
• At the moment submit button does not execute submit handlers, which is odd at minimum
• Button can have separate value (which is handy for the form handling) and label (which is purely UI)
• Button can have elements inside of itself (currently children are being rendered outside)

I would love to address those things here without any changes to the Drupal forms (there is only a single usage of #type => button across all core codebase) so impact of such changes is minimal (as most of the form will still be using input type=submit)

Then in the follow-up we can start converting some form elements from input type=submit to a button leveraging the new features of the latter, like in #17 📌 Use form element type instead of Needs work :

I belive the benefit of going with button is that we can finally remove that type of code from form processing:

if ($request->request->get('op') != $this->t('Preview')) {
And replace is with a more sensible:

if ($request->request->get('op') != 'preview') {
Because the value and the text displayed for button can be different. Granted that is the only use of get op that I could find. D7 is littered with that kind of stuff so I guess it's much less important now.

Thanks for working on this, seems like a good improvement.

Adding <code> tags to the first line of the summary, where <button> wasn't visible, making this confusing to read.

Do the other themes need a template?

Seems like this would break contrib themes though that still have the input__submit templates.

Adding screenshots and testing online using Simplytest now.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Testing and taking screenshots now.

The button can contain complex HTML content such as images, allowing for much more versatility. It can also be styled much more extensively with CSS. To implement buttons, all form elements rendered using '' can be replaced with elements.

For example, this button displays both an image and text, something not possible with ''

Before:

After, a button with an image in it:

Seems the solution is different then the issue summary proposal, can that be updated to reflect.

Can a draft change record be written.

Once the summary is clearer can ask the #accessibility channel if anyone can take a look.

Do have concern if this will impact the other themes or contrib ones but guess that will be covered under the manual testing.

Question though of adding {{ children }} outside the button. Is that needed? Seems odd.

I had an issue in AJAX views not picking up button[type=button]:

I needed the following to fix it:

diff --git a/core/modules/views/js/ajax_view.js b/core/modules/views/js/ajax_view.js
index c76e7073f5..aafb4aac45 100644
--- a/core/modules/views/js/ajax_view.js
+++ b/core/modules/views/js/ajax_view.js
@@ -101,7 +101,7 @@
 
     // Add the ajax to exposed forms.
     this.$exposed_form = $(
-      `form#views-exposed-form-${settings.view_name.replace(
+      `#views-exposed-form-${settings.view_name.replace(
         /_/g,
         '-',
       )}-${settings.view_display_id.replace(/_/g, '-')}`,
@@ -143,7 +143,7 @@
     // Exclude the reset buttons so no AJAX behaviors are bound. Many things
     // break during the form reset phase if using AJAX.
     $(
-      'input[type=submit], button[type=submit], input[type=image]',
+      'input[type=submit], button[type=submit], button[type=button], input[type=image]',
       this.$exposed_form,
     )
       .not('[data-drupal-selector=edit-reset]')

@seanB would you suggest incorporating this into the general MR? Could you do it then?