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

DropdownMenu v2 style tweaks #50967

Merged
merged 3 commits into from
May 26, 2023
Merged

DropdownMenu v2 style tweaks #50967

merged 3 commits into from
May 26, 2023

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented May 25, 2023

What?

Fixes #50910

Tweak the design of the new DropdownMenu component following the feedback from #50910

Why?

We want our components to look good and consistent with the rest of the editor UI

How?

  • Changed separator to go full width
  • Changed hover / focus / open styles to have a light gray background

Testing Instructions

Screenshots or screencast

trunk This PR
Screenshot 2023-05-25 at 16 07 44 Screenshot 2023-05-25 at 16 18 33

@ciampo ciampo requested a review from ajitbohra as a code owner May 25, 2023 14:09
@ciampo
Copy link
Contributor Author

ciampo commented May 25, 2023

My personal notes:

  • currently hover, focus and open styles (ie. the style of a menu item when its corresponding submenu is open) are currently visually represented the same way
  • with the new styles, there's no usage in the components, of the main accent color (defaulting to blueberry)

@ciampo ciampo requested review from mirka, jameskoster and SaxonF May 25, 2023 14:11
@ciampo ciampo self-assigned this May 25, 2023
@ciampo ciampo added [Type] Enhancement A suggestion for improvement. [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components labels May 25, 2023
@ciampo ciampo force-pushed the feat/dropdown-v2-design-feedback branch from a72bc9c to cee5a4d Compare May 25, 2023 14:18
@github-actions
Copy link

Flaky tests detected in cee5a4d.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5081049790
📝 Reported issues:

Copy link
Contributor

@chad1008 chad1008 left a comment

Choose a reason for hiding this comment

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

Changes look good to me and the new styles are looking great in Storybook. I agree on the full-width separators, I think it looks a lot nicer that way, personally. I'm sure you'll want input from others who offered feedback initially but I say 🚀 🚢

@jameskoster
Copy link
Contributor

I will change the hover status to gray, although we'd need to also consider:

  • focus state
  • "expanded" state (ie. the state that a subtrigger is in, when its corresponding submenu is open)

If we're committed to the radix-style color scale there are guides for these details. But:

  • I don't know that we are committed?
  • The current colors do not map adequately (we only have 8 vs Radix' 12).

It probably doesn't need to hold up this PR, but it feels like something that we need to make a decision on sooner rather than later.

@ciampo
Copy link
Contributor Author

ciampo commented May 26, 2023

it feels like something that we need to make a decision on sooner rather than later.

Absolutely. IMO, the current color config that we have doesn't feel comprehensive and clear to apply. We often end up having to interpret it, fill in gaps and/or making exceptions because the color combinations don't quite work out.

This definitely feels like a bigger task than the DropdownMenu. We can probably merge the PR as-is if that works for you, and revisit the component once we have more clear guidelines ?

Also flagging #50971 which is partially related to this topic

@ciampo
Copy link
Contributor Author

ciampo commented May 26, 2023

As discussed in person with @jameskoster , I will go ahead and merge this PR, and later work on updating the styles of this component to match the current look&feel of the legacy DropdownMenu component.

@ciampo ciampo merged commit 22435b6 into trunk May 26, 2023
@ciampo ciampo deleted the feat/dropdown-v2-design-feedback branch May 26, 2023 13:19
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DropdownMenu: Design feedback
3 participants