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

[Select] - First options receives .Mui-focusVisible always #23747

Open
2 tasks done
orest22 opened this issue Nov 27, 2020 · 18 comments
Open
2 tasks done

[Select] - First options receives .Mui-focusVisible always #23747

orest22 opened this issue Nov 27, 2020 · 18 comments
Labels
bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module!

Comments

@orest22
Copy link

orest22 commented Nov 27, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

There is some issue with :focus-visible for Select. For the first time when there is no value selected .Mui-focusVisible class is getting applied to the first item in a list. This is not the case for the Menu component though. Also, this happens only in Chrome.

Expected Behavior 🤔

Same behaviour as the Menu component has.

Steps to Reproduce 🕹

Two components side by side:https://codesandbox.io/s/material-ui-issue-forked-udm04?file=/src/Demo.js

Steps:

Case 1:

  1. Open Select via mouse click
  2. The first item is highlighted with a red border meaning the focusVisible class was added

Case 2:

  1. Refresh a page
  2. Open Menu via mouse click everything works as expected

Context 🔦

Want have focus applied on Select only when it was focused with a keyboard. The options should also stay without focus if the keyboard wasn't used

Your Environment 🌎

Tech Version
Material-UI v5.0.0.alpha.17
React 17
Browser Chrome
TypeScript
etc.
@orest22 orest22 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 27, 2020
@orest22
Copy link
Author

orest22 commented Nov 27, 2020

Screen Recording 2020-11-27 at 04 07 06 PM

@oliviertassinari
Copy link
Member

@eps1lon
Copy link
Member

eps1lon commented Nov 28, 2020

Thanks for the detailed report.

Definitely needs some investigation how this happens. Only then can we know whether this is a chrome bug or Material-UI bug.

@eps1lon eps1lon added bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 28, 2020
@orest22
Copy link
Author

orest22 commented Dec 3, 2020

@eps1lon could it be related to a question you have asked here https://bugs.chromium.org/p/chromium/issues/detail?id=1127875 ?

@fxlemire
Copy link
Contributor

This bug happens for me on a clean install on latest MUI version, both on chrome and firefox. Here is a sandbox.

@CarlosAmaral
Copy link

This is still a problem btw. Anyway to fix this?

@mraballard
Copy link

I'm currently experiencing this on Chrome using MenuUnstyled and MenuItemUnstyled.

@stuthib
Copy link

stuthib commented Nov 23, 2022

Same here. I have a search input in my MenuUnstyled component and auto-focus on that is getting overridden and comes to the first element in the list. Anyway to fix this ?

@hiroki0404508
Copy link

I also encountered the same event.
I think the environment is the same as @orest22.
Are there any plans to fix this?

@SaidMarar
Copy link
Contributor

SaidMarar commented Jan 30, 2023

Well this issue is not related to the MUI.
It is something related to :focus-visible that matches on initial programmatic focus w3c :focus-visible
And i think it is more related to the heurestic spec rule number 4 check the comment

  1. If the user interacts with the page via a pointing device, such that the focus is moved to a new element which does not support user input, the newly focused element should not match :focus-visible.

@orest22 i think you can reproduce the same thing for the menu with two possible implementations of Anchor that opens the menu:

  1. Button with mousedown and preventdefault this is the same implementation on Select in this PR to solve PERF issue.
  2. Change the Button to Div (click / mousedown)

Same behavior with vanilla js in this example

@oliviertassinari possible fix is to remove this line useIsFocusVisible the focus-visible should be applied only if the user uses keyboard no need to check focus-visible matches.
For more details this article https://www.deque.com/blog/give-site-focus-tips-designing-usable-focus-indicators/ in MDN

@allicanseenow
Copy link

Has there been any progress for this?

@qramit
Copy link

qramit commented May 24, 2023

I am also experiencing same!!

@harinair
Copy link

This issue is still there - I understand that according to spec, the first item gets the focus automatically... but so far MUI was good at applying the focusVisible class on focused elements only when user used keyboard. So the fix is to remove focusVisible class if the user interacted using mouse. Interestingly this issue is not there if user navigated from another screen in single page app.

@chirag-chan-dream11
Copy link

This issue can be tackled by using a MenuItem Component (as 1st Option) and setting it's display property to "none".

@davidko5
Copy link

davidko5 commented May 1, 2024

still the issue, but this workaround helped

This issue can be tackled by using a MenuItem Component (as 1st Option) and setting it's display property to "none".

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 14, 2024

Today I leaned about this option: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#focusvisible / https://html.spec.whatwg.org/multipage/interaction.html#focus-management-apis:

.focus({ focusVisible: false });

that we could maybe used to solve this issue and dodge w3c/csswg-drafts#5885. Now, I imagine that we can't systematically use this, first the false option isn't supported, second, we would need to bring #42467 back or actually, simpler, just look at the event type that triggered the programmatic focus cc @DiegoAndai.

@DiegoAndai
Copy link
Member

IMO the path forward for focus visible handling in Material UI should rely on :focus-visible. That would mean deprecating and eventually removing the .Mui-focusVisible classes, as well as our usage of them for styling, in favor of :focus-visible. This aligns better with current web development practices and technologies. What do you think @aarongarciah? If you agree, I'll create an issue to track this work.

@aarongarciah
Copy link
Member

Agree with @DiegoAndai. If possible, we should try to simplify and move work to CSS. I don't have the right context to know if :focus-visible is sufficient in our case or if we need a more advanced JS-centric solution (we're probably don't need something as advanced as React Aria https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/useFocusVisible.ts)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests