-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Coachmark accessibility #5155
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
Coachmark accessibility #5155
Changes from 29 commits
2b7eabf
eea962f
ec0dad4
f97afeb
689018c
a4f5f14
6e9019a
1576232
17ac4d7
4fa9c4d
1fd35d6
752ef79
e4c7b20
e70b32b
f2173de
f1e86c5
344167f
93811cf
32029ac
ef70ee4
101854f
7774b0e
3b1c254
94b7034
413e38d
89b06a9
e149c87
989b8b7
4884b00
b04c5b3
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 |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Coachmark: Add accessibility features to component, ARIA props, narrator support, and keyboarding controls", | ||
| "type": "minor" | ||
| } | ||
| ], | ||
| "packageName": "office-ui-fabric-react", | ||
| "email": "edwl@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| // Utilities | ||
| import * as React from 'react'; | ||
| import { BaseComponent, IRectangle, classNamesFunction, createRef, shallowCompare } from '../../Utilities'; | ||
| import { BaseComponent, classNamesFunction, createRef, IRectangle, KeyCodes, shallowCompare } from '../../Utilities'; | ||
| import { DefaultPalette } from '../../Styling'; | ||
| import { IPositionedData, RectangleEdge, getOppositeEdge } from '../../utilities/positioning'; | ||
|
|
||
|
|
@@ -18,7 +18,7 @@ import { | |
| ICoachmarkStyles, | ||
| ICoachmarkStyleProps | ||
| } from './Coachmark.styles'; | ||
| import { FocusZone } from '../../FocusZone'; | ||
| import { FocusTrapZone } from '../FocusTrapZone'; | ||
|
|
||
| const getClassNames = classNamesFunction<ICoachmarkStyleProps, ICoachmarkStyles>(); | ||
|
|
||
|
|
@@ -95,6 +95,11 @@ export interface ICoachmarkState { | |
| * Transform origin of teaching bubble callout | ||
| */ | ||
| transformOrigin?: string; | ||
|
|
||
| /** | ||
| * ARIA alert text to read aloud with Narrator once the Coachmark is mounted | ||
| */ | ||
| alertText?: string; | ||
| } | ||
|
|
||
| export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | ||
|
|
@@ -115,6 +120,7 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| */ | ||
| private _entityInnerHostElement = createRef<HTMLDivElement>(); | ||
| private _translateAnimationContainer = createRef<HTMLDivElement>(); | ||
| private _ariaAlertContainer = createRef<HTMLDivElement>(); | ||
| private _positioningContainer = createRef<IPositioningContainer>(); | ||
|
|
||
| /** | ||
|
|
@@ -149,7 +155,17 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| } | ||
|
|
||
| public render(): JSX.Element { | ||
| const { children, target, color, positioningContainerProps } = this.props; | ||
| const { | ||
| children, | ||
| target, | ||
| color, | ||
| positioningContainerProps, | ||
| ariaDescribedBy, | ||
| ariaDescribedByText, | ||
| ariaLabelledBy, | ||
| ariaLabelledByText, | ||
| ariaAlertText | ||
| } = this.props; | ||
|
|
||
| const { | ||
| beakLeft, | ||
|
|
@@ -160,7 +176,8 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| isBeaconAnimating, | ||
| isMeasuring, | ||
| entityInnerHostRect, | ||
| transformOrigin | ||
| transformOrigin, | ||
| alertText | ||
| } = this.state; | ||
|
|
||
| const classNames = getClassNames(getStyles, { | ||
|
|
@@ -188,6 +205,16 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| {...positioningContainerProps} | ||
| > | ||
| <div className={classNames.root}> | ||
| {ariaAlertText && ( | ||
| <div | ||
| className={classNames.ariaContainer} | ||
| role="alert" | ||
| ref={this._ariaAlertContainer} | ||
| aria-hidden={!isCollapsed} | ||
| > | ||
| {alertText} | ||
| </div> | ||
| )} | ||
| <div className={classNames.pulsingBeacon} /> | ||
| <div className={classNames.translateAnimationContainer} ref={this._translateAnimationContainer}> | ||
| <div className={classNames.scaleAnimationLayer}> | ||
|
|
@@ -202,13 +229,36 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| color={color} | ||
| /> | ||
| )} | ||
| <FocusZone> | ||
| <div className={classNames.entityHost} data-is-focusable={true} onFocus={this._onFocusHandler}> | ||
| <div className={classNames.entityInnerHost} ref={this._entityInnerHostElement}> | ||
| <FocusTrapZone isClickableOutsideFocusTrap={true}> | ||
| <div | ||
| className={classNames.entityHost} | ||
| tabIndex={-1} | ||
| data-is-focusable={true} | ||
| role="dialog" | ||
| aria-labelledby={ariaLabelledBy} | ||
| aria-describedby={ariaDescribedBy} | ||
| > | ||
| {isCollapsed && [ | ||
| ariaLabelledBy && ( | ||
| <p id={ariaLabelledBy} className={classNames.ariaContainer}> | ||
| {ariaLabelledByText} | ||
| </p> | ||
| ), | ||
| ariaDescribedBy && ( | ||
| <p id={ariaDescribedBy} className={classNames.ariaContainer}> | ||
| {ariaDescribedByText} | ||
| </p> | ||
| ) | ||
| ]} | ||
| <div | ||
| className={classNames.entityInnerHost} | ||
| ref={this._entityInnerHostElement} | ||
| aria-hidden={isCollapsed} | ||
| > | ||
| {children} | ||
| </div> | ||
| </div> | ||
| </FocusZone> | ||
| </FocusTrapZone> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
@@ -255,14 +305,43 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| this.forceUpdate(); | ||
| } | ||
|
|
||
| document.addEventListener('keydown', this._onKeyDown, true); | ||
|
Contributor
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. BaseComponent provides |
||
|
|
||
| // We dont want to the user to immediatley trigger the coachmark when it's opened | ||
| this._async.setTimeout(() => { | ||
| this._addProximityHandler(this.props.mouseProximityOffset); | ||
| }, this.props.delayBeforeMouseOpen!); | ||
|
|
||
| // Need to add setTimeout to have narrator read change in alert container | ||
| if (this.props.ariaAlertText) { | ||
| this._async.setTimeout(() => { | ||
| if (this.props.ariaAlertText && this._ariaAlertContainer.current) { | ||
| this.setState({ | ||
| alertText: this.props.ariaAlertText | ||
| }); | ||
| } | ||
| }, 0); | ||
| } | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| public componentWillUnmount(): void { | ||
| document.removeEventListener('keydown', this._onKeyDown); | ||
| } | ||
|
|
||
| private _onKeyDown = (e: any): void => { | ||
| // Open coachmark if user presses ALT + C (arbitrary keypress for now) | ||
| if ( | ||
| (e.altKey && e.which === KeyCodes.c) || | ||
| (e.which === KeyCodes.enter && | ||
| this._translateAnimationContainer.current && | ||
| this._translateAnimationContainer.current.contains(e.target)) | ||
| ) { | ||
| this._onFocusHandler(); | ||
| } | ||
| }; | ||
|
|
||
| private _onFocusHandler = (): void => { | ||
| if (this.state.isCollapsed) { | ||
| this._openCoachmark(); | ||
|
|
@@ -403,6 +482,13 @@ export class Coachmark extends BaseComponent<ICoachmarkTypes, ICoachmarkState> { | |
| this._entityInnerHostElement.current.addEventListener( | ||
| 'transitionend', | ||
| (): void => { | ||
| // Need setTimeout to trigger narrator | ||
| this._async.setTimeout(() => { | ||
| if (this.props.teachingBubbleRef && this.props.teachingBubbleRef) { | ||
|
Contributor
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. This is redundant |
||
| this.props.teachingBubbleRef.focus(); | ||
| } | ||
| }, 500); | ||
|
|
||
| if (this.props.onAnimationOpenEnd) { | ||
| this.props.onAnimationOpenEnd(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { Coachmark } from './Coachmark'; | |
| import { ICoachmarkStyles, ICoachmarkStyleProps } from './Coachmark.styles'; | ||
| import { IPositioningContainerTypes } from './PositioningContainer/PositioningContainer.types'; | ||
| import { IStyleFunctionOrObject } from '../../Utilities'; | ||
| import { ITeachingBubble } from '../../TeachingBubble'; | ||
|
|
||
| export interface ICoachmark {} | ||
|
|
||
|
|
@@ -32,6 +33,7 @@ export interface ICoachmarkTypes extends React.Props<Coachmark> { | |
|
|
||
| /** | ||
| * Whether or not to force the Coachmark/TeachingBubble content to fit within the window bounds. | ||
| * @default true | ||
| */ | ||
| isPositionForced?: boolean; | ||
|
|
||
|
|
@@ -114,4 +116,34 @@ export interface ICoachmarkTypes extends React.Props<Coachmark> { | |
| * Beacon color two. | ||
| */ | ||
| beaconColorTwo?: string; | ||
|
|
||
| /** | ||
| * Text to announce to screen reader / narrator when Coachmark is displayed | ||
| */ | ||
| ariaAlertText?: string; | ||
|
|
||
| /** | ||
|
Collaborator
Author
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. @JasonGore Any suggestions here? I'm not quite sure how make the interface for teachingBubbleRef look nicer. |
||
| * Ref for TeachingBubble | ||
| */ | ||
| teachingBubbleRef?: ITeachingBubble; | ||
|
|
||
| /** | ||
| * Defines the element id referencing the element containing label text for Coachmark. | ||
| */ | ||
| ariaLabelledBy?: string; | ||
|
|
||
| /** | ||
| * Defines the element id referencing the element containing the description for the Coachmark. | ||
| */ | ||
| ariaDescribedBy?: string; | ||
|
|
||
| /** | ||
| * Defines the text content for the ariaLabelledBy element | ||
| */ | ||
| ariaLabelledByText?: string; | ||
|
|
||
| /** | ||
| * Defines the text content for the ariaDescribedBy element | ||
| */ | ||
| ariaDescribedByText?: string; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| - Only one Coachmark + TeachingBubble combo should be displayed at a time | ||
| - Coachmarks can be stand alone or sequential. Sequential Coachmarks should be used sparingly, to walk through complex multi-step interactions. It is recommended that a sequence of Coachmarks does not exceed 3 steps. | ||
| - Coachmarks are designed to only hold TeachingBubbles. | ||
| - Coachmarks are designed to only hold TeachingBubbles | ||
| - Provide descriptive text in the `ariaDescribedByText` prop to let accessibility impaired users know how to open/access the Coachmark with keyboard controls. (See example in documentation) | ||
| - The keyboard shortcut for opening the Coachmark is `Alt + C` |
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.
You can move this up to the other isCollapsed section so you don't need to evaluate twice. You might need to make it an array if it complains about needing a single parent but I don't believe you'll need to do that.
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.
Fixed in latest commit