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

[Bug]: Hover, Pressed states in Menu items do not show filled icons #29950

Closed
2 tasks
stchoioutlook opened this issue Nov 29, 2023 · 1 comment · Fixed by #29951
Closed
2 tasks

[Bug]: Hover, Pressed states in Menu items do not show filled icons #29950

stchoioutlook opened this issue Nov 29, 2023 · 1 comment · Fixed by #29951

Comments

@stchoioutlook
Copy link

stchoioutlook commented Nov 29, 2023

Library

React Components / v9 (@fluentui/react-components)

System Info

N/A

Are you reporting Accessibility issue?

None

Reproduction

https://react.fluentui.dev/?path=/docs/components-menu-menu--default

Bug Description

Actual Behavior

The menu item on hover and pressed states do not change the icon to the filled version. It also looks like the wrong theme colored token is being used. Below is a screenshot of the current implementation.
MicrosoftTeams-image (10)

Expected Behavior

This is the intended behavior as shown in Fluent specs for Menu items. The icon colors are also CompoundBrandForeground1/Hover on Hover and CompoundBrandForeground1/Pressed on Pressed
MicrosoftTeams-image (11)

Logs

No response

Requested priority

Blocking

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@khmakoto
Copy link
Member

Hi @stchoioutlook, the icons do switch between the regular and filled versions as long as you use the bundleIcon utility that is shown in our Menu stories, for example this one.

I agree that we should add the correct color for the pressed state though, I'll make a PR to fix that.

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

Successfully merging a pull request may close this issue.

2 participants