Enhance Video Controls for Video Audio Description #6080

amandadupell · 2022-04-20T15:13:56Z

What does this PR do?

Adds the ability to pass in a video audio description for video controls.

Where should the reviewer start?

Video.js

What testing has been done on this PR?

Manual Storybook testing

How should this be manually tested?

Storybook

Do Jest tests follow these best practices?

screen is used for querying.
The correct query is used. (Refer to this list of queries)
userEvent is used in place of fireEvent.
asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6059

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Yes

Should this PR be mentioned in the release notes?

Yes

Is this change backwards compatible or is it a breaking change?

Compatible, new feature

taysea · 2022-04-20T17:51:59Z

src/js/languages/default.json

@@ -66,6 +67,9 @@
    "pauseButton": "pause",
    "playButton": "play",
    "volumeDown": "volume down",
-    "volumeUp": "volume up"
+    "volumeUp": "volume up",
+    "description": "video audio description",


Since we are referring to it as "Video description" below, should we just say "video description" here? OR you mentioned that "Audio description" is more common, should we be saying that across the board?

Leaning towards "Audio description" myself after doing some quick googling.

taysea · 2022-04-20T17:52:25Z

src/js/languages/default.json

@@ -66,6 +67,9 @@
    "pauseButton": "pause",
    "playButton": "play",
    "volumeDown": "volume down",
-    "volumeUp": "volume up"
+    "volumeUp": "volume up",
+    "description": "video audio description",


Should we call this key audioDescription to tie in with the accessibility language?

taysea · 2022-04-20T17:54:20Z

src/js/themes/base.js

@@ -1683,6 +1684,7 @@ export const generate = (baseSpacing = 24, scale = 6) => {
        play: Play,
        reduceVolume: VolumeLow,
        volume: Volume,
+        description: AssistListening,


It seems like there is a standard icon for Audio Description -- should we add that to grommet-icons to be able to use here?

yes this icon would be preferred. what is the timeline/process for getting this added to grommet icons?

I would create an issue for this on grommet-icons and add it to the backlog

taysea

From some research I was doing on where "Audio Descriptions" have already been successfully implemented (Netflix, among others), it seems like it's more of a supplemental audio track that is announced real time with the video as opposed to text on the screen like we are doing here.

For users who are deaf, closed captioning is helpful to show what is being said.
For users who are blind, audio descriptions are helpful to describe what is being shown.

The behavior seems somewhat standardized and so showing text on the screen like we are in this PR seems to deviate from that and we would want to instead support the option for someone to add an audio description track and for a user to be able to toggle that on/off in the controls. However, did any of this come up in your discussion w Bill and what he expects from an Audio Description?

amandadupell · 2022-05-04T18:13:38Z

for testing, i updated the Video/Controls Below story to include a <track kind="descriptions" /> for the video. this is current industry best practice, but has varied support across browsers. i've tried to find a chart that demonstrates the support but have just read and tested this one my own (not supported in VO). because of this, we are implementing our own system for announcing the active audio description for the video.

to test:

open Controls Below story
using keyboard, navigate to new "audio description" button in controls menu pop-up
play the video
audio descriptions will be announced when active

things to note:

announce is missing from the dependency array; this is to prevent re-announcing of active tracks when inBounds for a long time
we should update the audio description icon to align with other systems as taylor mentioned above
we should consider moving the icon outside of the menu pop-up and include it as a part of the bar next to the menu button; i think this aligns more with industry standards

… feat/video-controls-6059

jcfilben

Just a minor comment, otherwise looks good

jcfilben · 2022-05-09T22:54:13Z

src/js/languages/default.json

+    "showDescription": "Video description opened below video",
+    "hideDescription": "Video description closed"


Do we still need these?

… feat/video-controls-6059

jcfilben

Looks good!

amandadupell added 2 commits April 20, 2022 00:27

initial commit

d62bc7d

added video control for video description

1a16065

taysea reviewed Apr 20, 2022

View reviewed changes

added logic to announce active tracks when choosing audio desc

188d600

amandadupell added 6 commits May 4, 2022 14:17

small change to key name value

a37614c

merge master

cc14002

revert files

24cae15

revert

2e98c6d

gettinig active track and announing once

5da48e5

Merge branch 'master' of https://github.com/amandadupell/grommet into…

3dbb3c1

… feat/video-controls-6059

amandadupell requested review from jcfilben and taysea May 9, 2022 16:14

jcfilben reviewed May 9, 2022

View reviewed changes

amandadupell added 2 commits May 9, 2022 19:35

Merge branch 'master' of https://github.com/amandadupell/grommet into…

627f558

… feat/video-controls-6059

removing unneccessary keys

a7991eb

jcfilben approved these changes May 10, 2022

View reviewed changes

ericsoderberghp approved these changes May 10, 2022

View reviewed changes

ericsoderberghp merged commit 76b0551 into grommet:master May 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Video Controls for Video Audio Description #6080

Enhance Video Controls for Video Audio Description #6080

amandadupell commented Apr 20, 2022 •

edited

taysea Apr 20, 2022

taysea Apr 20, 2022

taysea Apr 20, 2022 •

edited

taysea Apr 20, 2022

amandadupell May 4, 2022

jcfilben May 4, 2022

taysea left a comment •

edited

amandadupell commented May 4, 2022

jcfilben left a comment

jcfilben May 9, 2022

amandadupell May 9, 2022

jcfilben left a comment

		"showDescription": "Video description opened below video",
		"hideDescription": "Video description closed"

Enhance Video Controls for Video Audio Description #6080

Enhance Video Controls for Video Audio Description #6080

Conversation

amandadupell commented Apr 20, 2022 • edited

What does this PR do?

Where should the reviewer start?

What testing has been done on this PR?

How should this be manually tested?

Do Jest tests follow these best practices?

Any background context you want to provide?

What are the relevant issues?

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

taysea Apr 20, 2022

Choose a reason for hiding this comment

taysea Apr 20, 2022

Choose a reason for hiding this comment

taysea Apr 20, 2022 • edited

Choose a reason for hiding this comment

taysea Apr 20, 2022

Choose a reason for hiding this comment

amandadupell May 4, 2022

Choose a reason for hiding this comment

jcfilben May 4, 2022

Choose a reason for hiding this comment

taysea left a comment • edited

Choose a reason for hiding this comment

amandadupell commented May 4, 2022

jcfilben left a comment

Choose a reason for hiding this comment

jcfilben May 9, 2022

Choose a reason for hiding this comment

amandadupell May 9, 2022

Choose a reason for hiding this comment

jcfilben left a comment

Choose a reason for hiding this comment

amandadupell commented Apr 20, 2022 •

edited

taysea Apr 20, 2022 •

edited

taysea left a comment •

edited