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

There is no way to control the hover, focus, and focus-visible styles for DropdownMenuItem #1164

Closed
KelvinOm opened this issue Feb 18, 2022 · 21 comments

Comments

@KelvinOm
Copy link

Bug report

There is no possibility to set styles for hover, focus, and focus-visible states separately for DropdownMenuItem

Current Behavior

The focus is set automatically and because of this, even just when hovering, the elements are in focus.

Expected behavior

It is possible to apply different styles for different states
Example: https://www.w3.org/TR/wai-aria-practices-1.1/examples/menubar/menubar-1/menubar-1.html#

Reproducible example

Here is an attempt to apply different styles for DropdownMenuItem. Please take a look at lines 71-73
https://codesandbox.io/s/naughty-chebyshev-bhnv3f?file=/App.js

@benoitgrelard
Copy link
Collaborator

Hi @KelvinOm,

This is really by design.
Have a read through this similar issue where I responded with more details: #956 (comment)

@KelvinOm
Copy link
Author

Hi @benoitgrelard. Thank you so much for such a quick response.

I understand your arguments about one focused element and agree with them. But what about making different styles for mouse and keyboard. I mean different styles for :focus and :focus-visible. This doesn't contradict your arguments and gives flexibility in customization.

@benoitgrelard
Copy link
Collaborator

The problem with :focus-visible is that it relies on heuristics applied by the browser to "guess" the input modality.
In most browsers (if not all), a programatic focus will be dimmed "visible" thus, both mouse and keyboard will appear "visible".

@KelvinOm
Copy link
Author

I got you. Thanks! 👍

@KelvinOm
Copy link
Author

@benoitgrelard Could you please add this information to the docs? I believe it can save some time for consumers of Radix.

@KelvinOm
Copy link
Author

KelvinOm commented Feb 20, 2022

And one more question: Could you tell please why the same approach as in the menu is not used in Toolbar?

Screenshot 2022-02-20 at 23 11 41

@benoitgrelard
Copy link
Collaborator

And one more question: Could you tell please why the same approach as in the menu is not used in Toolbar?

Screenshot 2022-02-20 at 23 11 41

Because this is "mixed" pattern, ie. a composition of potentially lots of different controls put together so there isn't a "single" concept for "highlighted". This here really is focus moving around and it wouldn't make sense to move focus whilst hovering.

I do agree with you though that this could present similar issues depending on how you decide to style your hover states. Personally in a toolbar, I would probably forego the hover states alltogether to avoid the issue.

@KelvinOm
Copy link
Author

I understand your position.

But for me as a consumer, there is no way to make consistent behavior for the dropdown menu and for the toolbar. Only if I fork dropdown menu, which is completely wrong in my opinion.

I still think it's not very right not to give consumers the opportunity to be able to adjust the state of the hover in the dropdown menu. As a library, you can give recommendations on best practices, and from my perspective, this is the best approach. This will allow the library to be more flexible.

@benoitgrelard
Copy link
Collaborator

But for me as a consumer, there is no way to make consistent behavior for the dropdown menu and for the toolbar.

What do you mean by that? The DropdownMenu styling will be consistent whether inside or outside a toolbar no?

@KelvinOm
Copy link
Author

I mean, I don't have the opportunity to make such picture.
Screenshot 2022-02-21 at 15 48 32

In the DropdownMenu I can use either box-shadow or background but not both at once.

Imagine that we open the menu by clicking on the button in Toolbar. It turns out that we have one behavior in the Toolbar and completely different behavior in the DropdownMenu. I suppose from the point of UX, this is not very good.

@benoitgrelard
Copy link
Collaborator

I mean, I don't have the opportunity to make such picture. Screenshot 2022-02-21 at 15 48 32

In the DropdownMenu I can use either box-shadow or background but not both at once.

Imagine that we open the menu by clicking on the button in Toolbar. It turns out that we have one behavior in the Toolbar and completely different behavior in the DropdownMenu. I suppose from the point of UX, this is not very good.

But from the POV of the dropdown menu, it will be consistent overall though.
It's up to you then whether you want to use hover states on the toolbar items or not.

I believe I have already explained why it has to be this way for menus, so I don't think we can do anything else here really.

@KelvinOm
Copy link
Author

KelvinOm commented Feb 21, 2022

It's up to you then whether you want to use hover states on the toolbar items or not.

I can decide I want to use hover or not in Toolbar, but I can't do this in DropdownMenu because the library does not give me such an opportunity.

I don't think we can do anything else here really.

You could give consumers the ability to customize states.

Anyway, thanks a lot for the answers and for the excellent library! 👍

@benoitgrelard
Copy link
Collaborator

…but I can't do this in DropdownMenu because the library does not give me such an opportunity.

I guess the point I'm trying to make is that it's not really just us deciding we should do it this way. It's a well established UX/accessibility pattern that has been designed/used by many other people far more experienced than us.

As an example, here are native menus (dropdown and menubar) and native select in MacOS:

Screen.Recording.2022-02-21.at.11.33.31.mov
Screen.Recording.2022-02-21.at.11.34.05.mov
Screen.Recording.2022-02-21.at.11.34.44.mov

@KelvinOm
Copy link
Author

@benoitgrelard I understand all this and accept it. I'm just trying to say that if the library provides this opportunity then it would become a little more flexible. We cannot know all the cases of using the component and sometimes it may be necessary to do something custom.

@KelvinOm
Copy link
Author

Another case. If the DropdownMenu is open and I click on the address bar, the focus will stop working. But I can still hover over a menu item and select something. The hover in such a situation would work out fine.

Screen.Recording.2022-02-22.at.11.06.34.mov

@jjenzz
Copy link
Contributor

jjenzz commented Feb 22, 2022

@KelvinOm That will be fixed by the following proposed change #956 (comment)

@ivanbanov
Copy link

@jjenzz is it expected to have the proposed change being released soon?

@jjenzz
Copy link
Contributor

jjenzz commented Mar 15, 2022

@ivanbanov i'm not on the modulz team anymore i'm afraid so perhaps @benoitgrelard or @andy-hook can help?

@benoitgrelard
Copy link
Collaborator

I am closing this issue now as I have captured the request for the enhancement in a new issue: #1289

@zecka
Copy link

zecka commented Aug 14, 2023

If we need more flexibility about when use focus style, maybe we can use library such as what-input in addition of radix-ui.

@aditya22314
Copy link

@benoitgrelard Also there is no option to open the DropMenuContent while hovering it .In the docs dropdown works only when clicking the DropMenuTrigger Items .

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

No branches or pull requests

6 participants