Skip to content
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

feat(menu): migrate to spectrum-tokens + add new features #1942

Merged
merged 41 commits into from
Aug 4, 2023

Conversation

pfulton
Copy link
Collaborator

@pfulton pfulton commented Jun 15, 2023

Description

  • Migrates the menu component to use core tokens.
  • Adds new features / variants.
  • Adds new stories and examples to cover new variants and some edge cases.

How and where has this been tested?

  • How this was tested:
  • Browser(s) and OS(s) this was tested with:

Screenshots

To-do list

  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have tested these changes in Windows High Contrast mode.
  • I have updated any relevant storybook stories and templates.
  • If my change(s) include visual change(s), a designer has reviewed and approved those changes.
  • This pull request is ready to merge.

BREAKING CHANGE: migrates the Menu to `@adobe/spectrum-tokens`
@pfulton pfulton added released-beta Indicates a beta release has been rolled for this PR do not merge A flag for a branch indicating it should not be merged. labels Jun 15, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 15, 2023

🚀 Deployed on https://pr-1942--spectrum-css.netlify.app

@github-actions github-actions bot temporarily deployed to pull request June 15, 2023 21:12 Inactive
jawinn added 19 commits June 20, 2023 10:29
Make .spectrum-Menu--sizeM the default; move its custom properties to
the parent class, and move sizing styles underneath the parent class
custom property definitions.
Add color declaration wherever there is 'fill', per SWC request. Because
"using fill to set the colour of icons in the menu css isn't
compatible with the way we process our icons, so the icons aren't
colouring properly or changing depending on the state".
Selected menu items with checkmark were shifting 2 pixels as compared
to the non-selected menu items. Looking over the updated design, the
text-to-control-* tokens are used for the space between the checkmark
and the text.
When a section heading was above menu items, it previously looked
strange because of the lack of space. The sectionHeading also needs
top and bottom padding.
Replace unnecessary CJK style rules with a change to the relevant custom
property values.
- Simplify high contrast mode styles. Less custom properties are needed.
- Fix for hover causing things to disappear on collapsible items. Fix
  may need to be replaced later with one that addresses the regular
  styles for children of menu-item being applied to the nested menu
  items in the collapsible variant.
- Remove skin.css as part of tokens migration. Its rules should already
  be handled now by the index.css.
Updates the Menu stories to account for additional variants on the docs
and changes to markup. Adds several new stories.
- Use state class naming instead of modifier class naming for
  is-selected and is-selectable.
- Show focus indicator line only with :focus-visible for keyboard focus
- Simplify RTL/LTR change of focus indicator with scalar custom prop
Add story for drill-in variant. Also makes isSelectable false by default
and change some stories to set it to false to better align with docs
examples.
In the collapsible variant, style rules applying to children of a menu
item were also applying to the nested menu child items. This was causing
some issues with high contrast hovers and could also be noticed by
changing a mod like --mod-menu-item-label-content-color-hover and then
hovering over the parent menu item in the collapsible variant (this
would also change the color of all the child menu items).
The displayed submenu for the drill-in example was not how submenus
should be displayed per the guidelines. They need to be positioned, and
are shown as being within another popover. Showing this adjacent menu
like this could cause some confusion as to its usage; the adjacent menu
was not positioned properly and does not have any separate styles within
the CSS for doing so.
For the collapsible variant:
The child menu items under a parent menu item that contains a workflow
icon should not show extra indentation, otherwise it looks like a
different tier when next to a menu item without an icon. Confirmed
with design team.

All sub-items are now indented to after the chevron and the start of the
parent item text/heading.
Add Collapsible story to menu in Storybook, based on example from docs
example.
Add control for t-shirt sizing to menu stories. Adds the size class to
various elements.
Fixes for several issues with the menu that is displayed in the docs
site search results, and theme/scale/direction popovers.

- Makes the adjacentText classes the default margin, allowing them to
  be removed (which fixes checkbox spacing in theme/scale popovers).
- Fixes extra top and bottom margin appearing in menu for docs
  theme/scale popovers. This was showing user agent values for top and
  bottom margin. In production, they were previously set to a popover
  padding token, which added more space than on the design (popover
  component already has padding).
- Fixes search results menu showing incorrectly because of difference in
  the JS created markup related to section headers.
@github-actions github-actions bot temporarily deployed to pull request July 11, 2023 16:45 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 19, 2023 19:15 Inactive
@pfulton
Copy link
Collaborator Author

pfulton commented Jul 19, 2023

New beta published: @spectrum-css/menu@5.0.0-beta.5

@github-actions github-actions bot temporarily deployed to pull request July 19, 2023 19:51 Inactive
jawinn and others added 2 commits August 1, 2023 16:01
Zero out the margin-inline-end for the chevron used at the end
of the drill-in menu items.
@pfulton
Copy link
Collaborator Author

pfulton commented Aug 1, 2023

New beta published: @spectrum-css/menu@5.0.0-beta.6

@github-actions github-actions bot temporarily deployed to pull request August 2, 2023 14:40 Inactive
@pfulton pfulton marked this pull request as ready for review August 2, 2023 14:44
Update divider margin-block to agree with newly added token for the
divider height. Sets minimum tokens package version to 11.0.2 where this
--spectrum-menu-item-section-divider-height token was added.
@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 14:08 Inactive
@jawinn jawinn added the run_vrt For use on PRs looking to kick off VRT label Aug 3, 2023
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 3, 2023
Replace medium divider with small divider in examples and storybook
markup. Updates migration guide to note the change. Removes unnecessary
large divider possibility from CSS, as that should never be used here
according to divider guidelines.
@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 19:55 Inactive
Fix migration guide section no appearing in docs because it was named
'section' and not 'sections' in the YML.

Adds new standard section about -mod custom properties.
@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 20:03 Inactive
@pfulton
Copy link
Collaborator Author

pfulton commented Aug 4, 2023

New beta published: @spectrum-css/menu@5.0.0-beta.7

@github-actions github-actions bot temporarily deployed to pull request August 4, 2023 13:57 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 4, 2023 20:40 Inactive
@pfulton pfulton added run_vrt For use on PRs looking to kick off VRT and removed do not merge A flag for a branch indicating it should not be merged. labels Aug 4, 2023
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 4, 2023
@pfulton pfulton merged commit d961abd into main Aug 4, 2023
8 checks passed
@pfulton pfulton deleted the pfulton/migrate-menu branch August 4, 2023 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released-beta Indicates a beta release has been rolled for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants