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

[fix][menu-item]: Emit current state when checked change event fires #33029

Merged

Conversation

eljefe223
Copy link
Contributor

Previous Behavior

change custom event fires when menu-item becomes checked and unchecked. When role is set to menuitemradio and another menu-item is checked it's difficult to decern between the menu-item being checked and the menu-item being unchecked.

New Behavior

This change emits the current state of the checked property so a developer can properly listen for the item being checked.

Related Issue(s)

  • Fixes #

@eljefe223 eljefe223 requested a review from a team as a code owner October 11, 2024 17:00
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 11, 2024

📊 Bundle size report

✅ No changes found

@eljefe223 eljefe223 force-pushed the users/jes/menu-item-checked-change-event branch from 3558540 to dbe7356 Compare October 11, 2024 17:14
@radium-v radium-v changed the title [fix][menu-item]: Emit current state when checked chnage event fires [fix][menu-item]: Emit current state when checked change event fires Oct 11, 2024
@eljefe223 eljefe223 force-pushed the users/jes/menu-item-checked-change-event branch from dbe7356 to 9df0287 Compare October 14, 2024 15:32
@eljefe223 eljefe223 enabled auto-merge (squash) October 14, 2024 17:57
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 14, 2024

🕵 fluentui-web-components-v3 No visual regressions between this PR and main

@eljefe223 eljefe223 force-pushed the users/jes/menu-item-checked-change-event branch from 3a69f68 to fcb800f Compare October 14, 2024 19:12
@eljefe223 eljefe223 merged commit bc620a1 into microsoft:master Oct 14, 2024
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants