diff --git a/docs/src/examples/components/Carousel/BestPractices/CarouselBestPractices.tsx b/docs/src/examples/components/Carousel/BestPractices/CarouselBestPractices.tsx index 1ed03d861c..bff5993ed9 100644 --- a/docs/src/examples/components/Carousel/BestPractices/CarouselBestPractices.tsx +++ b/docs/src/examples/components/Carousel/BestPractices/CarouselBestPractices.tsx @@ -1,5 +1,5 @@ import * as React from 'react' -import { Text } from '@fluentui/react' +import { Text, Box } from '@fluentui/react' import { link } from '../../../../utils/helpers' import ComponentBestPractices from 'docs/src/components/ComponentBestPractices' @@ -10,6 +10,15 @@ const doList = [ {link('reported issue', 'https://bugs.chromium.org/p/chromium/issues/detail?id=1040924')} for details). , + 'Provide localized string of the "carousel" using `ariaRoleDescription` prop.', + 'Provide label to the carousel using `ariaLabel` prop.', + + If carousel contains `navigation`: + + , ] const CarouselBestPractices: React.FunctionComponent<{}> = () => { diff --git a/docs/src/examples/components/Carousel/Types/CarouselExample.tsx b/docs/src/examples/components/Carousel/Types/CarouselExample.tsx index 27fd0c1b25..07b09efb90 100644 --- a/docs/src/examples/components/Carousel/Types/CarouselExample.tsx +++ b/docs/src/examples/components/Carousel/Types/CarouselExample.tsx @@ -37,6 +37,7 @@ const carouselItems = [ const CarouselExample = () => ( ({ diff --git a/docs/src/examples/components/Carousel/Types/CarouselPaginationExample.tsx b/docs/src/examples/components/Carousel/Types/CarouselPaginationExample.tsx index 7f47b9e54e..5d49c5ff76 100644 --- a/docs/src/examples/components/Carousel/Types/CarouselPaginationExample.tsx +++ b/docs/src/examples/components/Carousel/Types/CarouselPaginationExample.tsx @@ -23,6 +23,7 @@ const carouselItems = [ const CarouselExample = () => ( ( ({ diff --git a/packages/accessibility/src/behaviors/Carousel/carouselBehavior.ts b/packages/accessibility/src/behaviors/Carousel/carouselBehavior.ts index c0b80ca258..7e9eee6f6c 100644 --- a/packages/accessibility/src/behaviors/Carousel/carouselBehavior.ts +++ b/packages/accessibility/src/behaviors/Carousel/carouselBehavior.ts @@ -2,6 +2,13 @@ import { Accessibility } from '../../types' import * as keyboardKey from 'keyboard-key' /** + * @description + * Adds attribute 'role=region' to 'root' slot if 'navigation' property is false. Does not set the attribute otherwise. + * Adds attribute 'aria-roledescription' to 'root' slot if 'navigation' property is false. Does not set the attribute otherwise. + * Adds attribute 'aria-label' to 'root' slot if 'navigation' property is false. Does not set the attribute otherwise. + * Adds attribute 'aria-roledescription' to 'itemsContainer' slot if 'navigation' property is true. Does not set the attribute otherwise. + * Adds attribute 'aria-label' to 'itemsContainer' slot if 'navigation' property is true. Does not set the attribute otherwise. + * * @specification * Adds attribute 'role=region' to 'root' slot. * Adds attribute 'aria-live=polite' to 'itemsContainerWrapper' slot if 'ariaLiveOn' property is true. Sets the attribute to 'off' otherwise. @@ -9,6 +16,7 @@ import * as keyboardKey from 'keyboard-key' * Adds attribute 'aria-hidden=true' to 'paddlePrevious' slot if 'navigation' property is true. Does not set the attribute otherwise. * Adds attribute 'tabIndex=-1' to 'paddlePrevious' slot if 'navigation' property is true. Does not set the attribute otherwise. * Adds attribute 'tabIndex=-1' to 'paddlePrevious' slot if 'navigation' property is true. Does not set the attribute otherwise. + * Adds attribute 'role=group' to 'itemsContainer' slot if 'navigation' property is true. Does not set the attribute otherwise. * Triggers 'showNextSlideByKeyboardNavigation' action with 'ArrowRight' on 'itemsContainer'. * Triggers 'showPreviousSlideByKeyboardNavigation' action with 'ArrowLeft' on 'itemsContainer'. * Triggers 'showNextSlideByPaddlePress' action with 'Enter' or 'Spacebar' on 'paddleNext'. @@ -17,11 +25,14 @@ import * as keyboardKey from 'keyboard-key' const carouselBehavior: Accessibility = props => ({ attributes: { root: { - role: 'region', + ...(!props.navigation && { role: 'region', 'aria-roledescription': props.ariaRoleDescription, 'aria-label': props.ariaLabel }), }, itemsContainerWrapper: { 'aria-live': props.ariaLiveOn ? 'polite' : 'off', }, + itemsContainer: { + ...(props.navigation && { role: 'region', 'aria-roledescription': props.ariaRoleDescription, 'aria-label': props.ariaLabel }), + }, paddleNext: { ...(props.navigation && { @@ -63,6 +74,8 @@ export type CarouselBehaviorProps = { /** Element type. */ navigation: Object | Object[] ariaLiveOn: boolean + ariaRoleDescription?: string + ariaLabel?: string } export default carouselBehavior diff --git a/packages/accessibility/src/behaviors/Carousel/carouselItemBehavior.ts b/packages/accessibility/src/behaviors/Carousel/carouselItemBehavior.ts index 1e0d27c6c7..261c1a36b7 100644 --- a/packages/accessibility/src/behaviors/Carousel/carouselItemBehavior.ts +++ b/packages/accessibility/src/behaviors/Carousel/carouselItemBehavior.ts @@ -2,18 +2,20 @@ import { Accessibility } from '../../types' import * as keyboardKey from 'keyboard-key' /** + * @description + * Adds attribute 'tabIndex=0' to 'root' slot if 'active' property and 'navigation' property is true. Sets the attribute to '-1' otherwise. + * * @specification * Adds attribute 'role=tabpanel' to 'root' slot if 'navigation' property is true. Sets the attribute to 'group' otherwise. * Adds attribute 'aria-hidden=false' to 'root' slot if 'active' property is true. Sets the attribute to 'true' otherwise. - * Adds attribute 'tabIndex=0' to 'root' slot if 'active' property is true. Sets the attribute to '-1' otherwise. * Triggers 'arrowKeysNavigationStopPropagation' action with 'ArrowRight' or 'ArrowLeft' on 'root'. */ const carouselItemBehavior: Accessibility = props => ({ attributes: { root: { - role: props.navigation ? 'tabpanel' : 'group', - 'aria-hidden': props.active ? 'false' : 'true', - tabIndex: props.active ? 0 : -1, + role: props.navigation ? 'tabpanel' : 'none', + 'aria-hidden': props.active ? 'false' : 'true', + tabIndex: (props.navigation && props.active) ? 0 : -1, }, }, diff --git a/packages/accessibility/test/behaviors/caroselBehavior-test.tsx b/packages/accessibility/test/behaviors/caroselBehavior-test.tsx new file mode 100644 index 0000000000..a7e8e94c93 --- /dev/null +++ b/packages/accessibility/test/behaviors/caroselBehavior-test.tsx @@ -0,0 +1,99 @@ +import { carouselBehavior } from '@fluentui/accessibility' + +const roleDescription = 'carousel' +const label = 'portrait collection' + +describe('carouselBehavior.ts', () => { + describe('root', () => { + test(`sets "role=region" when carousel has NO navigation`, () => { + const expectedResult = carouselBehavior({ ariaLiveOn: false, navigation: false }) + expect(expectedResult.attributes.root.role).toEqual('region') + }) + + test('sets "aria-roledescription" when carousel has NO navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: false, + ariaRoleDescription: roleDescription, + }) + expect(expectedResult.attributes.root['aria-roledescription']).toEqual(roleDescription) + }) + + test('sets "aria-label" when carousel has NO navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: false, + ariaLabel: label, + }) + expect(expectedResult.attributes.root['aria-label']).toEqual(label) + }) + + test('do NOT set "aria-roledescription" when carousel has navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: true, + ariaRoleDescription: roleDescription, + }) + expect(expectedResult.attributes.root['aria-roledescription']).toBeUndefined() + }) + + test('do NOT set "aria-label" when carousel has navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: true, + ariaLabel: label, + }) + expect(expectedResult.attributes.root['aria-label']).toBeUndefined() + }) + + test(`do NOT set "role=region" when carousel has navigation`, () => { + const expectedResult = carouselBehavior({ ariaLiveOn: false, navigation: true }) + expect(expectedResult.attributes.root.role).toBeUndefined() + }) + }) + + describe('itemsContainer', () => { + test('sets "aria-roledescription" when carousel has navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: true, + ariaRoleDescription: roleDescription, + }) + expect(expectedResult.attributes.itemsContainer['aria-roledescription']).toEqual( + roleDescription, + ) + }) + + test('sets "aria-label" when carousel has navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: true, + ariaLabel: label, + }) + expect(expectedResult.attributes.itemsContainer['aria-label']).toEqual(label) + }) + + test('do NOT set "aria-roledescription" when carousel has NO navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: false, + ariaRoleDescription: roleDescription, + }) + expect(expectedResult.attributes.itemsContainer['aria-roledescription']).toBeUndefined() + }) + + test('do NOT set "aria-label" when carousel has NO navigation', () => { + const expectedResult = carouselBehavior({ + ariaLiveOn: false, + navigation: false, + ariaLabel: label, + }) + expect(expectedResult.attributes.itemsContainer['aria-label']).toBeUndefined() + }) + + test(`do NOT set "role=group" when carousel has NO navigation`, () => { + const expectedResult = carouselBehavior({ ariaLiveOn: false, navigation: false }) + expect(expectedResult.attributes.itemsContainer.role).toBeUndefined() + }) + }) +}) diff --git a/packages/accessibility/test/behaviors/caroseltemBehavior-test.tsx b/packages/accessibility/test/behaviors/caroseltemBehavior-test.tsx new file mode 100644 index 0000000000..623b1d6528 --- /dev/null +++ b/packages/accessibility/test/behaviors/caroseltemBehavior-test.tsx @@ -0,0 +1,23 @@ +import { carouselItemBehavior } from '@fluentui/accessibility' + +describe('carouselItemBehavior.ts', () => { + test('sets tabIndex="0" on root when carousel has navigation and item is visible ', () => { + const expectedResult = carouselItemBehavior({ navigation: true, active: true }) + expect(expectedResult.attributes.root.tabIndex).toEqual(0) + }) + + test('sets tabIndex="-1" on root when carousel has navigation and item is NOT visible ', () => { + const expectedResult = carouselItemBehavior({ navigation: true, active: false }) + expect(expectedResult.attributes.root.tabIndex).toEqual(-1) + }) + + test('sets tabIndex="-1" on root when carousel has NO navigation and item is visible', () => { + const expectedResult = carouselItemBehavior({ navigation: false, active: true }) + expect(expectedResult.attributes.root.tabIndex).toEqual(-1) + }) + + test('sets tabIndex="-1" on root when carousel has NO navigation and item is NOT visible', () => { + const expectedResult = carouselItemBehavior({ navigation: false, active: false }) + expect(expectedResult.attributes.root.tabIndex).toEqual(-1) + }) +}) diff --git a/packages/react/src/components/Carousel/Carousel.tsx b/packages/react/src/components/Carousel/Carousel.tsx index 33baa48dc0..cd9acd2c87 100644 --- a/packages/react/src/components/Carousel/Carousel.tsx +++ b/packages/react/src/components/Carousel/Carousel.tsx @@ -56,6 +56,11 @@ export interface CarouselProps extends UIComponentProps, ChildrenComponentProps */ ariaRoleDescription?: string + /** + * Sets the aria-label attribute for carousel. + */ + ariaLabel?: string + /** Specifies if the process of switching slides is circular. */ circular?: boolean @@ -75,8 +80,8 @@ export interface CarouselProps extends UIComponentProps, ChildrenComponentProps /** Shorthand array of props for the buttons of the CarouselNavigation. */ navigation?: - | ShorthandValue - | ShorthandCollection + | ShorthandValue + | ShorthandCollection /** * A Carousel can position its navigation below the content by default, @@ -135,6 +140,7 @@ class Carousel extends AutoControlledComponent, Carous }), activeIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), ariaRoleDescription: PropTypes.string, + ariaLabel: PropTypes.string, circular: PropTypes.bool, defaultActiveIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), getItemPositionText: PropTypes.func, @@ -192,27 +198,13 @@ class Carousel extends AutoControlledComponent, Carous }, showNextSlideByPaddlePress: e => { e.preventDefault() - const { activeIndex } = this.state - const { circular, items, navigation } = this.props - this.showNextSlide(e, false) - - // if 'next' paddle will disappear, will focus 'previous' one. - if (!navigation && activeIndex >= items.length - 2 && !circular) { - this.paddlePreviousRef.current.focus() - } + this.handleNextPaddleFocus() }, showPreviousSlideByPaddlePress: e => { e.preventDefault() - const { activeIndex } = this.state - const { circular, navigation } = this.props - this.showPreviousSlide(e, false) - - // if 'previous' paddle will disappear, will focus 'next' one. - if (!navigation && activeIndex <= 1 && !circular) { - this.paddleNextRef.current.focus() - } + this.handlePreviousPaddleFocus() }, } @@ -283,7 +275,7 @@ class Carousel extends AutoControlledComponent, Carous }) renderContent = (accessibility, classes, unhandledProps) => { - const { ariaRoleDescription, getItemPositionText, items, circular } = this.props + const { getItemPositionText, items, circular } = this.props const { activeIndex, itemIds, prevActiveIndex } = this.state this.itemRefs = [] @@ -295,7 +287,6 @@ class Carousel extends AutoControlledComponent, Carous >
, Carous slideToNext = false } - return ( + return active ? ( @@ -349,7 +337,7 @@ class Carousel extends AutoControlledComponent, Carous })} - ) + ) : null })}
@@ -364,13 +352,33 @@ class Carousel extends AutoControlledComponent, Carous this.setActiveIndex(e, this.state.activeIndex + 1, focusItem) } + handleNextPaddleFocus = () => { + // if 'next' paddle will disappear, will focus 'previous' one. + if ( + !this.props.navigation && + this.state.activeIndex >= this.props.items.length - 2 && + !this.props.circular + ) { + this.paddlePreviousRef.current.focus() + } + } + + handlePreviousPaddleFocus = () => { + // if 'previous' paddle will disappear, will focus 'next' one. + if (!this.props.navigation && this.state.activeIndex <= 1 && !this.props.circular) { + this.paddleNextRef.current.focus() + } + } + handlePaddleOverrides = (predefinedProps: ButtonProps, paddleName: string) => ({ onClick: (e: React.SyntheticEvent, buttonProps: ButtonProps) => { _.invoke(predefinedProps, 'onClick', e, buttonProps) if (paddleName === 'paddleNext') { this.showNextSlide(e, false) + this.handleNextPaddleFocus() } else if (paddleName === 'paddlePrevious') { this.showPreviousSlide(e, false) + this.handlePreviousPaddleFocus() } }, onBlur: (e: React.FocusEvent, buttonProps: ButtonProps) => { @@ -453,11 +461,12 @@ class Carousel extends AutoControlledComponent, Carous }), }) ) : ( - - ) +