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

[v4] MenuItem active + hover state styling looks off #4985

Closed
adidahiya opened this issue Oct 26, 2021 · 3 comments
Closed

[v4] MenuItem active + hover state styling looks off #4985

adidahiya opened this issue Oct 26, 2021 · 3 comments

Comments

@adidahiya
Copy link
Contributor

Environment

  • Package version(s): core v4.0.0-beta.5, select v4.0.0-beta.5
  • Operating System: macOS
  • Browser name and version: Chrome

Steps to reproduce

  1. Visit v4 select docs
  2. Select an item in dropdown
  3. Open the dropdown again, hover through items

Actual behavior

The active MenuItem goes from dark bg -> light bg, kind of an unexpected interaction:

2021-10-26 11 08 29

Expected behavior

??

Possible solution

Use a slightly lighter or darker bg, but don't invert the colors when hovering over an active MenuItem

@aycai
Copy link
Contributor

aycai commented Oct 26, 2021

Sorry, scratch my previous comment - for the select component specifically, using the new hover state for the selected state (the light background with blue text) is the intended static state, and it will still have an :active state that matches the primary intent - this allows multi-select menus to retain feedback when clicking on selected menu items.
image

@adidahiya
Copy link
Contributor Author

Ok, cool, the design proposed in your second comment makes more sense. I'll go head and implement that.

We have some unfortunate prop naming at the moment; <MenuItem active={true} /> actually represents the "selected" appearance. I can also go ahead and add selected?: boolean as a replacement and deprecate active?: boolean (although it won't fully go away until Blueprint 5.x; I don't want to add in additional breaking changes to v4.0 last minute).

@adidahiya
Copy link
Contributor Author

There are some further complications -- currently, we apply the primary intent class to the MenuItem when active={true} and intent is undefined... I think we should stop doing that and instead set up the styles differently to achieve the design in your screenshot.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants