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

Prevent buttons from being focused on mouse click #5474

Closed
gjanblaszczyk opened this issue Oct 3, 2018 · 8 comments · May be fixed by CleverTap/vue-video-player#3
Closed

Prevent buttons from being focused on mouse click #5474

gjanblaszczyk opened this issue Oct 3, 2018 · 8 comments · May be fixed by CleverTap/vue-video-player#3
Labels
a11y This item might affect the accessibility of the player

Comments

@gjanblaszczyk
Copy link
Member

gjanblaszczyk commented Oct 3, 2018

Description

Every time I clicked on the control bar buttons. I saw that the clicked button was focused.
However, this behavior is fine but sometimes I want to have a possibility to disable this.
The outline should be visible when the user uses the keyboard keys to navigate through control bar buttons.
Can we add the new videojs option to enable/disable the control buttons from being focused on mouse click?
I want to have similar behavior like youtube player has now.
videojs-play-outline

Steps to reproduce

  1. Use mouse device to click on the control bar button for example: play or fullscreen.
  2. The clicked button will be focused and you will see the outline around it.

Results

Expected

The button is focused after mouse click but I don't see the visual outline on it (which is good). The button outline should be shown when the user used the keyboard 'tab' key to navigate through control bar controls.

Actual

The button is focused and the outline is shown after the mouse click.

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

lates

browsers

any

OSes

any

@gkatsev
Copy link
Member

gkatsev commented Oct 3, 2018

Yeah, this is something we've been thinking about a lot. Specifically, there hasn't been a good way to hide it for mouse users but keep it for keyboard users. Starting with v6, we opted that it's better to have the outlines for mouse users to make keyboard and screen reader user's experience better.

It's possible that we should just add something like the following, then when browsers implement it it'll go away or we can direct users to the focus-visible polyfill.

:focus:not(:focus-visible) { outline: none; }

@gkatsev gkatsev added the a11y This item might affect the accessibility of the player label Oct 3, 2018
@gjanblaszczyk
Copy link
Member Author

Thanks for the information and this focus-visible polyfill is a really good idea.
btw @gkatsev Do you know any other solution for this problem?

@gkatsev
Copy link
Member

gkatsev commented Oct 4, 2018

There isn't a good way, unfortunately. One way I think we may be able to do it is by storing whether the component was interacted with with the mouse or not. Then, in the focus event, we can preventDefault if it was the mouse. But that's less than ideal.

@OwenEdwards
Copy link
Member

OwenEdwards commented Oct 5, 2018

@gjanblaszczyk you can see a fully implemented version of the :focus-visible polyfill being used at:

OwenEdwards@47f25a2

It's a very small change (EDIT: and it's available right now as a polyfill. I just reread @gkatsev comment, and realized that it implies that the :focus-visible solution is something we would have to wait for), and :focus-visible is on track to be a W3C recommendation.

I'd strongly recommend against doing any other kind of hack, including the preventDefault in the focus event; it breaks accessibility of the page for low-vision users who use a combination of mouse and keyboard, and has the potential to break other forms of assistive technology such as screen readers and voice control software (e.g. Dragon).

@gjanblaszczyk
Copy link
Member Author

Thanks, @OwenEdwards for the explanation.
I pushed new PR based on: focus-visible and closed the old one.

@gjanblaszczyk
Copy link
Member Author

I have also some question about default styling for video.js focus ring. For now, the video.js player uses default focus browser styles which mean that focus ring looks differently on every browser. I guess It will be a good idea to define a default focus style that will look consistent on all browsers. What do you think?

@gkatsev
Copy link
Member

gkatsev commented Oct 9, 2018

Yeah, I've been wanting to make a unified focus ring (probably similar to what Chrome has) across browsers, partially because firefox's focus ring is not accessible.

@OwenEdwards
Copy link
Member

The problem with creating a custom visible focus ring which is consistent across all browsers is that doesn't match users' expectations that the focus ring will be consistent across all elements in a page in the browser they're using, rather than across all browsers.

Of course, if the page designer implements a custom focus ring, then that needs to override everything including the video.js one. But it gets complicated if their custom ring doesn't show up clearly with the video.js styling (either the default styling, or their custom styling).

So there are lots of factors at play here! But as @gkatsev mentioned, with v6 we decided to put more of a focus on accessibility than in the past, and that included showing the focus outline (see http://www.outlinenone.com/)

gkatsev pushed a commit that referenced this issue Oct 31, 2018
Add support for focus-visible so that mouse-users don't need to see focus outlines but keyboard and Screen Reader users still do. It includes both the standard selector and the selector intended to work with the polyfill: https://github.com/WICG/focus-visible.
The polyfill is *not* included in Video.js and must be included on the page separately.

Fixes #5474.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This item might affect the accessibility of the player
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants