-
Notifications
You must be signed in to change notification settings - Fork 180
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
Sets ‘has captions or subtitles accessibility’ label #3608
Sets ‘has captions or subtitles accessibility’ label #3608
Conversation
…itle file language is the same as the video file language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix works as described, code changes make sense. Just one debugging statement to remove.
subtitleFileLanguageComparison(file) { | ||
if (this.oneSelected && this.language === file.language) { | ||
this.accessibility = [...this.accessibility, AccessibilityCategories.CAPTIONS_SUBTITLES]; | ||
console.log({ ...this.accessibility }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should remove this debugging statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed!
@@ -196,7 +196,7 @@ export const ContentModalities = { | |||
export const AccessibilityCategoriesMap = { | |||
// Note: audio is not included, as it is rendered in the UI differently. | |||
document: ['ALT_TEXT', 'HIGH_CONTRAST', 'TAGGED_PDF'], | |||
video: ['SIGN_LANGUAGE', 'AUDIO_DESCRIPTION'], | |||
video: ['SIGN_LANGUAGE', 'AUDIO_DESCRIPTION', 'CAPTIONS_SUBTITLES'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an important bug fix in its own right!
Summary
Description of the change(s) you made
This PR checks the ‘has captions or subtitles accessibility’ label automatically if the uploaded subtitle file language is the same as the video file language. The language dropdown placement for the captions and subtitles section was previously shown underneath it but would be cut off. This PR changes the placement of the menu options to show above instead.
Manual verification steps performed
Screenshots (if applicable)
Checked ‘Has captions or subtitles’ checkbox.
Previous placement for language dropdown underneath Captions and subtitles section.
Updated language dropdown placement.
Does this introduce any tech-debt items?
Reviewer guidance
How can a reviewer test these changes?
References
Fixes #3576
Comments
Note: The ‘Has captions and subtitles’ checkbox label description needs to be updated.
Radina and Jessica agree that the label is descriptive enough and suggest removing the info icon next to the label and reordering this label to be first in the list instead since it’s expected to be the most commonly used option out of those three.
Contributor's Checklist
Studio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)