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

Commit 884067c

Browse files
authored
fix(Carousel): improvement for windows screen readers experiences (#2223)
* first attempt, different fixes * some fix * addings specification for carouselItemBehavior * changing to aria label as voiceover has some issue * removing not necessary changes * removing empty line * adding prop navigation into carouselItem * adress fix from review
1 parent 37e6c6b commit 884067c

File tree

5 files changed

+34
-3
lines changed

5 files changed

+34
-3
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import * as React from 'react'
2+
import { Text } from '@fluentui/react'
3+
import { link } from '../../../../utils/helpers'
4+
5+
import ComponentBestPractices from 'docs/src/components/ComponentBestPractices'
6+
7+
const doList = [
8+
<Text>
9+
'Add textual representation for `CarouselItem`. Use `aria-label` attribute ( refer to{' '}
10+
{link('reported issue', 'https://bugs.chromium.org/p/chromium/issues/detail?id=1040924')} for
11+
details).
12+
</Text>,
13+
]
14+
15+
const CarouselBestPractices: React.FunctionComponent<{}> = () => {
16+
return <ComponentBestPractices doList={doList} />
17+
}
18+
19+
export default CarouselBestPractices

docs/src/examples/components/Carousel/Types/CarouselExample.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,25 @@ const carouselItems = [
1212
key: 'ade',
1313
id: 'ade',
1414
content: <Image src="public/images/avatar/large/ade.jpg" fluid alt={imageAltTags.ade} />,
15+
'aria-label': imageAltTags.ade,
1516
},
1617
{
1718
key: 'elliot',
1819
id: 'elliot',
1920
content: <Image src="public/images/avatar/large/elliot.jpg" fluid alt={imageAltTags.elliot} />,
21+
'aria-label': imageAltTags.elliot,
2022
},
2123
{
2224
key: 'kristy',
2325
id: 'kristy',
2426
content: <Image src="public/images/avatar/large/kristy.png" fluid alt={imageAltTags.kristy} />,
27+
'aria-label': imageAltTags.kristy,
2528
},
2629
{
2730
key: 'nan',
2831
id: 'nan',
2932
content: <Image src="public/images/avatar/large/nan.jpg" fluid alt={imageAltTags.nan} />,
33+
'aria-label': imageAltTags.nan,
3034
},
3135
]
3236

packages/accessibility/src/behaviors/Carousel/carouselItemBehavior.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ import { Accessibility } from '../../types'
22

33
/**
44
* @specification
5-
* Adds attribute 'role=tabpanel' to 'root' slot.
5+
* Adds attribute 'role=tabpanel' to 'root' slot if 'navigation' property is true. Sets the attribute to 'group' otherwise.
66
* Adds attribute 'aria-hidden=false' to 'root' slot if 'active' property is true. Sets the attribute to 'true' otherwise.
77
* Adds attribute 'tabIndex=0' to 'root' slot if 'active' property is true. Sets the attribute to '-1' otherwise.
88
*/
99
const carouselItemBehavior: Accessibility<CarouselItemProps> = props => ({
1010
attributes: {
1111
root: {
12-
role: 'tabpanel',
12+
role: props.navigation ? 'tabpanel' : 'group',
1313
'aria-hidden': props.active ? 'false' : 'true',
1414
tabIndex: props.active ? 0 : -1,
1515
},
@@ -25,4 +25,5 @@ export default carouselItemBehavior
2525
export type CarouselItemProps = {
2626
/** If item is visible in the carousel. */
2727
active?: boolean
28+
navigation?: boolean
2829
}

packages/react/src/components/Carousel/Carousel.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ class Carousel extends AutoControlledComponent<WithAsProp<CarouselProps>, Carous
278278
defaultProps: () => ({
279279
active: activeIndex === index,
280280
id: itemIds[index],
281+
navigation: !!this.props.navigation,
281282
...(getItemPositionText && {
282283
itemPositionText: getItemPositionText(index, items.length),
283284
}),
@@ -428,5 +429,7 @@ Carousel.create = createShorthandFactory({
428429
*
429430
* @accessibility
430431
* Implements [ARIA Carousel](https://www.w3.org/WAI/tutorials/carousels/structure/) design pattern.
432+
* @accessibilityIssues
433+
* [VoiceOver doens't narrate label referenced by aria-labelledby attribute, when role is "tabpanel"](https://bugs.chromium.org/p/chromium/issues/detail?id=1040924)
431434
*/
432435
export default withSafeTypeForAs<typeof Carousel, CarouselProps, 'div'>(Carousel)

packages/react/src/components/Carousel/CarouselItem.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ export interface CarouselItemProps
3232
* up by screen readers.
3333
*/
3434
itemPositionText?: string
35+
36+
/** Whether or not navigation exists in carousel. */
37+
navigation?: boolean
3538
}
3639

3740
class CarouselItem extends UIComponent<WithAsProp<CarouselItemProps>> {
@@ -44,6 +47,7 @@ class CarouselItem extends UIComponent<WithAsProp<CarouselItemProps>> {
4447
static propTypes = {
4548
...commonPropTypes.createCommon(),
4649
active: PropTypes.bool,
50+
navigation: PropTypes.bool,
4751
itemPositionText: PropTypes.string,
4852
}
4953

@@ -64,13 +68,13 @@ class CarouselItem extends UIComponent<WithAsProp<CarouselItemProps>> {
6468
{...unhandledProps}
6569
{...applyAccessibilityKeyHandlers(accessibility.keyHandlers.root, unhandledProps)}
6670
>
71+
{childrenExist(children) ? children : content}
6772
<div
6873
className={CarouselItem.slotClassNames.itemPositionText}
6974
style={screenReaderContainerStyles}
7075
>
7176
{itemPositionText}
7277
</div>
73-
{childrenExist(children) ? children : content}
7478
</ElementType>
7579
)
7680
}

0 commit comments

Comments
 (0)