fix: make seek-to-live accessible#5651
Conversation
gkatsev
left a comment
gkatsev
left a comment
There was a problem hiding this comment.
I think this is a great start to making it more accessible. @OwenEdwards would love your input.
Also, given our usage of the control text, probably not an issue, but we may want to take #5653 into account here as well.
| if (this.player_.liveTracker.behindLiveEdge()) { | ||
| this.setAttribute('aria-disabled', false); | ||
| this.removeClass('vjs-at-live-edge'); | ||
| this.controlText('Seek To LIVE'); |
There was a problem hiding this comment.
I wonder if this should be more verbose like not at live edge, click to seek to live.
There was a problem hiding this comment.
I am going to change it to that, as being verbose is probably better.
There was a problem hiding this comment.
Is there a display somewhere in the control bar which shows how far behind live you are? I'm wondering if that ought to also be indicated here, e.g. "Seek to Live, currently 1:23 behind live". I think you should stay away from the term "edge" - that's a technical term, but not one a typical user would recognize.
There was a problem hiding this comment.
the current time shows up as how far behind live you are, I think. We can probably use that to update this.
There was a problem hiding this comment.
Also, try to avoid "click to..." - that's implied with a button, but some users may tap, some click, some use the keyboard. And try to front-load the action, then add more information afterwards - "Seek to Live, currently 1:23 behind live" instead of "not currently live, seek to live".
There was a problem hiding this comment.
Ah, makes sense for the action to be front loaded, I was thinking that maybe the info is more important but it isn't
| this.controlText('Seek To LIVE'); | ||
| } else { | ||
| this.addClass('vjs-at-live-edge'); | ||
| this.controlText('At LIVE edge'); |
There was a problem hiding this comment.
"Playing Live", perhaps? As I mentioned, I don't think "edge" is meaningful outside the technical world.
There was a problem hiding this comment.
Or "Seek to Live, currently playing Live", which would then be announced as "button, disabled".
| } else { | ||
| this.addClass('vjs-at-live-edge'); | ||
| this.controlText('Seek to live, currently playing live'); | ||
| this.setAttribute('aria-disabled', true); |
There was a problem hiding this comment.
do we want to use this.disable() and this.enable() here instead? https://github.com/videojs/video.js/blob/master/src/js/clickable-component.js#L147-L177 and
https://github.com/videojs/video.js/blob/master/src/js/button.js#L79-L95
There was a problem hiding this comment.
I don't know if we want the button to dim visually, which will happen if we do that. I think we just want the button to "dim" for screen readers, normal users will see a cursor change and will already be at live with a red circle.
brandonocasey
force-pushed
the
fix/seek-to-live-a11y
branch
from
20ed451 to
1bf033f
Compare
brandonocasey
force-pushed
the
fix/seek-to-live-a11y
branch
from
a088c25 to
72d5bce
Compare
|
Just tried it out with VO, seems to be working well. @OwenEdwards do you have any other input or is this good to go? |
3 participants
Makes seek to live accessible and indicates when the button can be pressed or not to the screen reader.