-
Notifications
You must be signed in to change notification settings - Fork 244
docs(menu, action-menu): component analysis #5981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e165f73
c3e3d7a
13922a1
f0963cb
16a941f
b41f6e0
601f7f9
a21c7b4
7e7651f
d817bf0
96e320d
faffe45
437d23f
0936149
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| # Action Menu migration roadmap | ||
|
|
||
| ## Component specifications | ||
|
|
||
| ### CSS | ||
|
|
||
| Action Menu does not have its own CSS component. It is a composite pattern that combines Action Button, Popover, and Menu components. | ||
|
|
||
| <details> | ||
| <summary>CSS selectors</summary> | ||
|
|
||
| None found for this component. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After the migration, there are 2 action menu selectors. maybe ask cursor to tell you what branches its going to pull this information from before re-running this portion of the prompt. |
||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Passthroughs</summary> | ||
|
|
||
| None found for this component. | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Modifiers</summary> | ||
|
|
||
| None found for this component. | ||
|
|
||
| </details> | ||
|
|
||
| ### SWC | ||
|
|
||
| <details> | ||
| <summary>Attributes</summary> | ||
|
|
||
| - `selects` - By default sp-action-menu does not manage a selection. If you'd like for a selection to be held by the sp-menu that it presents in its overlay, use selects="single" to activate this functionality | ||
| - `static-color` - white | ||
| - `static-color` - black | ||
| - `quiet` (inherited from PickerBase) | ||
| - `disabled` (inherited from PickerBase) | ||
| - `invalid` (inherited from PickerBase) | ||
| - `label` (inherited from PickerBase) | ||
| - `open` (inherited from PickerBase) | ||
| - `size` (inherited from SizedMixin) | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Slots</summary> | ||
|
|
||
| - Default slot - menu items to be listed in the Action Menu | ||
| - `icon` - The icon to use for the Action Menu | ||
| - `label` - The label to use for the Action Menu | ||
| - `label-only` - The label to use for the Action Menu (no icon space reserved) | ||
| - `tooltip` - Tooltip to be applied to the Action Button | ||
|
|
||
| </details> | ||
|
|
||
| ## Comparison | ||
|
|
||
| ### DOM structure changes | ||
|
|
||
| <details> | ||
| <summary>Spectrum Web Components:</summary> | ||
|
|
||
| ```html | ||
| <sp-action-menu> | ||
| <sp-action-button | ||
| aria-describedby="..." | ||
| ?quiet=${this.quiet} | ||
| ?selected=${this.open} | ||
| static-color=${ifDefined(this.staticColor)} | ||
| aria-haspopup="true" | ||
| aria-controls=${ifDefined(this.open ? 'menu' : undefined)} | ||
| aria-expanded=${this.open ? 'true' : 'false'} | ||
| aria-label=${ifDefined(this.label || undefined)} | ||
| id="button" | ||
| class="button" | ||
| size=${this.size} | ||
| ?disabled=${this.disabled} | ||
| > | ||
| <slot name="icon" slot="icon" ?icon-only=${!this.hasLabel} ?hidden=${this.labelOnly}> | ||
| <sp-icon-more class="icon" size=${this.size}></sp-icon-more> | ||
| </slot> | ||
| <slot name="label" ?hidden=${!this.hasLabel}></slot> | ||
| <slot name="label-only"></slot> | ||
| </sp-action-button> | ||
| <slot name="tooltip"></slot> | ||
| <!-- renderMenu --> | ||
| <!-- renderDescriptionSlot --> | ||
| </sp-action-menu> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Legacy (CSS main branch):</summary> | ||
|
|
||
| ```html | ||
| <!-- Action Menu is a composite pattern in CSS --> | ||
| <div class="spectrum-Popover" data-testid="actionmenu" id="actionmenu-..."> | ||
| <button | ||
| class="spectrum-ActionButton spectrum-ActionButton--size..." | ||
| aria-haspopup="menu" | ||
| id="actionmenu-trigger-..." | ||
| > | ||
| <svg class="spectrum-Icon spectrum-Icon--size... spectrum-ActionButton-icon"> | ||
| <!-- More icon --> | ||
| </svg> | ||
| </button> | ||
| <ul class="spectrum-Menu spectrum-Menu--size..."> | ||
| <!-- Menu items --> | ||
| </ul> | ||
| </div> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Spectrum 2 (CSS spectrum-two branch):</summary> | ||
|
|
||
| ```html | ||
| <!-- Action Menu is a composite pattern in CSS --> | ||
| <div class="spectrum-Popover" data-testid="actionmenu" id="actionmenu-..."> | ||
| <button | ||
| class="spectrum-ActionButton spectrum-ActionButton--size..." | ||
| aria-haspopup="menu" | ||
| id="actionmenu-trigger-..." | ||
| > | ||
| <svg class="spectrum-Icon spectrum-Icon--size... spectrum-ActionButton-icon"> | ||
| <!-- More icon --> | ||
| </svg> | ||
| </button> | ||
| <ul class="spectrum-Menu spectrum-Menu--size..."> | ||
| <!-- Menu items --> | ||
| </ul> | ||
| </div> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this pattern better too, but I checked the prompt and we're still telling it to omit the section entirely if there's no diff. I would argue that updating the prompt to be more like this avatar example would be better? FWIW, Avatar, Badge, Help text, Opacity checkerboard, Swatch group, Swatch, and Thumbnail all use this "No significant structural changes" pattern, so it is pretty common in spite of the fact that the prompt doesn't have it. It might be worth fiddling around in Cursor to see if we can get it to look at the existing docs then update the prompt based on those patterns, but if we go that route, we definitely should have a human read through the prompt changes and make sure they make sense. |
||
| ### CSS => SWC mapping | ||
|
|
||
| Action Menu does not have dedicated CSS selectors. It is implemented as a composition of: | ||
| - Action Button (trigger) | ||
| - Popover (container) | ||
| - Menu (content) | ||
|
|
||
| | CSS selector | Attribute or slot | Status | | ||
| |-------------|------------------|--------| | ||
| | N/A | `selects` | Missing from CSS | | ||
| | N/A | `static-color` | Missing from CSS | | ||
| | N/A | `quiet` | Missing from CSS | | ||
| | N/A | `disabled` | Missing from CSS | | ||
| | N/A | `label` | Missing from CSS | | ||
| | N/A | `open` | Missing from CSS | | ||
| | N/A | `size` | Missing from CSS | | ||
| | N/A | `icon` slot | Missing from CSS | | ||
| | N/A | `label` slot | Missing from CSS | | ||
| | N/A | `label-only` slot | Missing from CSS | | ||
| | N/A | `tooltip` slot | Missing from CSS | | ||
| | N/A | Default slot (menu items) | Missing from CSS | | ||
|
|
||
| ## Summary of changes | ||
|
|
||
| ### CSS => SWC implementation gaps | ||
|
|
||
| Action Menu is a composite pattern, not a standalone component in CSS. In Spectrum CSS, Action Menu is implemented by combining: | ||
| 1. An Action Button as the trigger | ||
| 2. A Popover as the container | ||
| 3. A Menu for the content | ||
|
|
||
| In Spectrum Web Components, Action Menu is a dedicated component (`<sp-action-menu>`) that manages these three elements internally. This provides a simpler, more cohesive API for developers. | ||
|
|
||
| All WC-specific attributes and slots listed as "Missing from CSS" are implementation details of the web component abstraction and don't have direct CSS equivalents since CSS implements this as separate components. | ||
|
|
||
| ### CSS Spectrum 2 changes | ||
|
|
||
| No changes between CSS main and spectrum-two branches for Action Menu template structure. Both branches implement Action Menu identically as a composite pattern using Action Button, Popover, and Menu. | ||
|
|
||
| ## Resources | ||
|
|
||
| - [CSS migration]() | ||
| - [Spectrum 2 preview]() | ||
| - [React]() | ||
|
Comment on lines
+182
to
+184
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you find the links to all of these and add them please? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # Menu Divider migration roadmap | ||
|
|
||
| ## Component specifications | ||
|
|
||
| ### CSS | ||
|
|
||
| <details> | ||
| <summary>CSS selectors</summary> | ||
|
|
||
| - `.spectrum-Menu .spectrum-Menu-divider` | ||
| - `.spectrum-Menu .spectrum-Menu-divider.spectrum-Divider--sizeS` | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Passthroughs</summary> | ||
|
|
||
| None found for this component. | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Modifiers</summary> | ||
|
|
||
| - `--mod-menu-section-divider-margin-block` | ||
|
|
||
| </details> | ||
|
|
||
| ### SWC | ||
|
|
||
| <details> | ||
| <summary>Attributes</summary> | ||
|
|
||
| - `size` (string: 's', 'm', 'l') - inherited from SizedMixin | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Slots</summary> | ||
|
|
||
| None found for this component. | ||
|
|
||
| </details> | ||
|
|
||
| ## Comparison | ||
|
|
||
| ### DOM Structure changes | ||
|
|
||
| <details> | ||
| <summary>Spectrum Web Components:</summary> | ||
|
|
||
| ```html | ||
| <sp-menu-divider role="separator" size="m"></sp-menu-divider> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Legacy (CSS main branch):</summary> | ||
|
|
||
| ```html | ||
| <li class="spectrum-Divider spectrum-Divider--sizeS spectrum-Menu-divider"></li> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| <details> | ||
| <summary>Spectrum 2 (CSS spectrum-two branch):</summary> | ||
|
|
||
| ```html | ||
| <li class="spectrum-Divider spectrum-Divider--sizeS spectrum-Menu-divider"></li> | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| ### CSS => SWC mapping | ||
|
|
||
| | CSS selector | Attribute or slot | Status | | ||
| | ----------------------------------------- | -------------------------------- | ----------- | | ||
| | `.spectrum-Menu-divider` | Menu divider element | Implemented | | ||
| | `.spectrum-Divider--sizeS` | `size="s"` | Implemented | | ||
| | `.spectrum-Divider--sizeM` | `size="m"` | Implemented | | ||
| | `.spectrum-Divider--sizeL` | `size="l"` | Implemented | | ||
| | `--mod-menu-section-divider-margin-block` | CSS modifier for divider spacing | Implemented | | ||
|
|
||
| ## Summary of changes | ||
|
|
||
| ### CSS => SWC implementation gaps | ||
|
|
||
| No implementation gaps found. The menu divider component has complete feature parity between CSS and Web Components implementations. All size variants are supported, and the component properly implements the separator role for accessibility. | ||
|
|
||
| ### CSS Spectrum 2 changes | ||
|
|
||
| The CSS template files are identical between the `main` and `spectrum-two` branches. No structural changes were detected in the DOM between legacy and Spectrum 2 implementations for menu dividers. Both branches use the same HTML structure with the divider rendered as a `<li>` element with appropriate CSS classes. | ||
|
|
||
| ## Resources | ||
|
|
||
| - [CSS migration]() | ||
| - [Spectrum 2 preview]() | ||
| - [React]() |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Action menu got migrated in the
spectrum-twobranch in October, I think? That migration does add a really minimal CSS file that we'll probably want to mention.