Page MenuHomePhabricator

MenuButton: Publish the MenuButton component (take it out of WIP)
Closed, ResolvedPublic5 Estimated Story Points

Description

Once T363858 is complete, there is some follow-up work which will need to happen before the MenuButton component can be taken out of WIP and made available to users.

Feature updates
  • Question: Do we want persistent selection state in this component, or should it only be responsible for triggering actions (via emitting events based on selection), with the selection immediately cleared after an event is emitted? Currently the component functions both as a menu (persistent selection remains after a choice is made) AND as a button (an event is immediately fired upon selection, allowing the parent component to respond in whatever way is appropriate).
  • The toggle button should only have one label, either an aria-label for icon-only buttons or the visible button label (when the button has text).
    • Remove the duplicate button label that occurs when a button has a visible button label and the default aria-label. Refer to this comment and subtask T366538.
Design Updates
  • Include minimum and maximum width.
  • Evaluate removing the selection within the menu within the main demo, since this selection is not recommended in the MenuButton Best practices.
Documentation Updates

The component should include a full documentation page in the Codex docs site before publication, including component guidelines, illustrations, and interactive demos.

  • Add the complete set of component guidelines to the docs page (Completed in patch 1035775)
  • Add a configurable demo to the docs page
  • Add demos illustrating key modes of usage: default icon ("..."), custom button content, menu items with icons
  • Add a demo illustrating selection behavior similar to ARIA APG's MenuButton Pattern example. Refer to this comment for implementation.
Acceptance criteria
  • Move the component out of WIP and export in the lib
  • Include the 2 new types introduced with this component (MenuButtonItemData and FloatingMenuOptions) in the lib

Event Timeline

CCiufo-WMF set the point value for this task to 5.May 28 2024, 6:48 PM
CCiufo-WMF triaged this task as Medium priority.May 28 2024, 6:51 PM
lwatson changed the task status from Open to In Progress.May 29 2024, 2:27 PM
lwatson updated the task description. (Show Details)

Change #1037120 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: remove `export` from public types

https://gerrit.wikimedia.org/r/1037120

Change #1037199 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: add MenuButton to Menu docs

https://gerrit.wikimedia.org/r/1037199

Change #1037120 merged by jenkins-bot:

[design/codex@main] docs: remove `export` from public types

https://gerrit.wikimedia.org/r/1037120

Change #1037547 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs: add MenuButton demos

https://gerrit.wikimedia.org/r/1037547

I noticed when looking at Lauralyn's demo page patch that, if the button has a visible label, it also has a redundant aria-label. Buttons should only ever have one of these things, not both.

I would recommend we do what we do for similar patterns in Codex:

  • Make toggleButtonLabel a required prop (done)
  • Show it by default
  • Add a hideLabel prop, which defaults to false and can be set to true to visually hide the label

This way we will never be mixing the toggleButtonLabel prop with the button slot.

cc @egardner @bmartinezcalvo

Change #1037199 merged by jenkins-bot:

[design/codex@main] docs: add MenuButton to Menu docs

https://gerrit.wikimedia.org/r/1037199

@Anne - Thanks, I'll create a separate patch to address the label as you suggested.

The design and feature updates from this task mention changing the logic behind the selection of menu items. As per ARIA APG, we'd need to (1) clear the previously selected menu item, (2) focus on the first menu item when the menu opens, and (3) update the keyboard navigation. Do we expect these changes to be part of the acceptance criteria for the current sprint before taking the MenuButton out of WIP?

cc: @egardner @bmartinezcalvo

The design and feature updates from this task mention changing the logic behind the selection of menu items. As per ARIA APG, we'd need to (1) clear the previously selected menu item, (2) focus on the first menu item when the menu opens, and (3) update the keyboard navigation. Do we expect these changes to be part of the acceptance criteria for the current sprint before taking the MenuButton out of WIP?

cc: @egardner @bmartinezcalvo

For selection behavior, I think we can handle this in the demos without having to modify the underlying MenuButton component.

I think we'd want to approach it this way:

  • The parent component (let's call it "MenuButtonDemo") would listen for @update:selected events on the MenuButton and bind it to a local handler method (let's call it onSelect).
  • When onSelect is called, it would trigger some sort of action and then it would reset selection to the initial null value.
  • As far as what should actually happen – we want the user to see something visual, and it should probably be temporary (because they may keep selecting different options to see what happens). Maybe the demo could cause a Message component to appear below the button that says "You chose ${selection}" with an autodismiss property set so it goes away after a few seconds.

I don't think the configurable demo needs to behave this way (there is already a "reset demo" button in that case. Really we probably only need one demo with this behavior.

Change #1037547 merged by jenkins-bot:

[design/codex@main] docs: add MenuButton demos

https://gerrit.wikimedia.org/r/1037547

Change #1037854 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] MenuButton: remove duplicate toggle button label

https://gerrit.wikimedia.org/r/1037854

Change #1037889 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs, MenuButton: demo a selection triggering an action

https://gerrit.wikimedia.org/r/1037889

Change #1037854 abandoned by LWatson:

[design/codex@main] MenuButton: remove duplicate toggle button label

Reason:

New proposed solution: T366538

https://gerrit.wikimedia.org/r/1037854

@lwatson as decided during our DST meeting, since the MenuButton needs to be fixed width for now, let's use @size-1600 (256px) instead. I will update the MenuButton guidelines accordingly.

Change #1037889 merged by jenkins-bot:

[design/codex@main] docs, MenuButton: demo a selection triggering an action

https://gerrit.wikimedia.org/r/1037889

Change #1039784 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] docs, MenuButton: update demos based on design feedback

https://gerrit.wikimedia.org/r/1039784

Change #1039814 had a related patch set uploaded (by Bmartinezcalvo; author: Bmartinezcalvo):

[design/codex@main] docs: update MenuButton width in the guidelines

https://gerrit.wikimedia.org/r/1039814

Change #1040172 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] MenuButton: Publish the MenuButton component

https://gerrit.wikimedia.org/r/1040172

Change #1039784 merged by jenkins-bot:

[design/codex@main] docs, MenuButton: update demos based on design feedback

https://gerrit.wikimedia.org/r/1039784

Change #1039801 merged by jenkins-bot:

[design/codex@main] MenuButton: update the maximum width

https://gerrit.wikimedia.org/r/1039801

Change #1039814 merged by jenkins-bot:

[design/codex@main] docs: update MenuButton max-width in the guidelines

https://gerrit.wikimedia.org/r/1039814

Change #1040172 merged by jenkins-bot:

[design/codex@main] MenuButton: Publish the MenuButton component

https://gerrit.wikimedia.org/r/1040172

Change #1041032 merged by jenkins-bot:

[design/codex@main] docs: update MenuButton's "Component limitations" guidelines

https://gerrit.wikimedia.org/r/1041032

Change #1041776 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v1.6.1 to v1.7.0

https://gerrit.wikimedia.org/r/1041776

Change #1041776 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v1.6.1 to v1.7.0

https://gerrit.wikimedia.org/r/1041776