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

Add a player option noUITitleAttributes to prevent title attributes in the UI (fixes #6767). #7134

Merged
merged 3 commits into from
Mar 23, 2021

Conversation

OwenEdwards
Copy link
Member

Description

Some designers and developers feel that the presence of (redundant) title attribute values on UI controls makes the accessibility of those controls worse. The title attributes can also conflict with additions to make truly accessible tooltips on the controls.

This PR allows developers to easily disable the addition of title attributes on all UI controls with a single player configuration option.

Specific Changes proposed

Add a player option noUITitleAttributes which, if set to true, prevents video.js from adding any title attributes to UI controls.

Requirements Checklist

  • Feature implemented
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created
  • Reviewed by Two Core Contributors

@OwenEdwards OwenEdwards added the a11y This item might affect the accessibility of the player label Mar 11, 2021
@OwenEdwards OwenEdwards requested a review from gkatsev March 11, 2021 01:28
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

@OwenEdwards
Copy link
Member Author

I need to add documentation in docs/guides/options.md before this is ready to merge.

@@ -28,8 +28,11 @@
* [language](#language)
* [languages](#languages)
* [liveui](#liveui)
* [liveTracker.trackingThreshold](#livetrackertrackingthreshold)
* [liveTracker.liveTolerance](#livetrackerlivetolerance)
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 don't know where these two lines came from?! I can remove them?

Copy link
Member

Choose a reason for hiding this comment

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

If you've run npm run docs:fix, it probably added these two items that were added to the doc but we missed updating the TOC with them. #7097

Copy link
Member Author

Choose a reason for hiding this comment

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

@gkatsev so it would be okay to merge this change and let those two lines get added in options.md? Or would you prefer to have them in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

They can go in here.

@gkatsev gkatsev merged commit 5f59391 into videojs:main Mar 23, 2021
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y This item might affect the accessibility of the player
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants