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

DropdownMenuV2: remove flashing styles when moving focus with keyboard #64873

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Aug 28, 2024

What?

Part of #50459

Removing "flash of unwanted styles" when navigating DropdownMenuV2 items using the keyboard.

Why?

While working on DropdownMenuV2, I noticed that menu items would sometimes flash briefly with the "active (hover)" styles, before applying the expected "focus-visible" styles — ie. it seems that the underlying ariakit implementation doesn't apply the data-focus-visible attribute soon enough, and therefore the focus-visible styles are applied with a delay, causing the hover (active) styles to apply for a few frames.

How?

I removed the flash by using the native :focus-visible CSS selector, which matches without any delays.

Testing Instructions

In Storybook, interact with the DropdownMenuV2 examples using keyboard, moving across the menu items.

On trunk, you could sometimes see the aforementioned flash — with the changes from this PR, the flash should never happen.

Screenshots or screencast

Before (trunk) After (this PR)
ddmv2.flash.before.mp4
ddmv2.flash.after.mp4

@ciampo ciampo requested a review from ajitbohra as a code owner August 28, 2024 14:33
@ciampo ciampo requested review from diegohaz and a team August 28, 2024 14:33
Copy link

github-actions bot commented Aug 28, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: diegohaz <hazdiego@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ciampo
Copy link
Contributor Author

ciampo commented Aug 28, 2024

cc @diegohaz — do you have any insights on why this is happening, and if there is a better fix?

@ciampo ciampo self-assigned this Aug 28, 2024
@ciampo ciampo added [Package] Components /packages/components [Type] Bug An existing feature does not function as intended labels Aug 28, 2024
@ciampo ciampo mentioned this pull request Aug 28, 2024
53 tasks
@diegohaz
Copy link
Member

cc @diegohaz — do you have any insights on why this is happening, and if there is a better fix?

Yes, data-focus-visible is applied using requestAnimationFrame, but the delay is likely caused by React batching state updates rather than rAF. Could you please open an issue on https://github.com/ariakit/ariakit?

@ciampo
Copy link
Contributor Author

ciampo commented Aug 28, 2024

Could you please open an issue on ariakit/ariakit?

Sure — here it is: ariakit/ariakit#4083

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Good one, @ciampo 👍 🚀

I'm assuming this will be useful regardless of a potential Ariakit fix. Should we add a note to check if we can clean it up in the future?

@ciampo ciampo force-pushed the fix/dropdown-menu-item-item-styles-flash branch from af10bc2 to e03508b Compare August 28, 2024 18:45
@ciampo ciampo enabled auto-merge (squash) August 28, 2024 18:45
@ciampo ciampo merged commit 8325439 into trunk Aug 28, 2024
61 checks passed
@ciampo ciampo deleted the fix/dropdown-menu-item-item-styles-flash branch August 28, 2024 21:44
@github-actions github-actions bot added this to the Gutenberg 19.2 milestone Aug 28, 2024
bph pushed a commit to bph/gutenberg that referenced this pull request Aug 31, 2024
WordPress#64873)

* DropdownMenuV2: remove flashing styles when moving focus with keyboard

* CHANGELOG

* Add comment about ariakit issue

---

Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: diegohaz <hazdiego@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants