Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(carousel): pagination carousel - accessibility improvements #2278

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -10,6 +10,15 @@ const doList = [
{link('reported issue', 'https://bugs.chromium.org/p/chromium/issues/detail?id=1040924')} for
details).
</Text>,
'Provide localized string of the "carousel" using `ariaRoleDescription` prop.',
'Provide label to the carousel using `ariaLabel` prop.',
<Box>
If carousel contains `navigation`:
<ul>
<li> provide label to `navigation` and to navigation item using `aria-label` attribute</li>
<li> add `aria-controls` attribute to navigation item referencing to `carouselItem` id </li>
</ul>
</Box>,
]

const CarouselBestPractices: React.FunctionComponent<{}> = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const carouselItems = [
const CarouselExample = () => (
<Carousel
ariaRoleDescription="carousel"
ariaLabel="Portrait collection"
navigation={{
'aria-label': 'people portraits',
items: carouselItems.map((item, index) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const carouselItems = [
const CarouselExample = () => (
<Carousel
ariaRoleDescription="carousel"
ariaLabel="Portrait collection"
items={carouselItems}
paddleNext={{ 'aria-label': 'go to next slide' }}
paddlePrevious={{ 'aria-label': 'go to previous slide' }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const CarouselExample = () => (
<Carousel
circular
ariaRoleDescription="carousel"
ariaLabel="Portrait collection"
navigation={{
'aria-label': 'people portraits',
items: carouselItems.map((item, index) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@ 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.
* Adds attribute 'aria-hidden=true' to 'paddleNext' slot if 'navigation' property is true. Does not set the attribute otherwise.
* 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'.
Expand All @@ -17,11 +25,14 @@ import * as keyboardKey from 'keyboard-key'
const carouselBehavior: Accessibility<CarouselBehaviorProps> = 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: {
Copy link
Collaborator

@silviuaavram silviuaavram Feb 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments as above here.

...(props.navigation && { role: 'region', 'aria-roledescription': props.ariaRoleDescription, 'aria-label': props.ariaLabel }),
},

paddleNext: {
...(props.navigation && {
Expand Down Expand Up @@ -63,6 +74,8 @@ export type CarouselBehaviorProps = {
/** Element type. */
navigation: Object | Object[]
ariaLiveOn: boolean
ariaRoleDescription?: string
ariaLabel?: string
}

export default carouselBehavior
Original file line number Diff line number Diff line change
Expand Up @@ -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<CarouselItemProps> = 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,
},
},

Expand Down
99 changes: 99 additions & 0 deletions packages/accessibility/test/behaviors/caroselBehavior-test.tsx
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
Original file line number Diff line number Diff line change
@@ -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)
})
})
Loading