Add new Splitbutton render element to eventually replace Dropbutton

This pertains to VDC since dropbutton was brought into core for Views.

I'd like to see the link-based one stay, abusing it to wrap buttons is what I think is causing your frustration.

I support replacing the existing dropbutton. Using actual button elements should be possible, and in places where the action is JS-driven we probably shouldn't use links.

Besides the theming issues, the current dropbutton is not optimal from a code reuse point of view. Functionally, the dropbutton is just a button group plus a separate dropdown-menu component. Taking a more modular approach would allow button groups and dropdowns to be used separately in other situations, and also solves the problem of the current dropbutton having to change size when it's opened.

I'm in favor of implementing something very much like what Bootstrap has.

Up until the save/published issue, none of the elements inside dropbuttons were buttons. So sue us for the naming. But they are all links.

Just because another part of core used #prefix and #suffix to mock the markup of dropbuttons, doesn't mean the original ones should be hosed.

Also, this is is not major. If the other issue should be reverted, it should be reverted. But we'll have to be very very careful here to not introduce any regressions in the Views UI or any of our entity listings.

@tim.plunkett I think the issue here is that the dropbutton represents a design pattern that, when we tried to re-use it in a slightly different (and legitimate) way, turned out to be problematic. Why not take the opportunity to make something that is more flexible, more re-usable and more straightforward in its implementation?

That's a noble goal. But I'd think it would make more sense to fix the hacky usage, rather than completely rewrite it.

"Common implementations" are not accessible. Most of the work put into dropbuttons have been for accessibility purposes.

If the naming is all that is a problem here, then this should handle it.

@tim.plunkett I do have a tendency to shoot for the stars :)

I don’t really think the name is the issue. I am willing to bet that what we all want is the same thing: a usable, themable, accessible dropbutton. If the realistic path is to adapt the current implementation to support the new use case, then that’s what we should do. However, I can see lots of room for improvement in several areas, outlined in the OP and in #4. I’m not aware of any reasons why a solution similar to Bootstrap’s couldn’t be made as accessible as the current dropbutton, but I would be genuinely interested in the details.

I'm not saying bootstrap couldn't be accessible, just that it isn't. And in spending the time to make it so, we've duplicated work already done and still ended up with a custom solution. If you want to fix ours to work with more than just links, that's great. But let's retitle the issue to reflect it.

We need something approaching an actionable bug report here, rather than ranting. Avoiding insulting and meaningless terms such as "unholy" and "unacceptable" would do wonders to get anyone to care about this issue.

Ok, so is the actionable task here "We need to introduce a new splitbutton element to differentiate it from a dropbutton element, so that themers can write targeted CSS?" If so, let's get a spec on what that looks like in the issue summary.

You keep insulting people's efforts and insinuating that they're stupid for not giving you what you want, so be a leader. Educate them.

@sun pointed out this is related to #1914800: Dropbutton width is smaller than longest item

@sun

Is there a way how we can move forward on this issue? It seems to be that your oppinion is that we should not have both buttons
and links in there? The @todo in form_pre_render_actions_dropbutton() seems to assume that there should be just links, so we maybe should not use it for buttons?

I built a prototype as a proof-of-concept, at least for the front-end implementation and accessibility challenges. I’ve got a demo up that shows all the various components that would make up a split button: buttons, menus, and popups. The code is over at GitHub for those interested. I’ve made sure this is keyboard-navigable, and have tested using VoiceOver with good – but not perfect – results. I haven’t done cross-browser testing yet, and everything is pretty rough.

I’m looking to get very general feedback at this point, and spark further discussion.

@ry5n: I have no comments on the actual code, but I do have some feedback on the controls. Do we do that on this issue here or would it be OT?

@klonos Thanks! I guess it depends. This issue is about whether we should replace the core dropbutton with a split button. The demo (though imperfect) attempts to show that this is possible, and would work for us. If your feedback bears on that, then here is great. If it’s more general feedback on the style outlined in the proposed Seven style guide, then I’d say post it there.

Updated issue summary.

I’ve updated the summary, expanding on the rationale (it’s not just about implementation and themability). Summary now includes a list of what has been done and what remains.

I have working prototype code that demonstrates a solution including addressing #12, i.e. that works with 'a', 'button' and 'input' elements. I’m assigning this to me with the task of posting that code here for review. It won’t be a core patch, but will be html/css/js that is easier to review than where it is now.

Update summary to include additional rationale (UX and front-end architecture POV) as well as developments from the Seven style guide.

“Original Report” should be h3

Code attached. It feels a bit weird to attach static html/css/js to a d.o issue, but there it is. If there’s a better way to share this, please let me know.

Reviewers: please look at this prototype especially for:
- JS implementation
- Accessibility
- Feasibility for integration with core. I want to have a minimal affect on APIs.

Couple of questions, how much is taken from bootstrap? (if any, haven't checked)
It uses jQuery widgets, ask jesse or wim all the good they think of it from their work on edit. Also from the file: "This is dumb, reasons are inline." not looking good for core :p

But the real reason: our current dropbutton takes 1ms (on desktop) to initialise and 10ms is a safe estimate for mobile. Here I can see 5ms on average. Now think of the content listing page shown on a mobile. Currently 50*10ms =500ms to render all buttons. With this 50*50 = 2.5s to render only the buttons.

( edit ) actually that's 8ms, 5ms is the minimum. So 50*80 = 4s

@nod_ None of the code is from bootstrap. I don’t like the dependency on jQuery UI either, for a bunch of reasons.

I pretty much knew this JS would need a re-write; your feedback is exactly the kind I was looking for: specific issues to address. I was worrying more about my general structural approach, but yeah: performance :)

My plan is to re-write the JS for this:
- Remove the dependency on jQuery UI
- Use native DOM methods instead of jQuery where support is IE8+ (eg. querySelectorAll)
- Testing for init speed
- Merging the 'menu' and 'popup' into a single component. Right now they are completely separate JS plugins. The reason is flexibility: you can put anything inside a popup, and use a menu outside of one. However, core has no use cases that I know of for that, and I want to keep new theme functions to a minimum. Merging them should remove some JS complexity and overhead.

Gah, how did those tags get removed?!

Fix broken list markup

Status update: working on it :) I’ve managed to reduce the JS complexity quite a bit, and am planning to have new code for review in the next week.

Attached is a refactored version of #20 with:
- no dependency on jQuery UI
- positioning of popup menus in CSS rather than JS
- fewer internal function calls

Note:
- fully commented!
- still uses standard jQuery plugin patterns (would need to be adapted to use Behaviors etc.)
- the Menu and Popup plugins remain separate. When I got into it, there is so little overlap in functionality that I didn’t feel right about merging them.
- profiling in Chrome’s dev tools shows an average of ~4ms of combined execution, as far as I can tell. (I’d appreciate advice on how best to profile this.)

Some of the performance difference could be because there’s more going on for keyboard navigation and screen readers than in the current dropbutton. Focus is managed programmatically using aria attributes, requiring ids to be set on the individual menu items at init. There are also more event handlers being registered, mostly for keyboard navigation. Of course, it would be great if this was as fast as the current dropbutton.

I’d like to ask specifically if you think this is headed in the right direction for D8 core.

Gave it a quick overview, much better. I like the Menu/Popup separation.

Times I got with the same setup as before is similar to yours:
Popup: 1.8ms
Menu: 2.1ms

I get why we'd use arrow down/arrow up but it feels strange since I'm used to tabbing. The menu takes a long time to init. Seeing how it's not unusable without it I'd like to discuss if there is something we can do about it.

Popup time as 1.8ms is okayish, digging into it the $(documen).on() and this.trigger.on() takes about 500ms each and the Class adding and this.$element.on() take about 100ms each.

The current implementation doesn't do much. At most it'll add a class, 4 event listeners (on the same element) and add some HTML (that'd be the most expensive bit). The button action of toggling the popup is a delegated event on the body. If we can get the events on the document be delegated listeners and cut down the time of Menu by half it'll be good to go. It'd still be a bit slow but then we can explore if it's possible to delay initialisation of the menu until the user actually tabs on a splitbutton.

Since we're not using an existing plugin we don't have to make the items "standalone" and we can cheat to make things faster and delagate when useful.

Best thing I like here is that there is no extra markup added from JS. I'm not digging to much the class names, i'd rather use data- attribute to target things but that's details.

Nice work, thanks :)

So it's getting close but still NW. It'd be cool if someone could wrap that up to be used in the core UI.

@nod_ Great feedback; thanks! Glad this is on the right track.

- I really like the idea of initializing the Menu only when a Dropbutton gets focus. Since the Menu plugin is entirely devoted to keyboard interaction, that should work very well.
- I would be happy to use data-* attributes to attach behaviors. I assume that’s just for attaching, and we’d still use classes within a behavior for picking up additional DOM nodes. (Bonus: you could look at the markup alone and know where behaviors were being attached.)
- RE: class naming, the style used here follows the new CSS standards (affectionately: 'ugly classes').
- The 'is-{componentName}' and 'is-listening' classes are completely speculative. In D7, I believe the convention is '{componentName}-processed', and there is no pub/sub mechanism AFAIK.
- As far as keyboard interaction, I’ve tried to follow the ARIA best practices for menus, which recommends up/down key navigation.

I’ll make some adjustments to use data-* attributes and delegate more of the listeners.

.@tim.plunkett If this is a feature we should talk sooner than later about how the theme layer might output this markup.

- I’ve switched to using a 'data-behavior' attribute to initialize plugins ('js-*' classes are still used to find child elements).
- Popups now delegate most handlers to the body, and init time is down to ~1.5ms.
- Menus are now initialized by the the Popup script only on focus, so they have no overhead on load or when using only the mouse or touch.
- Some CSS and comment tweaks.

Very nice an beautiful.
Beauty need screen shot :)

Expanded

I think blue dropbutton expanded should be blue.

@jibran Thanks for the screens :)

I don’t think there’s a need to make the popup menu blue for primary buttons. There isn’t a usability win there and I don’t think it would look very good either.

If there are no objections, I’d like to move on to making this an actual patch. The core button styles will need to go in first #1986074: Buttons style update, but in the meantime I’d like to ask for some help/pointers integrating the JS with the behaviors system.

Here is a patch that is horribly trying to integrate that in Drupal.

It's super-messy, haven't deleted the dropbutton files, nor renamed everything and the style is a mess because we don't have the right classes on elements.

Js is pretty much ok now, it's time to get this in.

@nod_ Thanks! Anything to get started is good. I have earmarked some time this weekend for this too.

Remove redundant intro sentence

I’m a bit burned out, and dealing with some health issues. Need to ease back a bit for at least a few weeks. However, this likely needs to get in before code freeze. If someone wants to take it on, please do.

@nod is this something you can pick up? I'd like to get the consistent dropbutton styling into core and its being held up on this.

I got zero time these days. Not sure I'll make it on time.

This patch should apply now, which is a start...

Since this requires API changes and we are short on personnel and time, I gave this another go today. It’s now working for some admin pages, but currently breaks views UI.

Specific changes:
- moves theme stylesheets into Seven under /css, where they belong. If this gets closer these will need to be styled in Bartik too.
- introduces a theme_ui_menu to handle the menu and menuitems, since the markup does not use ul >li and so theme_links is more trouble than it’s worth.

This is probably all I’ve got left. Someone else needs to pick this up.

I had a quick look over this patch on my commute. It looks like we're having some issues with the views edit forms because views is trying to theme a regular button as a dropbutton. I'm not sure if we should make the dropbutton theme function flexible enough to output an regular button when there is no meny attached or if we should create a button theme function, and then views decides which one to call.

I spent a lot of time looking at the views UI and figuring out how to fix it. It's a lot better now but the code isn't great. It woud be good to get a few eyes (or at least a bot) on it.

We need to figure out a way to specify a small variant when calling the theme function.

Here's a little bit more work on the drop buttons. I'm struggling with the top display drop button. I seems to be passing in a different data structure. The views edit page is look pretty good to!

Just set that to needs review so the bot can have a go at it.

Back to needs work per #46 @LewisNyman

... the top display drop button. I seems to be passing in a different data structure.

needs reroll

I don't understand why this qualifies for any of these tags.
The issue summary has a lot of opinion and bias, and not much fact.

Unsubscribe

Always a pleasure to interact with you.

This issue kind of pulled the new style guide visuals into it. I'm concerned this patch is getting too weighty and more complex than it can be. It's a good idea to split the visual styling into #1989470: Dropbutton style update for Seven. Hopefully this will be easier to pick up after that issue is in. There are some UI improvements that rely on this patch.

This dropbutton has caused so much harm starting by #1608878: Add CTools dropbutton to core, but Views will guide us in the implementation so we'll think about not introduce any regressions in the Views UI or any of our entity listings at the very first step instead of re-factor it to embrace best practices and improve usability/maintainability/dependability.

Re-adding VDC as per #2 since dropbutton was brought into core for Views.

For the record, Jesse was not really OK (#1608878-167: Add CTools dropbutton to core) to rely on jQuery UI, its true, but sun didn't proposed to reuse jQuery, nor bootstrap, just being inspired by the separation of components, which is a good practice for themers and for code (de-)coupling.

Added twig related

Let's figure out what this issue is about now. I think it might be better as a meta.

Converting this issue into a meta.

See #50.

@lauriii just pointed me towards this issue, and I like the direction. I have a few points for accessibility, which I'm adding to the plan.

• We can make it much better for assistive tech users: reporting the open/closed state with machine-readable ARIA properties, by following the Menu Button pattern in the WAI-ARIA Authoring Practices. I already filed #2893663: Dropbutton should report open/closed state to assistive technology a while back - now I've added it to the problem and proposed resolution here, and made it into a child issue.
• The proposed markup in #2278473: Simplify Dropbutton markup in line with our CSS standards is much better, and will make it easier to implement this ARIA Menu Button pattern. Great!
• I noticed the proposed markup has a <ul role="menu">. That's good, the ARIA menu button pattern calls for it. However it's not sufficient to just put that role on the list container. We also need to manage the roles for the descendant LI and A elements, as well as some dynamic aria-* properties.
• In the ARIA Menu Button pattern, focus is constrained when the drop-down menu is open. Arrow keys are used to select from the menu, and the whole drop-down menu behaves as a single tab-stop, so it's much more like a native HTML select (and native desktop drop buttons). In our current implementation, arrow keys do nothing, and tabbing enough times makes you drop off the end of the menu and carry on through the rest of the controls on the page. I would like to follow the ARIA Authoring Practices on this too. Note that our drupal.tabbingManager is probably overkill for this; in particular, the "tabbing is now constrained..." message isn't needed if we handle the aria-expanded property correctly. I've added this focus constraint to the proposed resolution. Note that because our current menu collapse when focus moves out of it, it doesn't leave any extra tab-stops behind as litter. So this part is really about following the Menu Button pattern completely, rather than doing things by half.
• The more-actions button currently has a very weak focus style, just a faint background colour change like the hover style. It's hard to see and need to be stronger. @lauriii showed me https://cgit.drupalcode.org/sandbox-ry5n-1932040/plain/index.html - the re-use of the blue halo outline style there is much better. Added the need for a stronger focus style to the problem + proposed resolution.

It'll need a detailed accessibility review. The ARIA usage is very fussy about what roles and properties go were.

I discussed this with @lauriii.

It seems that we cannot solve these Dropbutton issues for BC reasons (especially, we need dramatic markup changes to make these happen), the way to move on is:

• Create a new Splitbutton element with the required markup, and functionality.
• Replace core's Dropbutton and Operations usage with the new Splitbutton.
• Deprecate Dropbutton.

IS updated.

I'm aware this is a meta, but the child issues aren't necessarily applicable and it wasn't possible to build off of any of them. It seemed best to reawaken this effort with a patch here, which can be split into other issues if needed. There's still work to be done but this is a working splitbutton

About the implementation:

• Any dropbutton can be changed to a splitbutton by changing #type to 'splitbutton'. These uses will trigger a deprecation error for 10.0.0
• #splitbutton_type accepts strings and arrays, so multiple types (such as primary and small) can be added.
• If the #title property is present, the main button is just a toggle with that title - as opposed to the default behavior of the main button being the first item plus a separate toggle
• The splitbutton accepts link, button, and submit render elements.
• The ARIA Menu Button pattern is only partially implemented. Some attributes are added, but keyboard navigation still needs work.

There is a testing module called Splitbutton Test that provides a page with most (all?) splitbutton variations at /splitbuttons
Although this module exists, there are no automated tests at the moment, so tagging with "needs tests"

Thanks for the patch, looking good :)

Agreed we either need a new issue or make this one an actual issue and not a "plan".

On the implementation a few things:

1. For toggle button need to add the type="button" attribute, otherwise it behaves like a submit button
2. performance is an issue: keep in mind that whatever you see on desktop it's x10 on mobile, and we have regularly 50 items of this type on page listings. I think the constructor does too much:
   • setting min-width is the most expensive action and could be deferred to when we actually open the dropdown
   • Ideally the popper init would be when needed and not on page load
   • can we put the aria attributes in the markup in the first place? what's the need for doing it in JS?
3. We have #splitbutton_items, why not #items like in lists? since it's more or less a fancy list. Dropbuttons were thought for actions links so the specificity comes from here. And there is no naming conflict that requires us to prefix with splitbutton.
4. the dropbutton code is old, I think these days we're going for classes for this type of pattern? maybe @justafish can confirm?
5. The looks like this, was there something planned in the design for this? i don't think it's great. But I see we don't have the no-js class on the html element anymore. might want to bring that back.

Glad to see this moving !

#75.1 added type="button"
#75.2/4 JS was fully refactored to use class and the more expensive operations only occur as needed.
#75.3

We have #splitbutton_items, why not #items like in lists?

This is to distinguish an operation list from a normal list. In this case, only link, button, and submit elements will get sent to the template. I was concerned #items would bring with it an expectation that a wider range of render elements could be added.
#75 Good catch on the disabled JS, I added a few html:not(.js) styles that seem to take care of it. Screenshots attached,

This patch also adds docs and keyboard navigation support based on the menu button pattern https://www.w3.org/TR/2016/WD-wai-aria-practices-1.1-20161214/examples/m...

Setting to NR since additional review is welcome, but aware that tests are still needed.

Fixed Test, Please review.

1. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +((Drupal, Popper) => {
   

   no jQuery, nice :)

   But use it still, don't duplicate the .once feature in the behavior please.

2. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +      this.splitbutton.addEventListener('mouseleave', e =>
   +        this.pointerCursorOut(e),
   +      );
   

   
   Would avoid having a extra function with : this.splitbutton.addEventListener('mouseleave', this.pointerCursorOut.bind(this)); keep it ignore that one.

   And would put this.splitbutton.addEventListener in a variable to shorten this whole thing:

   const listener = this.splitbutton.addEventListener;
   listener('mouseleave', e  => this.pointerCursorOut.bind(e));
   

3. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +        Array.prototype.slice
   +          .call(this.menu.querySelectorAll('input, a, button'))
   

   Too bad we have support for IE11, could have used Array.from instead.

4. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +            this.firstChars.push(
   

   if it's only one char: firstChar no?

5. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +      if (!disregard) {
   

   I would return early instead of nesting the switch in that if.

6. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,358 @@
   +          default:
   +            preventDefault = false;
   +            if (char.length === 1 && char.match(/\S/)) {
   +              this.setFocusByFirstCharacter(e, char.toLowerCase());
   +            }
   +            break;
   

   Nice, surprised by how helpful it is :)

7. The seven theme needs some love, the difference in the :focus of links/buttons doesn't look good.
8. There is an issue when navigating with the keyboard, when opening a dropdown and tabbing to another element, the dropdown doesn't close and the up/down arrow focus back there.
9. I think the delayed initialization broke focusNext, focusPrev.

for the jQuery.once thing, we could split the files, the class in it's own file, and the behavior in a different file, would be easier to override things if jQuery is an issue.

These reviews are great @nod_!

#80.1

But use it still, don't duplicate the .once feature in the behavior please.

To avoid the need for using once() I added anot() in the selector of elements to initialize: '.splitbutton:not(.js-splitbutton-enabled)'. Since .js-splitbutton-enabled is added during initialization, the selector would not find dropbuttons that had already been initialized. This selector, plus the fact that it is querying context instead of the whole document, seems like it would provide the same guardrails as once(), but it's possible there are edge cases or performance considerations I'm not aware of.

#80.2 I tried the

const listener = this.splitbutton.addEventListener;
listener('mouseleave', e  => this.pointerCursorOut.bind(e));

approach and it resulted in scoping issues. Events were broadcasting to every splitbutton on the page. I tried a few variations but ran into these problems unless I used my original formatting.

#80.4 I named it firstChars since it's an array - to me each item within the array would be a firstChar and the array firstChars . Keeping as-is for now, but if this naming still seems counterintuitive I'll change.

#80.5

I would return early instead of nesting the switch in that if.

Agreed. I thought eslint wasn't allowing this, but that was a mistake on my part 🙂.

#80.7

The seven theme needs some love

Wow, that needed some work!

#80.8+9

There is an issue when navigating with the keyboard, when opening a dropdown and tabbing to another element, the dropdown doesn't close and the up/down arrow focus back there. || I think the delayed initialization broke focusNext, focusPrev.

I ran into these symptoms after implementing the suggestion in #80.2, but not running into problems if I revert it. If this isn't the case on your end, I should be able to take care of it with a few additional details on how to reproduce.

This patch also adds tests.

Added missing test groups

About the .once it's actually how things were done back in drupal 6 so no worries about this working. It's about consistency If we do something we should always do it the same way, that way when the inevitable refactor comes, it'll be easier. We move the processed marker out of the classes to make sure we won't have performance impacts. If you want a no-jquery version have a look at #2402103: Add once.js to core. But we do have to use .once() and I would separate the files to have a file with a class and one with the behavior.

I see some problems with the JS crashing when there is only one item in the splitbutton, it can happen with permissions and whatnot.

About the .bind(): i see that, something weird in the compiling maybe. In any case, happy with what you have, as long as it works :) would still like a intermediate variable to make everything fit on one line.

firstChars: that makes sense too, ok with that.

keyboard works, my bad.

+++ b/core/lib/Drupal/Core/Render/Element/Splitbutton.php
@@ -0,0 +1,208 @@
+      if (count($items)) {
+        $element['trigger']['toggle'] = $toggle_element;
+        $element['trigger']['toggle']['#attributes']['class'][] = 'splitbutton__toggle';
+        $element['trigger']['toggle']['#value'] = '';
+        $element['trigger']['toggle']['splitbutton_arrow'] = [
+          '#type' => 'html_tag',
+          '#tag' => 'span',
+          '#attributes' => [
+            'class' => ['splitbutton__toggle-arrow'],
+          ],
+          'list_additional_actions' => [
+            '#type' => 'html_tag',
+            '#tag' => 'span',
+            '#attributes' => [
+              'class' => ['visually-hidden'],
+            ],
+            'message' => [
+              '#markup' => t('List additional actions'),
+            ],
+          ],
+        ];
+
+        foreach ($splitbutton_types as $toggle_splitbutton_type) {
+          $element['trigger']['toggle']['#attributes']['class'][] = 'button--' . $toggle_splitbutton_type;
+        }
+      }
+    }

That feels not very overritable? can we have a template for that? not sure, asking for advice here.

Adding credits from duplicated issue #2278473: Simplify Dropbutton markup in line with our CSS standards

credited myself from another issue :p

+++ b/core/core.libraries.yml
@@ -245,6 +245,17 @@ drupal.progress:
+drupal.splitbutton:
+  version: VERSION
+  js:
+    misc/splitbutton/splitbutton.js: {}
+  css:
+    component:
+      misc/splitbutton/splitbutton.css: {}
+  dependencies:
+    - core/drupal
+    - core/popperjs
+

I don't know how to test it but look at the code......
To remove the dropdown and replace it with Splitbutton, or add a TOTALLY NEW splitbutton is not smart.

Splitbutton = button + dropdown.

1. Code a Dropdown (there're dropdown usages in Core already. Re-use or re-made it)
2. Style the Dropdown to Splitbutton

@droplet it's to avoid breaking code for people.

Make a splitbutton, tag dropbutton deprecated => replace all core dropbutton with splitbutton => wait for D10 to remove dropbutton code.

not sure what the issue is. Once core dropbuttons are replaced we don't really need to fix dropbutton bugs if there are any we tell people to use splitbutton.
Or maybe i didn't understand the issue?

I meant we should make a shared component for the Core or common usage.

There're some dropdown usages. We can share the same codebase for that part.

For example, we have QuickEdit Button, which is a dropdown. A dropdown menu is too common for every site. Even on the Contrib Toolbar modules

And Splitbutton is totally the Button + Dropdown. There's no reason not to split it into 2 parts and take the advantages of code sharing

I don't know how to test it but look at the code......

To test, enable the splitbutton_test module provided in the patch, then go to /splitbuttons.

There're some dropdown usages. We can share the same codebase for that part.

For example, we have QuickEdit Button, which is a dropdown. A dropdown menu is too common for every site. Even on the Contrib Toolbar modules

And Splitbutton is totally the Button + Dropdown. There's no reason not to split it into 2 parts and take the advantages of code sharing

This was anticipated and it's already part of the solution in the patch. If a splitbutton has a #title attribute, there is no separate toggle button behaves like a dropdown. In the /splitbutton page provided by the splitbutton_test module, you can see examples of this in the "Splitbuttons where the primary button is just a toggle" section. A little bit of styling (which would be needed for any approach) would be all that is needed to provide a full dropdown experience.

Providing selectors to style it as a dropdown could be as simple as adding a "dropdown" #splitbutton_type, or by creating a '\Dropdown' render element that extends \SplitButton and makes #title mandatory

And code reuse was a priority in this approach overall. Unlike dropbutton, Splitbutton uses existing button styles, aside from changing border-radius in a few spots. This includes variants - unlike dropbutton a splitbutton can use primary/danger/small etc.

I believe this addresses all the concerns I saw. If not, perhaps spend some time on the /splitbutton page provided by splitbutton_test and come back with your concerns.

Hmm. nice.

Personally, I don't really like how it mixed. I preferred the bootstrap naming and structures
https://getbootstrap.com/docs/4.5/components/dropdowns/#split-button

If I think in Splitbutton side...first..

splitbutton__main--with-toggle should be splitbutton__main

splitbutton__main
splitbutton__main--no-toggle // not a good name. It has toggle acutally.

----

splitbutton_with_title
splitbutton_link_first

A bit odd.

----

splitbutton__toggle
splitbutton__title--toggle

I preferred (, actual it should be ....)

splitbutton__toggle
splitbutton__toggle--title-toggle //or
splitbutton__toggle--with-title

----

Add a new .splitbutton__button. And add to below 2 buttons

.splitbutton__main-button
.splitbutton__toggle

Then, move the shared style on .splitbutton__main-button to .splitbutton__button

----

splitbutton__operation-list
// =>
splitbutton__dropdown-menu // or
splitbutton__menu

----

splitbutton__toggle-arrow

with-toggle styling the arrow with a different way.

----

Missing aria-* for dropdown. eg. aria-haspopup="true" aria-expanded="false"

-

I stopped now. It's too confusing. Leaving the remaining for next round.

@droplet the suggestions regarding selector names were very helpful! It made is possible to remove some repetition from the CSS, among other things.

Re: #87

• Initializing using jQuery once() in a separate file. Looks like #2402103: Add once.js to core may have some momentum, which is exciting, but I now see why we need to stick with once()
• Addressed the issue with the single item splitbutton and added that use case to test coverage
• Was able to get all the event listener additions to a single line with a little bit of renaming
• Completely removed the splitbutton_arrow from the render array. This was originally pulled from dropbutton, but isn't efficient here and Claro doesn't use it at all. All the arrows can be added by styling .splitbutton__trigger, like Claro already did, and the screenreader text can be added with an aria attribute

Re #96

• Refactored classnames and the CSS that use them
• Implemented the missing aria-expanded attributes

Also these changes:

• RTL support
• Per the menu button pattern, first item is focused on open (unless done via the up arrow, in which case the last item is focused)

1. +++ b/core/includes/theme.inc
   @@ -630,6 +630,35 @@ function template_preprocess_datetime_wrapper(&$variables) {
   +  $variables['list_item_attributes'] = new Attribute($variables['list_item_attributes']);
   

   While it's nice to be able to control the attributes of all list items at once, it could be a bit limiting in some cases. I think traditionally we have allowed controlling attributes of each item individually. This is the case for item-list for example.

2. +++ b/core/includes/theme.inc
   @@ -630,6 +630,35 @@ function template_preprocess_datetime_wrapper(&$variables) {
   +      if (isset($operation['#type']) && in_array($operation['#type'], $elements)) {
   

   I'm wondering is we should also assert the type so that we could trigger an exception in dev environments.

3. +++ b/core/lib/Drupal/Core/Render/Element/Splitbutton.php
   @@ -0,0 +1,195 @@
   +class Splitbutton extends RenderElement {
   

   Could be move some of the theming aspects of this render element to a template? We still have #2655374: Move classes from render elements to templates to resolve but I guess it would make sense to try to avoid adding more classes to the scope of that issue where possible.

4. +++ b/core/lib/Drupal/Core/Render/Element/Splitbutton.php
   @@ -0,0 +1,195 @@
   +        'data-splitbutton-trigger' => $trigger_id,
   
   +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,355 @@
   +      this.triggerContainer = splitbutton.querySelector('.splitbutton__main');
   

   I think we have pretty consistently prefixed our data attributes with drupal- to avoid conflicts.

5. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,355 @@
   +      splitbutton.classList.add(
   +        'splitbutton--enabled',
   ...
   +      );
   

   Should this be something that could be overrided for theming purposes?

6. +++ b/core/modules/system/templates/operation-list.html.twig
   @@ -0,0 +1,21 @@
   +    <ul {{ attributes }}>
   ...
   +            <li {{ list_item_attributes }}>{{ item }}</li>
   

   Nit: Extra space before the attributes.

Should we add something to the open button in Stark just to make it usable?

Splitbutton is totally the Button + Dropdown. There's no reason not to split it into 2 parts and take the advantages of code sharing

Providing selectors to style it as a dropdown could be as simple as adding a "dropdown" #splitbutton_type, or by creating a '\Dropdown' render element that extends \SplitButton and makes #title mandatory

As a developer, I wouldn't expect to make a drop-down by modifying a split button. And even if it worked, I'd worry that what I'd done was unanticipated, might break in edge cases I hadn't considered, and might be brittle to future changes.

If SplitButton extended Dropdown, then I'd understand that what I was doing was reasonable and intended. It would signal that to me.

As a developer, I wouldn't expect to make a drop-down by modifying a split button. And even if it worked, I'd worry that what I'd done was unanticipated, might break in edge cases I hadn't considered, and might be brittle to future changes.

If SplitButton extended Dropdown, then I'd understand that what I was doing was reasonable and intended. It would signal that to me.

There's currently no Dropdown render element. I mentioned in #95, once Splitbutton is added to core, we could then introduce a Dropdown render element that extends Splitbutton Adding a dropdown would be as simple as something like

[
  '#type' => 'dropdown',
 '#title' => $this-t('I am a dropdown'),
 '#items' => $items,
]

my expectation:

dropdown-core
dropdown-core + splitbutton-core
dropdown-core + splitbutton-core + splitbutton-core__submit_button_handler
dropdown-core + (splitbutton-core) + autocomple-core
dropdown-core + selection (replacing HTML select)
dropdown-core + many+many

both RED & BLUE are buttons
RED is button
BLUE can be called dropdown also

ALL BLUE should be the same structure (code)
RED can be different than BLUE

To make a great component, we should consider more fixable usage, e.g. using on Toolbar.

In another word, the below case should be accepted: No direct hierarchical dependency.

<div class="dropdown">
  <button class="btn dropdown-toggle">
    Dropdown
  </button>
  <div class="dropdown-menu"></div>
</div>

<div class="dropdown">
  <button class="btn dropdown-toggle">
    Dropdown
  </button>
</div>
...
...
...
...
..
..

<div class="dropdown-menu"></div>
</body>

Drupal needed a coder to annotate the UX style-guide to figure the common issues and suggestions from the NEW style-guide before actual patching.

This addresses #99 and #100.
Additional changes were made to make extending splibutton more flexible. This was inspired by #3076171: Provide a new library to replace jQuery UI autocomplete, which proposed leveraging splitbutton for a new autocomplete element. The tests were expanded to include a custom element extending Splitbutton. This element is not a full autocomplete, it's more to prove that it's possible to extend splitbutton in certain way. It is close enough, however, that it could be used as a starter for an autocomplete render element.

Note that I didn't see #103 until I was posting this. I don't yet fully understand what is being asked there (I'll read through a few more times), but wanted to make it clear that this patch is not informed by anything in that comment despite appearing after it.

Missed #99.5 in the patch in #105. Will take care of that in the next patch that will be needed regardless -- there's bound to be changes needed in that 97k

1. +++ b/core/includes/theme.inc
   @@ -2042,6 +2095,26 @@ function drupal_common_theme() {
   +    'splitbutton_item_list' => [
   

   This seems quite specific. Maybe we could make some changes to the item_list element to support splitbutton use case.

2. +++ b/core/themes/stable/templates/misc/splitbutton.html.twig
   --- /dev/null
   +++ b/core/themes/stable9/css/core/splitbutton/splitbutton.css
   

   This seems pretty functional. Can we target js- prefixed classes or data-drupal-selector attributes to make sure people don't accidentally break these?

3. +++ b/core/themes/stable9/templates/misc/splitbutton.html.twig
   @@ -0,0 +1,52 @@
   +    'splitbutton',
   ...
   +    splitbutton_multiple == true ? 'splitbutton--multiple' : 'splitbutton--single',
   

   Can we remove these in Stable? According to consensus banana, we should have only functional classes in core / stable. Functional classes should be prefixed with js-.

4. +++ b/core/themes/stable9/templates/misc/splitbutton.html.twig
   @@ -0,0 +1,52 @@
   +    'js-splitbutton',
   

   @nod_ pointed out on another issue that he would prefer us to use data-drupal-attribute instead.

Tagging for review at future UX meeting

Nice to see that finally 👍

It also paves the way to the last missing peace of view 7 to land in core 9 - https://www.drupal.org/project/views_jump_menu

I'm also glad to see this happen soon :)

This addresses #99.5 and #107.2-4. It was a significant refactoring to separate functional from visual.

Re #107.1

+++ b/core/includes/theme.inc
@@ -2042,6 +2095,26 @@ function drupal_common_theme() {
+ 'splitbutton_item_list' => [

This seems quite specific. Maybe we could make some changes to the item_list element to support splitbutton use case.

I tried a few different approaches before deciding on this one - the first was to declare item_list__splitbutton as #theme, but that kind of override won't work in a module. I then looked into leveraging the existing item-list element. This would work if using the template in modules/system/templates, but all of the overrides add a wrapper <div> around the <ul>, which would make this deviate from the a11y menu button pattern. Adding a conditional property to include/exclude the wrapper <div> seems too unweildy, but perhaps there's a way to use template suggestions on the module level that I overlooked?

Confirmed with @rain that the div sometimes present in item_list does not break the menu button pattern. This eliminates the need for a separate 'splitbutton_item_list'. The existing item list can be used.

1. +++ b/core/lib/Drupal/Core/Render/Element/Splitbutton.php
   @@ -0,0 +1,210 @@
   + * Provides a form element for a toggleable menu button.
   

   Should we explicitly document that this is based on the accessible menu button design pattern?

2. +++ b/core/misc/splitbutton/splitbutton-init.es6.js
   @@ -0,0 +1,29 @@
   +  Drupal.behaviors.splitButton = {
   

   Nit: s/splitButton/splitbutton

3. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   + * Splitbutton feature.
   

   I'm wondering if we should document that we have used the example from W3C as a starting point and reference to the original license https://www.w3.org/Consortium/Legal/2015/copyright-software-and-document 🤔

4. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   +      // The splitbutton items use the item-list template, but should not be
   +      // styled as an item list. Removing the class here addresses this for any
   +      // theme displaying a splitbutton.
   +      const itemList = splitbutton.querySelector('.item-list');
   +      if (itemList) {
   +        itemList.classList.remove('item-list');
   +      }
   

   What if the theme has overridden the template and is using another class for targeting it? Unfortunately, I think we need a backend solution for this. Maybe we should use the theme system hack that allows modules to provide templates for theme suggestions.

5. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   +          this.menu.getAttribute('data-drupal-item-tags') || 'a, input, button';
   

   Should we name this with something specific to splitbuttons?

6. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   +      setTimeout(() => this.focusFirst(), 0);
   

   Do we want to focus the first item even when using mouse for opening the tray?

7. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   +      // Wait half a second before closing.
   +      this.timerID = window.setTimeout(() => this.toggle(false), 500);
   

   Can we add explanation why this is needed here?

8. +++ b/core/misc/splitbutton/splitbutton.es6.js
   @@ -0,0 +1,374 @@
   +            this.setFocusByFirstCharacter(e, char.toLowerCase());
   

   This is v cool 😎

9. +++ b/core/modules/system/templates/splitbutton.html.twig
   @@ -0,0 +1,38 @@
   +      {{ main_element }}
   +      {% if splitbutton_multiple and exclude_toggle == FALSE %}
   +          <button {{ toggle_attributes }}>
   +            {{ title }}
   +          </button>
   +      {% endif %}
   

   Nit: This should be intended with 4 spaces instead of 6.

10. +++ b/core/themes/seven/templates/splitbutton.html.twig
    @@ -0,0 +1,74 @@
    +      {{ main_element }}
    +      {% if splitbutton_multiple and exclude_toggle == FALSE %}
    +          <button {{ toggle_attributes.addClass(toggle_button_classes) }}>
    +            {{ title }}
    +          </button>
    +      {% endif %}
    
    +++ b/core/themes/stable/templates/misc/splitbutton.html.twig
    @@ -0,0 +1,36 @@
    +      {{ main_element }}
    +      {% if splitbutton_multiple and exclude_toggle == FALSE %}
    +          <button {{ toggle_attributes }}>
    +            {{ title }}
    +          </button>
    +      {% endif %}
    
    +++ b/core/themes/stable9/templates/misc/splitbutton.html.twig
    @@ -0,0 +1,36 @@
    +      {{ main_element }}
    +      {% if splitbutton_multiple and exclude_toggle == FALSE %}
    +          <button {{ toggle_attributes }}>
    +            {{ title }}
    +          </button>
    +      {% endif %}
    

    Nit: The indentations need some adjustments.

11. 
    I think rendering the focus ring inside the tray would look better. You can look at the primary tabs on mobile for an example.
12. 
    How should this look like?
13. 
    Seems like text inside the tray in Seven need some adjustment because for some reason it looks distorted.
14. 
    I think we should increase the right padding to ensure that long texts don't overlap with the arrow.

This addresses everything in #113, but it's likely some items will need another round of attention.

#113.3

I'm wondering if we should document that we have used the example from W3C as a starting point and reference to the original license

I'm not sure I fully understood the license requirements on the w3 page, but I provided my best attempt - at worst it's probably easier for a reviewer to point out what needs changing instead of having to write it entirely.

#113.6

Do we want to focus the first item even when using mouse for opening the tray?

Subjectively I don't prefer this, when using the mouse there's an expected correlation of pointer position and focused element. The W3 examples don't do this either. I did, however, notice that some examples were able to open on hover, so I added a #hover property to the splitbutton element. When TRUE (provided that #title is not empty), the splitbutton will open on hover. This defaults to FALSE.

#113.13

Seems like text inside the tray in Seven need some adjustment because for some reason it looks distorted.

This is due to using text-shadow because using font-weight:bold changes the width of the items on hover/focus. I changed the values a bit to see if that helps. If not, another approach should be explored, or the design may need to be changed a bit. The dropbutton styles I used for reference at https://groups.drupal.org/node/283223#Split_Buttons_and_Dropbuttons were from 7 years ago, so it probably isn't necessary to adhere to that specific vision

Needed to copy the new templates to Stable/Stable9 to address the failed tests in #114.

I think #115/#116 starts to feel quite nice! ✨I think also the Seven designs look good after the changes in #114. 👏

However, I'm getting lots of following errors on the page:

User error: "attributes" is an invalid render array key in Drupal\Core\Render\Element::children() (line 97 of core/lib/Drupal/Core/Render/Element.php).
Drupal\Core\Render\Element::children(Array) (Line: 1143)
template_preprocess_item_list(Array) (Line: 1185)
template_preprocess_item_list__splitbutton(Array, 'item_list__splitbutton', Array) (Line: 287)
Drupal\Core\Theme\ThemeManager->render('item_list__splitbutton', Array) (Line: 431)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 200)
Drupal\Core\Render\Renderer->render(Array) (Line: 450)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 138)

I think the next steps should be to:

1. Schedule a call with the accessibility topic maintainers to review the accessibility aspects of this in detail. On that call we should make a decision on #113.6 and whether we should support the hover variation
2. Get review on the licensing requirements from a member of the licensing working group and/or committer more familiar with licensing.

The error in #118 was due to the theme system hack being a little more complex than the system_branding_block hack I referenced in order to do it for splitbutton. A change to template_preprocess_item_list__splitbutton() takes care of it, and I'm definitely open to feedback if there's a more effective way to accomplish this.

Discussed with @rainbreaw and @bnjmnm about #113.6 and we agreed that when using a pointer device, the focus should remain on the toggle button because users using the pointer could find it confusing that the focus doesn't match with the pointer after the tray has been opened.

Crediting @rainbreaw for accessibility guidance.

Addresses #120

Thank you @bnjmnm 👏 I believe the next step is to get thorough UX and accessibility review on the new element, as well as get review on the licensing requirements from a member of the licensing working group and/or committer more familiar with licensing.

This patch:

1. Adds tests for the deprecations introduced in the patch
2. Uses a less clunky way to loop through splitbutton initialization
3. Added a usage example to the Splitbutton render element docblock

Would it be possible to implement this in a way that wouldn't cause this slight change in the position of the text when an item in the tray receives focus?

This was discussed as part of the usability call yesterday. People on the call recommended two next steps:

• Under "Proposed resolution", mention the main visual difference between the two: when expanded, the drop button's primary option is part of the list, but in a split button it is not.
• Under "User interface changes", add "before" and "after" screenshots. There are already some screenshots in the comments, but none in the issue summary.

One more todo is to add splitbutton to the cspell dictionary

Created an issue for assistance with licensing: #3167444: Guidance on adhering to w3 licensing guidelines.
Created followup for creating a dropdown element that extends splitbutton: #3167446: [PP-1] Create "dropdown" render element that extends splitbutton

I'm still don't understand why extend dropdown from splitbutton. BUT not to extend splitbutton from dropdown.

This patch addresses #126 and refactors the svg use to comply with the approach implemented in #3085245: Un-inline SVGs in pcss.css files, add build tool to inline them when compiled.

There is some additional work needed to address some issues in Claro when the .touchevents class is present.

1. Padding on splitbuttons without dedicated toggles still need to be fixed. At narrower widths the text and icon can overlap.
2. Small/extrasmall variants for dropbuttons in .touchevents are currently not available, but may be after #3083256: Create smaller variations for form elements lands. The way it is styled in this issue will be informed by how that issue turns out, I'm going to try to get that moving as it's close to completion.

Regarding #130

I'm still don't understand why extend dropdown from splitbutton. BUT not to extend splitbutton from dropdown.

This explanation has more to do with process and circumstances rather than a architectural ideal, but hope this helps.

• Dropdown doesn't currently exist as a render element, so there's nothing for splitbutton to extend.
• Claro could not be made stable without a replacement for Dropbuttons that met certain criteria. Splitbutton would meet this criteria.
• Splitbutton's implementation makes it a suitable dropbutton replacement, and efforts were made to make it more versatile than dropbutton since dropbutton had proven inflexible. Part of this flexibility happens to be it can easily be repurposed as a dropdown.

I personally prefer the approach of dropdown extending splitbutton as this will require very little additional code, it simply needs to be a splitbutton that requires #title and adds a wrapper class. The complexity is centralized in the splitbutton render element, which I find easier for debugging.

While this is my personal preference, I'm open to discovering why the other approach is more beneficial for Drupal overall. Fortunately, determining that does not have to prevent the current splitbutton implementation from being added to core. That decision can be made in #3167446: [PP-1] Create "dropdown" render element that extends splitbutton, and splitbutton can be refactored to extend dropdown if there are compelling arguments to do so.

This addresses the padding issue I mentioned in #131. Figured out enough about the direction of #3083256: Create smaller variations for form elements to determine it shouldn't block this issue.

Added a deprecation for dropbutton and that deprecation is included in \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations since replacing every dropbutton in core isn't easily done.

Received usability review & usability blessing at #3165887: Drupal Usability Meeting 2020-08-25

Reroll

Reroll

I no longer think we should use the ARIA menu roles here. Sorry to flip-flop on this one!

Looking back at comment #67, I'm embarrassed by what I wrote there. I was basically saying "Oooh, here's an ARIA thing. Let's use it!". But I didn't stop to appraise whether it was useful or appropriate for the contexts in which we currently use the dropbutton.

Some important points:

• The button needs to convey it's open/closed behaviour to assistive technology. A simple disclosure button will suffice for that; it doesn't need the menu roles.
• The ARIA menu roles are for intended modelling desktop application menus. However our existing dropbuttons are overwhelmingly used for navigation links.
• The ARIA Authoring Practices document does include some patterns which use the menu roles for navigation use cases. However, these are proving controversial in the accessibility community, and there are many detractors. I'll dig out some material I have bookmarked about this (there's plenty of it).
• Most of our existing dropbuttons have a short list of items (typically 2-6, rarely anything higher). Claiming the page up/down, home, and end buttons is quite greedy I think. Skimming through the patch, I don't think it would be too disruptive to remove these behaviours. The ESC behaviour may be worth keeping.

I'll dig out some material I already have bookmarked about this.

Quoting myself from #3165887-17: Drupal Usability Meeting 2020-08-25 this issue was discussed during the meeting and unfortunately I missed the meeting.

Just catching up on the meeting recording, very pleased to see how far progress has come with the Split Button render element(s), this looks like a good improvement over the existing Dropdown Button element(s) and I agree overall with what was said during the meeting.

The only thing I wonder, during the meeting it was mentioned that any render elements could be added as items inside the drop-down area, examples were given of needing to put buttons inside the drop-down area, and for good reason. However, is there a risk that by given too much freedom, the UI can break? For instance, hypothetically a developer could decides to place a select render element as one of the items in the drop-down area. Am I understanding this correctly, and if so is this a problem that we need to be concerned about, or is it even a problem at all, is that "by design" so that we don't make any assumptions and we put the responsibility of fixing any broken UI on the developer?

Another specific thing I wanted to highlight, I do like the new behaviour of the Split Button element where the primary item is always highlighted as the primary item and not shown as part of the list. My thinking here is that, as a developer or designer I'm making an active decision to say which of these items is the primary item because (presumably) I know that this is the most valuable one in the majority of cases, I wouldn't expect that to change when the drop-down area is open, therefor I don't have any strong objections to just loosing the behaviour of the "legacy" drop-down/drop-button because I don't think it adds any real value compared with the value that the new Split Button element adds. Besides, as highlighted, if I really have a use-case for all items being treated equally, I can just use the option where the button is just a toggle.
TLDR: new one is great, covers enough common use-cases, I think the old pattern can safely be retired.

Great work everyone!

I was reconsidering the implementation of Splitbutton based on what I'd learned from an accessibility course I recently completed, and it turns out #136 made essentially the same conclusion (with a far better explanation than I could muster 🙂).

The information in #136 would be all I'd need for making the appropriate changes to Splitbutton, but I'd like to make sure this is implemented in a way that facilitates easy extensibility and reuse. It would help to know a bit more about the bit mentioned here:

The ARIA Authoring Practices document does include some patterns which use the menu roles for navigation use cases. However, these are proving controversial in the accessibility community, and there are many detractors. I'll dig out some material I have bookmarked about this (there's plenty of it).

This could give me a better sense of how the functionality provided here could potentially be used for dropdown navigation. There are some reasonable assumptions I can make, but more details will help predict how this may get used.

Regarding this from #137

The only thing I wonder, during the meeting it was mentioned that any render elements could be added as items inside the drop-down area, examples were given of needing to put buttons inside the drop-down area, and for good reason. However, is there a risk that by given too much freedom, the UI can break? For instance, hypothetically a developer could decides to place a select render element as one of the items in the drop-down area. Am I understanding this correctly, and if so is this a problem that we need to be concerned about, or is it even a problem at all, is that "by design" so that we don't make any assumptions and we put the responsibility of fixing any broken UI on the developer?

I'm leaning towards "we put the responsibility of fixing any broken UI on the developer". In the next patch I'll add docs that list the render elements guaranteed to work without the need for further customization.

The following articles provided me a much better sense of where menu roles should/shouldn't be used

• Don’t Use ARIA Menu Roles for Site Nav
• Inclusive Components: Menus & Menu Buttons

Based on what I learned there it became pretty clear that the menu pattern is not appropriate in this case, and it's probably also worth reducing some of characters commandeered by keypresses.

The next step would be to figure out the best way to rein in some of splitbutton's functionality while continuing to offer it in a way that facilitates easy reuse such as dropdowns. This may require splitting this into multiple render elements. I'm assigning to myself as I have an idea of how do this and intend to provide a patch. I may not be able to do this immediately so if anyone would like to have a go at it, ping me on Drupal slack and we can coordinate!

This patch changes the accessibility implementation inspired by #136, the articles I mentioned in #140 and an an additional article https://www.marcozehe.de/wai-aria-menus-use-with-care/. A toggleable list of links/buttons doesn't require all the extra baggage that the menu button pattern introduces. It should be sufficient to have a button with aria-haspopup and aria-expanded, with the first item being focused when the list is expanded.

The greedy keyboard handling has been reduced to arrows, escape, enter and space. Arrow up/down still work like the previous implementation, but it's now also possible to tab through the items instead of having it close the list and advance to the next tabbable item. Some of the reasons informing this decision:

• The somewhat recent "tab to widget, only permit arrow within widget" A11y pattern that is being encouraged doesn't strike me as applicable here as a list with toggleable visibility doesnt seem complex enough to qualify as a widget. It should still be possible to use arrows as that is a common expectation, but it doesn't have to ONLY use arrows.
• The arrows-or-tab functionality is a familiar pattern to users (things such as bootstrap navbar use it). I'd like Splitbutton to feel familiar to use, provided doing so does not cause accessibility problems.
• Some users may expect it to work like a select element, so it's good to support arrows. Some users may expect tab as it's a list of focusable elements, plus and that's how Drupal dropbuttons work. Supporting both === fewer annoyed users.

Tests were also updated to reflect these changes, plus the render element in the splitbutton_test module can be repurposed fairly easily for #3167446: [PP-1] Create "dropdown" render element that extends splitbutton, and demonstrates why creating a dropdown element that extends splitbutton winds up being easier to manage than a splitbutton extending a dropdown - despite the latter scenario seeming like the better way to do it.

Had to update SplitbuttonDeprecationTest due to changes in #141 and the @expectDeprecation annotation being deprecated.

Deleting this from the issue summary (introduced in #132 without explanation):

Dropbutton does not adhere to any established a11y pattern

Oh yes it does! It's a simple disclosure button, and it works pretty well. You press it, and additional content is revealed. Press it again, and that content is hidden.

It could be greatly improved by conveying it's behaviour and state programmatically, for which we have #2893663: Dropbutton should report open/closed state to assistive technology. At the time when dropbutton was created, support for aria-expanded was patchy. So the approach of using the button name to convey the behaviour and infer the current state was good enough.

tagging

This appears to have gone through several robust code reviews, I think a decent next-step would be condensing the knowledge here into a change record.

Hiding patches now that we have a MR.

I acknowledge this is way over my head (props to all contributors 👏) so my review is mostly cosmetic, feel free to disregard anything that doesn't make much sense. I'll try to get more involved in the coming days if I can.

Moving back to needs work for some cleanup based on feedback above.