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

[Draft] Separate default video viewer from custom controls and improve accessibility #6655

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

goplayoutside3
Copy link
Contributor

@goplayoutside3 goplayoutside3 commented Jan 29, 2025

(Draft while I figure out why Firefox and Safari left/right arrows behave differently in a slider than every other browser 🙄)

Package

lib-classifier

Linked Issue and/or Talk Post

Toward #6334
Toward #3759

Describe your changes

  • Create separate files for VideoViewer and VideoWithDrawing
    • VideoWIthDrawing displays custom video controls while default VideoViewer does not
  • Improve accessibility of custom video controls
    • Use of spacebar and/or Enter while focused on the play/pause button has no lag (I think this PR is as good as it's going to get. Any lag might be due to the progress interval set to 100ms. Most Zooniverse videos are <10seconds in length, and a smaller progress interval reduces the Grommet Slider performance
    • Use of spacebar and/or Enter while focused on the Slider should play/pause the video
    • Using left/right keys while focused on the select slider should step the video backwards and forwards
    • Keyboard use while focused on the volume slider: the up and down arrows should increase and decrease the volume
    • If a video file has no sound, disabled the volume button
      • Firefox desktop does not accurately check whether a video has volume. Even if a video doesn't have sound, the volume button is enabled which is better than accidentally disabling volume when it's available. Can't find a way around this Firefox behavior.
    • ? When the classifier loads, focus on the video controls so volunteers can play it right away via spacebar or Enter
      • This is a question I want to pose to Sean after this PR is merged and available to test on frontend.preview.

How to Review

I tested the above keyboard + mouse functionality of custom controls in:

  • Desktop Chrome
  • Desktop Safari (must have tab via keyboard enabled in Accessibility settings, this is different than every other browser which by default lets you tab to buttons in a webpage)
  • Desktop Firefox

Touch to scrub the progress slider doesn't work well in mobile browsers

  • Mobile Safari
  • Mobile Brave browser

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

New Feature

  • The PR creator has listed user actions to use when testing the new feature
  • Unit tests are included for the new feature
  • A storybook story has been created or updated
    • Example of SlideTutorial component with docgen comments, and its story

Refactoring

  • The PR creator has described the reason for refactoring
  • The refactored component(s) continue to work as expected

@goplayoutside3 goplayoutside3 added the refactor Refactoring existing code label Jan 29, 2025
@coveralls
Copy link

coveralls commented Jan 30, 2025

Coverage Status

coverage: 75.467% (-0.2%) from 75.621%
when pulling 39ca59a on video-subject-viewer-enhancements
into e68e2c6 on master.

@goplayoutside3 goplayoutside3 changed the title [Draft] Separate default video viewer code from features needed for drawing tools [Draft] Separate default video viewer from custom controls and improve accessibility Jan 31, 2025
@goplayoutside3 goplayoutside3 added the accessibility Improving the experience for users of assistive technologies label Jan 31, 2025
@goplayoutside3 goplayoutside3 added the video subject Issue related to the video subject viewer label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Improving the experience for users of assistive technologies refactor Refactoring existing code video subject Issue related to the video subject viewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants