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

Focus-visible support breaking in newer versions of the polyfill #2591

Closed
Pogodaanton opened this issue Jan 15, 2020 · 4 comments
Closed

Focus-visible support breaking in newer versions of the polyfill #2591

Pogodaanton opened this issue Jan 15, 2020 · 4 comments

Comments

@Pogodaanton
Copy link

Versions following the implementation of a Shadow-DOM tracker don't work with FAST's focus-visible JSS, as the js-focus-visible class is now placed into the <html/> tag rather than the <body/> tag.

What are the steps to reproduce the issue?

  1. Setup a react instance with @microsoft/fast-components-react-msft^4.28.4and focus-visible^5.0.2
  2. Include a component that can be focused, but should not be highlighted when using a mouse (e.g.: Button, Hyperlink, etc.)
  3. Click the added component in the browser
  4. The component will incorrectly reveal the :focus styling

What behavior did you expect?
Selected component does not appear to be in focus while using a mouse.

Temporary solution
Downgrading to an older version of the said package: focus-visible^4.1.5

@triage-new-issues triage-new-issues bot added the status:triage New Issue - needs triage label Jan 15, 2020
@chrisdholt
Copy link
Member

Thanks for the issue @Pogodaanton - I think there's an issue of being between implementations for this to get fixed. I do think we'll want to be on the latest version, but that will require a dependency change that would impact folks already on v4 of the polyfill. One quick fix is to document that we require ^v4.

Alternatively, we could make the change in the dependency and update our code - but that would potentially break folks with a lower version.

@nicholasrice for thoughts as well.

@chrisdholt chrisdholt added the bug A bug label Jan 15, 2020
@triage-new-issues triage-new-issues bot removed the status:triage New Issue - needs triage label Jan 15, 2020
@nicholasrice
Copy link
Contributor

I think it would be relatively trivial to select for both html and body for the .js-focus-visible class, enabling support for both v4 and v5.

@stale
Copy link

stale bot commented Apr 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Apr 17, 2020
@EisenbergEffect EisenbergEffect added area:react and removed complexity : low bug A bug warning:stale No recent activity within a reasonable amount of time labels Jul 17, 2020
@chrisdholt
Copy link
Member

closed with #3517

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

4 participants