-
Notifications
You must be signed in to change notification settings - Fork 14
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
ADA-1065-Added aria-labelby attribute to the dialog #896
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ class MainCaptionsWindow extends Component<any, any> { | |
render(props: any): VNode<any> { | ||
return ( | ||
<div className={[style.overlayScreen, style.active].join(' ')}> | ||
<h2 className={style.title}> | ||
<h2 className={style.title} id="captionsDialogTitle"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a way to avoid using id ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately the dialog has a visible title and it must be referenced with aria-labelledby that must point to the title id. If the title would be missing we could've used just an aria-label property that doesn't use id. Should I find a way to create a dynamic id so that each player has a unique one ? |
||
<Text id={'cvaa.title'} /> | ||
</h2> | ||
<div role="group"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,7 +127,7 @@ class Overlay extends Component<any, any> { | |
* @returns {React$Element} - component | ||
* @memberof Overlay | ||
*/ | ||
render({type, open, label = 'dialog'}: any): VNode<any> { | ||
render({type, open}: any): VNode<any> { | ||
const overlayClass = [style.overlay]; | ||
if (type) { | ||
const classType = style[type + '-overlay'] ? style[type + '-overlay'] : type + '-overlay'; | ||
|
@@ -140,7 +140,7 @@ class Overlay extends Component<any, any> { | |
} | ||
|
||
return ( | ||
<div tabIndex={-1} className={overlayClass.join(' ')} role="dialog" onKeyDown={this.onKeyDown} aria-label={label}> | ||
<div tabIndex={-1} className={overlayClass.join(' ')} role="dialog" onKeyDown={this.onKeyDown} aria-labelledby="captionsDialogTitle"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does tooltip still work if aria-label is removed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, works the same as before. |
||
<div className={style.overlayContents}>{this.props.children}</div> | ||
{this.renderCloseButton(this.props)} | ||
</div> | ||
|
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.
why remove it ?
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.
The label was passed as a prop just for the aria-label property that was replaced.