Skip to content

Conversation

Westbrook
Copy link
Contributor

@Westbrook Westbrook commented Jul 28, 2021

Description

Allow <sp-menu> elements to manage their own selection via selects="single|multiple|inherit". By default, an <sp-menu> element will take no selection, as it has previously. A selects value of single or multiple will make this element a selection root and manage the selection of <sp-menu-item> children and descendants of [selects="inherit"] children. When a Menu element has selects="inherit", it's <sp-menu-item> descendants will participate in the selection of the nearest ancestor with selects="single|multiple". Each Menu root that manages a selection will dispatch change events outlining updates to its value. In this way, multiple selection roots inside of a non-selecting <sp-menu> parent can manage these changes via event delegation, as seen here.

Single - Demo

<sp-menu selects="single">
<sp-menu-item selected>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-menu>

Multiple - Demo

<sp-menu selects="multiple">
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item selected>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item selected>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-menu>

Inherit - Demo

<sp-menu label="Groceries" selects="multiple">
<sp-menu-group label="Juice" selects="inherit">
<sp-menu-item selected>Orange</sp-menu-item>
<sp-menu-item selected>Apple</sp-menu-item>
<sp-menu-item>Grape</sp-menu-item>
</sp-menu-group>
<sp-menu-divider></sp-menu-divider>
<sp-menu-group label="Vegetables" selects="inherit">
<sp-menu-item>Carrots</sp-menu-item>
<sp-menu-item selected>Summer Squash</sp-menu-item>
<sp-menu-item>Zuccini</sp-menu-item>
</sp-menu-group>
<sp-menu-divider></sp-menu-divider>
<sp-menu-group label="Dry Goods" selects="inherit">
<sp-menu-item>Ceral</sp-menu-item>
<sp-menu-item selected>Flour</sp-menu-item>
<sp-menu-item>Salt</sp-menu-item>
<sp-menu-item>Sugar</sp-menu-item>
</sp-menu-group>
</sp-menu>

Complex - Demo

<sp-menu label="Style/Color" @change=${update}>
<sp-menu-group label="Font Style" selects="multiple" id="font">
<sp-menu-item value="bold" selected>Bold</sp-menu-item>
<sp-menu-item value="italic" selected>Italic</sp-menu-item>
</sp-menu-group>
<sp-menu-divider></sp-menu-divider>
<sp-menu-group label="Text Color" selects="single" id="color">
<sp-menu-item value="black">Black</sp-menu-item>
<sp-menu-item value="blue" selected>Blue</sp-menu-item>
<sp-menu-item value="red">Red</sp-menu-item>
<sp-menu-item value="green">Green</sp-menu-item>
</sp-menu-group>
<sp-menu-divider></sp-menu-divider>
<sp-menu-group
label="Text Decoration"
selects="single"
id="decoration"
>
<sp-menu-item value="none">None</sp-menu-item>
<sp-menu-item value="overline" selected>
Overline
</sp-menu-item>
<sp-menu-item value="line-through">
Line-through
</sp-menu-item>
<sp-menu-item value="underline">Underline</sp-menu-item>
</sp-menu-group>
</sp-menu>

  • <sp-menu> now manages selection inside of <sp-picker> elements
  • surfaces selects attribute on <sp-action-menu> so that it can choose whether to take a selection of not. THIS IS A BREAKING CHANGE!
  • <sp-picker> will validate selects to single when set for all derivative elements, preventing the application of multiple. This should be added in follow up work.
  • <sp-menu-group> is now an extension of <sp-menu> simplifying the code there
  • corrects header correlation in <sp-menu-group> to ensure availability in the accessibility tree.
  • <sp-split-button> no longer takes a selection in type="more", correcting the visual delivery there
  • flips Alex's initial implementation where each Menu had a data representation of all of it's Menu Items to a data model of each Menu have access to it's Menu Items so that there was only a single representation of their state allowing for the event cycle there not to require new Event()s in each Menu Item
  • the above leverages events extended from Event rather than CustomEvents
  • Mentioning here (as well as below) that I need to add some documentation to various READMEs, but with so much functionality here, I wanted to get an official PR open for reviewers while I did so.

Related Issue

fixes #350
fixes #715
fixes #1189
fixes #1505

Motivation and Context

Menus need the ability to do much more than look pretty, and as the basis of a number of other UIs in the ecosystem they can reduce code duplication my having a central selection management implementation.

How Has This Been Tested?

  • lots of unit tests
  • lots of VRTs
  • lots of manual

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions
Copy link
Contributor

@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2021

Tachometer results for changed packages

action-bar permalink

Version Bytes Avg Time vs remote vs branch
remote 312 kB 20.81ms - 23.45ms - unsure 🔍
-3% - +13%
-0.60ms - +2.72ms
branch 310 kB 20.06ms - 22.07ms unsure 🔍
-12% - +2%
-2.72ms - +0.60ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
remote 773 kB 411.49ms - 421.10ms - unsure 🔍
-0% - +3%
-0.53ms - +13.03ms
branch 784 kB 405.26ms - 414.83ms unsure 🔍
-3% - +0%
-13.03ms - +0.53ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
remote 515 kB 305.30ms - 314.36ms - faster ✔
14% - 18%
52.69ms - 68.73ms
branch 526 kB 363.92ms - 377.16ms slower ❌
17% - 22%
52.69ms - 68.73ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
remote 765 kB 745.68ms - 769.43ms - slower ❌
0% - 5%
0.54ms - 34.84ms
branch 775 kB 727.49ms - 752.23ms faster ✔
0% - 5%
0.54ms - 34.84ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
remote 315 kB 36.28ms - 39.12ms - unsure 🔍
-2% - +7%
-0.88ms - +2.66ms
branch 313 kB 35.76ms - 37.87ms unsure 🔍
-7% - +2%
-2.66ms - +0.88ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
remote 784 kB 2253.61ms - 2277.93ms - faster ✔
6% - 8%
142.71ms - 187.37ms
branch 795 kB 2412.09ms - 2449.53ms slower ❌
6% - 8%
142.71ms - 187.37ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
remote 318 kB 30.61ms - 31.77ms - unsure 🔍
-2% - +3%
-0.52ms - +0.90ms
branch 316 kB 30.60ms - 31.41ms unsure 🔍
-3% - +2%
-0.90ms - +0.52ms
-

Copy link
Collaborator

@najikahalsema najikahalsema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most beautiful PR I've ever seen. Awesome job, and thanks for your hard work!

Copy link
Contributor

@stanvladut stanvladut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the changes look great 💯

Copy link
Collaborator

@adixon-adobe adixon-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a bunch of little comments, but overall this looks really awesome. Love seeing those added selects tests, and it's nice to see the simplification you made to item management.

These changes are deviously complex and inter-related so nice freaking job on this!

Thank you!

@adixon-adobe
Copy link
Collaborator

Updates look great!

@Westbrook Westbrook merged commit 8bd318a into next Jul 29, 2021
@Westbrook Westbrook deleted the menu-selects branch July 29, 2021 20:45
@Westbrook Westbrook restored the menu-selects branch July 29, 2021 20:52
@spdev3000
Copy link
Contributor

Awesome PR! Thanks for the hard work here!

@Westbrook Westbrook deleted the menu-selects branch July 31, 2021 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants