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

Interactive video transcript #5631

Merged
merged 61 commits into from
Nov 12, 2019

Conversation

bjester
Copy link
Member

@bjester bjester commented Jun 6, 2019

Summary

This adds a new transcript feature to the video player in Kolibri. It utilizes existing text tracks that generate the subtitles (captions) to create a transcript that appears alongside of the video.

Before After
screenshot-kolibridemo learningequality org-2019 11 06-08-56-23 screenshot-localhost-8000-2019 11 06-08-40-39
screenshot-localhost-8000-2019 11 06-08-40-06

In technical terms, this uses the component architecture of Video.js to add and modify behavior within the video's container managed by Video.js. Mixins are used to connect Vue components with the Video.js components to allow for using Vue within the Video.js container. The mixin connection allows for utilizing some of handling that video.js provides as well as the flexibility of Vue components and our infrastructure built upon that.

Reviewer guidance

Testing areas:

  1. Initial state should have subtitles enabled by default if there is matching language with the current Kolibri language.
  2. Switching between videos that have different language options and ensuring options are disabled and enabled appropriately.
  3. Various video preferences should persist, including subtitle and transcript preferences.
  4. Responsiveness to different screen sizes.
  5. Performance: There is quite a bit of event handling to produce and render the transcript.

Known issues:

  1. Control buttons in the video's menu bar do not have active highlighting like video.js adds by default.
  2. Video.js menus are not exclusive when open, and these new custom menus do not change that behavior, meaning more than one menu may be open at once.
  3. In Safari, cues do not load in transcript until after first cue becomes active or transcript is toggled.

References

https://www.notion.so/learningequality/Interactive-Video-Transcript-9ac195ce394942838a85ed22c0ebeaf0


Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Testing:

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@bjester bjester added the work-in-progress Not ready for review label Jun 6, 2019
@bjester bjester changed the title [WIP] Interactive video transcript Interactive video transcript Nov 7, 2019
@bjester bjester added TODO: needs review Waiting for review and removed work-in-progress Not ready for review labels Nov 7, 2019
@indirectlylit indirectlylit removed the TODO: needs gherkin update Add to our manual integration tests label Nov 7, 2019
@bjester
Copy link
Member Author

bjester commented Nov 7, 2019

Added issue #6034

@radinamatic
Copy link
Member

radinamatic commented Nov 8, 2019

Great to be able to activate/deactivate the captions by keyboard! 👍 👏

One issue I run into with this is when the user is navigating up and down the transcript while video is playing, video player bar with all the rest of controls disappears, not only visually, but from the TAB order too. This results in user being unable to navigate back and pause/stop or control the playback in any other way. Happens with both positions of the transcript (sidebar and below).

(screencast here, too big for GH attachment)

Not sure what here is the best solution: are we able to keep the control bar visible, and in the TAB order while the keyboard focus is inside the transcript?

Second problem might be more hairy: I see a real issue when the video is long, and the list of captions becomes interminable. Could we think about the option to allow users to go to beginning or the end of the transcript with some keyboard shortcut? 🤔

I was looking at various patterns at WAI-ARIA Authoring Practices, and several (for example Listbox) include using Home and End keys for precisely this, and add that Supporting this key is strongly recommended for lists with more than five options.

Now, I'm not suggesting you refactor the transcript completely to reflect this whole pattern, just to see if we could add something similar to be able to navigate more easily to beginning and the end of the long transcripts

@bjester
Copy link
Member Author

bjester commented Nov 8, 2019

@radinamatic

One issue I run into with this is when the user is navigating up and down the transcript while video is playing, video player bar with all the rest of controls disappears, not only visually, but from the TAB order too. This results in user being unable to navigate back and pause/stop or control the playback in any other way. Happens with both positions of the transcript (sidebar and below).

That's a good catch. I'm not a fan of the control bar disappearing, but that was an existing part of the renderer, so without the transcript, the issue exists with it disappearing from the TAB order.

Not sure what here is the best solution: are we able to keep the control bar visible, and in the TAB order while the keyboard focus is inside the transcript?

That could work, but I think you found a larger issue in that it disappears with or without the transcript. So perhaps a solution shouldn't be dependent on the transcript? Perhaps in keyboard modality, we keep the control bar visible regardless?

I was looking at various patterns at WAI-ARIA Authoring Practices, and several (for example Listbox) include using Home and End keys for precisely this, and add that Supporting this key is strongly recommended for lists with more than five options.

I think adding support for Home and End should be reasonably straightforward. Perhaps longer term we could think about changing the roles of the items in the transcript such that it's traversalable by arrow keys and perhaps Tab jumps between Beginning of transcript and End of transcript

@radinamatic
Copy link
Member

Perhaps in keyboard modality, we keep the control bar visible regardless

I think this a reasonable solution, considering how much keyboard-only users depend on easily locating controls they need, and control bar disappearing by default makes that more difficult!

If this approach does not involve too much tweaking, go for it 👍

changing the roles of the items in the transcript such that it's traversalable by arrow keys and perhaps Tab jumps between Beginning of transcript and End of transcript

Yes, I like this suggestion a lot! 😍
Let's put it on the 0.14 feature list, and proceed to add the support for Home and End in the scope of this PR.

Thank you!

@@ -41,6 +41,9 @@ export default function KThemePlugin(Vue) {
outline: 'none',
};
},
$inputModality() {
return globalThemeState.inputModality;
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had tried to add it as a computed prop solely to the media player component, but it wouldn't update. Although, this works even though it's nearly the same.

@indirectlylit indirectlylit added the TAG: user strings Application text that gets translated label Nov 10, 2019
@bjester bjester merged commit 6209aa1 into learningequality:develop Nov 12, 2019
@bjester bjester deleted the interactive-video-transcript branch November 12, 2019 19:37
@jonboiser jonboiser removed the TODO: needs review Waiting for review label Nov 12, 2019
@indirectlylit indirectlylit mentioned this pull request Nov 14, 2019
9 tasks
@radinamatic radinamatic removed the TODO: needs user docs Requires user docs update label Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog Important user-facing changes TAG: user strings Application text that gets translated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants