Skip to content

Conversation

@adrums86
Copy link
Contributor

Description

Fixes text track modal controlText.

Specific Changes proposed

Only apply reset text to the Reset button and done text to the Done button.

Requirements Checklist

  • Feature implemented / Bug fixed
  • 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 (starter template on JSBin)
    • Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab)
    • Has no changes to JSDoc which cause npm run docs:api to error
  • Reviewed by Two Core Contributors

@codecov
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.13%. Comparing base (8842d37) to head (d2f7612).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8989   +/-   ##
=======================================
  Coverage   84.13%   84.13%           
=======================================
  Files         120      120           
  Lines        8117     8117           
  Branches     1948     1948           
=======================================
  Hits         6829     6829           
  Misses       1288     1288           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

const doneText = this.localize('Done');
const doneButton = new Button(player, {
controlText: defaultsDescription,
controlText: doneText,
Copy link
Member

Choose a reason for hiding this comment

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

if it's repeating the text content, we shouldn't need control text (though, worth verifying with a screen reader).

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tested these changes in NVDA & Voiceover, there's no repeat, the changes here just ensure the done and reset buttons have the correct text. Previously the done button had the same text as the reset button, which was misleading.

@Essk Essk merged commit 751ac56 into main Apr 10, 2025
13 checks passed
@Essk Essk deleted the fix-texttrackbuttons branch April 10, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants