docs(picker): updated docs with allowed menu content #5446

nikkimk · 2025-05-06T22:30:20Z

Description

added example of picker using sp-menu-group
added accessbility warning not to use submenus

Motivation and context

While answering Slack support questions about sp-picker, I noticed we do not have examples of sp-picker with grouped menu items using sp-menu-group. This is a valid use case, so we should show it in the docs.

Additionally, I had a number of questions about using submenus and other interactive items in asp-picker. This is not accessible, and we should note that in the accessibility section of the docs.

Related issue(s)

SWC-848

Screenshots (if appropriate)

Author's checklist

I have read the CONTRIBUTING and PULL_REQUESTS documents.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
I have added automated tests to cover my changes. (N/A docs update only)
I have included a well-written changeset if my change needs to be published. (N/A docs update only)
I have included updated documentation if my change required it.

Reviewer's checklist

Includes a Github Issue with appropriate flag or Jira ticket number without a link
Includes thoughtfully written changeset if changes suggested include patch, minor, or major features (N/A docs update only)
Automated tests cover all use cases and follow best practices for writing (N/A docs update only)
Validated on all supported browsers
All VRTs are approved before the author can update Golden Hash

Manual review test cases

Picker Docs
1. Go to the picker docs
2. Does the documentation include that picker can be used with sp-menu-group elements in its menu?
3. Does the documentation make it clear that picker should not contain submenus, as they will render it inaccessible?
4. Does the documentation suggest using an action menu instead of a picker if submenus are needed?

Device review

Did it pass in Desktop?
Did it pass in (emulated) Mobile?
Did it pass in (emulated) iPad?

changeset-bot · 2025-05-06T22:30:23Z

⚠️ No Changeset found

Latest commit: 1463cad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

nikkimk · 2025-05-06T22:31:46Z

packages/picker/README.md

+If you require a submenu, use and [action menu](./action-menu) instead.
+
+```html demo
+<sp-picker>


Example with menu groups since picker can accept them.

github-actions · 2025-05-06T22:32:25Z

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

nikkimk · 2025-05-06T22:32:47Z

packages/picker/README.md

@@ -91,6 +93,32 @@ For an accessible label that renders within the bounds of the picker itself, but
 </sp-tab-panel>
 </sp-tabs>

+#### Menu
+
+The picker menu is a menu element that is used to display the options for the picker. A picker menu can include menu items, menu dividers, and menu groups. A picker menu should never contain submenus, as doing so would render it inaccessible.


Notes that submenus would make a picker inaccessible.

nikkimk · 2025-05-06T22:33:23Z

packages/picker/README.md

+
+#### Do not use submenus
+
+A picker menu should never contain submenus, as doing so would render it in accessible. A picker's menu role is a listbox and it's menu items are listbox options, which are not allowed to have submenus.


A more detailed explanation and reminder not to use submenus in a picker.

Reinforcement 🤌 TY

github-actions · 2025-05-06T22:35:27Z

Tachometer results

Currently, no packages are changed by this PR...

cdransf · 2025-05-06T22:36:48Z

packages/picker/README.md

+
+#### Do not use submenus
+
+A picker menu should never contain submenus, as doing so would render it in accessible. A picker's menu role is a listbox and it's menu items are listbox options, which are not allowed to have submenus.


Should this be inaccessible? ✨

packages/picker/README.md

caseyisonit · 2025-05-06T22:39:21Z

packages/picker/README.md

+
+The picker menu is a menu element that is used to display the options for the picker. A picker menu can include menu items, menu dividers, and menu groups. A picker menu should never contain submenus, as doing so would render it inaccessible.
+
+If you require a submenu, use and [action menu](./action-menu) instead of a picker.


🎶 I loveeeeee this so much

packages/picker/README.md

caseyisonit

Lets Get This Merged

Co-authored-by: Casey Eickhoff <[email protected]>

nikkimk · 2025-05-06T22:45:08Z

Lets Get This Merged

Oooh your LGTM game is strong, @caseyisonit!

docs(picker): updated docs with allowed menu content

4194208

docs(picker): fixed types

42cbf31

nikkimk commented May 6, 2025

View reviewed changes

Merge branch 'main' into nikkimk/picker-group-submenu-docs

9dc004d

nikkimk marked this pull request as ready for review May 6, 2025 22:33

nikkimk requested a review from a team as a code owner May 6, 2025 22:33

nikkimk added ready-for-review auto-update Rebase PR automatically labels May 6, 2025

cdransf reviewed May 6, 2025

View reviewed changes

packages/picker/README.md Outdated Show resolved Hide resolved

cdransf approved these changes May 6, 2025

View reviewed changes

caseyisonit reviewed May 6, 2025

View reviewed changes

packages/picker/README.md Outdated Show resolved Hide resolved

caseyisonit reviewed May 6, 2025

View reviewed changes

packages/picker/README.md Outdated Show resolved Hide resolved

caseyisonit approved these changes May 6, 2025

View reviewed changes

nikkimk and others added 2 commits May 6, 2025 18:44

docs(picker): fixed typo

261d866

Co-authored-by: Casey Eickhoff <[email protected]>

docs(picker): fixed typos

d9f8121

Co-authored-by: Casey Eickhoff <[email protected]>

nikkimk enabled auto-merge (squash) May 6, 2025 22:45

Rajdeepc approved these changes May 7, 2025

View reviewed changes

Merge branch 'main' into nikkimk/picker-group-submenu-docs

1463cad

nikkimk merged commit 41c1a1b into main May 7, 2025
24 checks passed

nikkimk deleted the nikkimk/picker-group-submenu-docs branch May 7, 2025 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(picker): updated docs with allowed menu content #5446

docs(picker): updated docs with allowed menu content #5446

nikkimk commented May 6, 2025 •

edited

Loading

changeset-bot bot commented May 6, 2025 •

edited

Loading

nikkimk May 6, 2025

github-actions bot commented May 6, 2025

nikkimk May 6, 2025

nikkimk May 6, 2025

caseyisonit May 6, 2025

github-actions bot commented May 6, 2025

cdransf May 6, 2025

caseyisonit May 6, 2025

caseyisonit left a comment

nikkimk commented May 6, 2025


		#### Do not use submenus

		A picker menu should never contain submenus, as doing so would render it in accessible. A picker's menu role is a listbox and it's menu items are listbox options, which are not allowed to have submenus.


		The picker menu is a menu element that is used to display the options for the picker. A picker menu can include menu items, menu dividers, and menu groups. A picker menu should never contain submenus, as doing so would render it inaccessible.

		If you require a submenu, use and [action menu](./action-menu) instead of a picker.

docs(picker): updated docs with allowed menu content #5446

docs(picker): updated docs with allowed menu content #5446

Conversation

nikkimk commented May 6, 2025 • edited Loading

Description

Motivation and context

Related issue(s)

Screenshots (if appropriate)

Author's checklist

Reviewer's checklist

Manual review test cases

Device review

changeset-bot bot commented May 6, 2025 • edited Loading

⚠️ No Changeset found

nikkimk May 6, 2025

Choose a reason for hiding this comment

github-actions bot commented May 6, 2025

Branch preview

Review the following VRT differences

nikkimk May 6, 2025

Choose a reason for hiding this comment

nikkimk May 6, 2025

Choose a reason for hiding this comment

caseyisonit May 6, 2025

Choose a reason for hiding this comment

github-actions bot commented May 6, 2025

Tachometer results

cdransf May 6, 2025

Choose a reason for hiding this comment

caseyisonit May 6, 2025

Choose a reason for hiding this comment

caseyisonit left a comment

Choose a reason for hiding this comment

nikkimk commented May 6, 2025

nikkimk commented May 6, 2025 •

edited

Loading

changeset-bot bot commented May 6, 2025 •

edited

Loading