Problem/Motivation

Dropbutton / Operations groups a set of links rendered as a drop-down button.

However, in some cases, we need to add Button (Submit) elements to Dropbutton:

Proposed resolution

Provide a way to add Button (or Submit) elements to Dropbutton.

Remaining tasks

Patch.

User interface changes

Nothing.

API changes

@todo

Data model changes

@todo

Release notes snippet

@todo

CommentFileSizeAuthor
#2 3057577-add-buttons-to-dropdown-2.patch2.75 KBquiron

Comments

huzooka created an issue. See original summary.

quiron’s picture

quiron
Primary language Catalan
Location Barcelona
commented
Status: Active » Needs review
Related issues: +#2855458: Action dropbuttons are prematurely rendered and available only in #process
StatusFileSize
new 3057577-add-buttons-to-dropdown-2.patch2.75 KB

I guess something similar to what is done in actions when https://www.drupal.org/project/drupal/issues/2855458 is done will work here. If is ok maybe we should look for a better location for the shared code between both methods.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue tags: +Accessibility
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +Needs tests
  1. 👍 It's great that the patch in #2 passes tests: this means it does not break any existing #type ⇒ 'button' uses!
  2. However, without explicit test coverage, there's no guarantee that this new capability actually works as expected. Nor would we know if another future change would break it. For something trivial I'd not expect test coverage, but A) this is not trivial, B) the issue summary already indicates that this is an area that is complex, brittle and known to be buggy. So we better ensure that this actually does what we need it to do :)
  3. +++ b/core/lib/Drupal/Core/Render/Element/Button.php
@@ -97,4 +98,58 @@ public static function preRenderButton($element) {
+        // Add button to the corresponding dropbutton by cloning it.
+        // Note: do not prematurely render this as it will break any #attached
+        // bubbled metadata that is associated with the button.
+        $button = \Drupal::service('renderer')->renderPlain($element[$key]);

    🤔 I think this took inspiration from \Drupal\Core\Render\Element\Actions::preRenderActionsDropbutton(). Let's add an @see to ensure that we keep them in sync. 

  4. +++ b/core/lib/Drupal/Core/Render/Element/Button.php
@@ -97,4 +98,58 @@ public static function preRenderButton($element) {
+        // Hide original cloned element by indicating it was already printed.
+        $element[$key]['#printed'] = TRUE;

    🤔 Why do we even need to keep the original?

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.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

Re #2:

How can a Button element have child elements? (I think that it cant have at all).

The only place where core tries to hack <input type="submit"/>s in a Dropbutton is the ViewsEditForm (I referenced that in the issue summary), and I'm using that for testing our options and even this patch.
The hack starts there at line 403 and ends at line 511. If you check the code, you will see that the path key is a link and not an input submit. So the ideal Dropbutton must be able to render links, Buttons and Submit elements, even mixed.

And even contrib modules do that (at least Paragraphs does that, see #3079128: Broken Dropdown prevents Save in Claro).

Since Dropbutton uses the links theme function, these are hacks now, and this kind of hacks are not acceptable imho.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Related issues: +#1899236: Add new Splitbutton render element to eventually replace Dropbutton

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.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Closed (won't fix)

The Splitbutton element will be capable of this, and is nearing completion in #1899236: Add new Splitbutton render element to eventually replace Dropbutton. It will be very easy to change dropbuttons to splitbuttons.

As @huzooka already stated in #6, making this possible in a dropbutton would either require something quite hacky or a major API change that is far more complex than simply changing to Splitbutton once available.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Thank you so much for the update!