-
Notifications
You must be signed in to change notification settings - Fork 12
Screenreader Support for Oceans Guide #411
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
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 |
|---|---|---|
|
|
@@ -29,11 +29,37 @@ export const stopTypingSounds = () => { | |
| }; | ||
|
|
||
| let UnwrappedGuide = class Guide extends React.Component { | ||
| guideDialogRef = React.createRef(); | ||
|
|
||
| componentDidUpdate() { | ||
| // Focus the dialog only when the guide changes, not on every re-render | ||
| const currentGuide = guide.getCurrentGuide(); | ||
| const currentGuideId = currentGuide ? currentGuide.id : null; | ||
|
|
||
| if ( | ||
| currentGuideId !== this.lastFocusedGuideId && | ||
| currentGuide && | ||
| this.guideDialogRef && | ||
| this.guideDialogRef.current | ||
| ) { | ||
| this.guideDialogRef.current.focus(); | ||
| this.lastFocusedGuideId = currentGuideId; | ||
| } else if (!currentGuide) { | ||
| this.lastFocusedGuideId = null; | ||
| } | ||
| } | ||
| onTypingDone() { | ||
| clearInterval(getState().guideTypingTimer); | ||
| setState({guideShowing: true, guideTypingTimer: undefined}); | ||
| } | ||
|
|
||
| onGuideKeyDown = (e) => { | ||
| if (e.key === ' ' || e.key === 'Enter' || e.key === 'Spacebar') { | ||
| e.preventDefault(); | ||
| this.onGuideClick(); | ||
| } | ||
| }; | ||
|
|
||
| onGuideClick = () => { | ||
| const state = getState(); | ||
| const currentGuide = guide.getCurrentGuide(); | ||
|
|
@@ -157,22 +183,36 @@ let UnwrappedGuide = class Guide extends React.Component { | |
| key={currentGuide.id} | ||
| style={guideBgStyle} | ||
| onClick={this.onGuideClick} | ||
| onKeyDown={this.onGuideKeyDown} | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label={I18n.t('continue')} | ||
| id="uitest-dismiss-guide" | ||
| > | ||
| <div | ||
| ref={this.guideDialogRef} | ||
| aria-labelledby="guide-heading" | ||
| tabIndex={-1} | ||
| className="guide-dialog" | ||
| style={{ | ||
| ...styles.guide, | ||
| ...styles[`guide${currentGuide.style}`] | ||
| }} | ||
| > | ||
| <div> | ||
| {currentGuide.style === 'Info' && ( | ||
| <div style={styles.guideHeading}> | ||
| <div id="guide-heading" style={styles.guideHeading}> | ||
| {I18n.t('didYouKnow')} | ||
| </div> | ||
| )} | ||
|
|
||
| <div style={styles.guideTypingText}> | ||
|
|
||
| {/* Visually hidden aria-live region for screen readers */} | ||
| <div style={{position: 'absolute', left: '-9999px', width: '1px', height: '1px', overflow: 'hidden'}} aria-live="polite"> | ||
|
||
| {currentGuide.textFn(getState())} | ||
| </div> | ||
| {/* Visible Typist animation for sighted users */} | ||
| <div style={styles.guideTypingText} aria-hidden="true"> | ||
| <Typist | ||
| avgTypingDelay={35} | ||
| stdTypingDelay={15} | ||
|
|
@@ -190,7 +230,7 @@ let UnwrappedGuide = class Guide extends React.Component { | |
| : styles.guideFinalTextContainer | ||
| } | ||
| > | ||
| <div style={styles.guideFinalText}> | ||
| <div style={styles.guideFinalText} aria-hidden="true"> | ||
| {currentGuide.textFn(getState())} | ||
| </div> | ||
| </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.
The guide dialog uses aria-labelledby='guide-heading' but this element only exists when currentGuide.style === 'Info'. For other guide styles, the aria-labelledby will reference a non-existent element, which could confuse screenreaders. Consider using aria-label with appropriate text for non-Info guides, or conditionally setting aria-labelledby only when the heading exists.
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.
If we don't have a guide providing info, we don't need a label here. Plus, I tried adding a conditional aria-label and it took over precedence