This repository was archived by the owner on Mar 4, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
fix(Loader): Adding labeling when loader get focus #2352
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
033bbc4
adding labelling when loader get focus
mituron 73dd54e
Update docs/src/examples/components/Loader/BestPractices/LoaderBestPr…
kolaps33 b9a6c57
Update packages/react/src/components/Loader/Loader.tsx
kolaps33 25c6edf
changes after review
mituron c6135be
prettier applied
mituron 79666c2
changelog update
mituron 313ca72
reducing the code, apply fix from review
mituron 6dfa16d
Merge branch 'master' into mituron/focusable-loader
kolaps33 79c753c
lint fix
mituron 7eb176e
Merge branch 'mituron/focusable-loader' of https://github.com/microso…
mituron f3daa85
fix unit test
mituron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
docs/src/examples/components/Loader/BestPractices/LoaderBestPractices.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import * as React from 'react' | ||
|
||
import ComponentBestPractices from '../../../../components/ComponentBestPractices' | ||
|
||
const doList = [ | ||
'Use react-aria-live or similar component to announce the loading state.', | ||
'If loader is only the element in the screen or region, consider making it focusable by setting `tabIndex` prop to the `Loader`. In most of these cases value of `tabIndex` should be 0.', | ||
] | ||
|
||
const LoaderBestPractices: React.FunctionComponent<{}> = () => { | ||
return <ComponentBestPractices doList={doList} /> | ||
} | ||
|
||
export default LoaderBestPractices |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
packages/accessibility/test/behaviors/loaderBehavior-test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import { loaderBehavior } from '@fluentui/accessibility' | ||
|
||
describe('LoaderBehavior.ts', () => { | ||
test('do NOT add aria-labelledby, when aria-label was set already', () => { | ||
const props = { labelId: 'label-id', 'aria-label': 'any loading string' } | ||
const expectedResult = loaderBehavior(props) | ||
expect(expectedResult.attributes.root['aria-labelledby']).toEqual(undefined) | ||
}) | ||
|
||
test('do NOT add aria-labelledby, when aria-labelled was set already', () => { | ||
const props = { labelId: 'label-id', 'aria-labelledby': 'id' } | ||
const expectedResult = loaderBehavior(props) | ||
expect(expectedResult.attributes.root['aria-labelledby']).toEqual(undefined) | ||
}) | ||
|
||
test('do NOT add aria-labelledby, when there is no tabIndex specified', () => { | ||
const props = { labelId: 'label-id' } | ||
const expectedResult = loaderBehavior(props) | ||
expect(expectedResult.attributes.root['aria-labelledby']).toEqual(undefined) | ||
}) | ||
|
||
test('add aria-labelledby, when there is tabIndex=0 specified', () => { | ||
const props = { labelId: 'label-id', tabIndex: 0 } | ||
const expectedResult = loaderBehavior(props) | ||
expect(expectedResult.attributes.root['aria-labelledby']).toEqual('label-id') | ||
}) | ||
|
||
test('add aria-labelledby, when there is tabIndex=-1 specified', () => { | ||
const props = { labelId: 'label-id', tabIndex: -1 } | ||
const expectedResult = loaderBehavior(props) | ||
expect(expectedResult.attributes.root['aria-labelledby']).toEqual('label-id') | ||
}) | ||
}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import { | |
commonPropTypes, | ||
SizeValue, | ||
ShorthandFactory, | ||
getOrGenerateIdFromShorthand, | ||
} from '../../utils' | ||
import { WithAsProp, ShorthandValue, withSafeTypeForAs } from '../../types' | ||
import Box, { BoxProps } from '../Box/Box' | ||
|
@@ -49,6 +50,7 @@ export interface LoaderProps extends UIComponentProps { | |
|
||
export interface LoaderState { | ||
visible: boolean | ||
labelId: string | ||
} | ||
|
||
/** | ||
|
@@ -94,6 +96,13 @@ class Loader extends UIComponent<WithAsProp<LoaderProps>, LoaderState> { | |
|
||
this.state = { | ||
visible: this.props.delay === 0, | ||
labelId: '', | ||
} | ||
} | ||
|
||
static getDerivedStateFromProps(props, state) { | ||
return { | ||
labelId: getOrGenerateIdFromShorthand('loader-label-', props.label, state.labelId), | ||
} | ||
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. Why don't use 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. do you mean 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.
|
||
} | ||
|
||
|
@@ -114,7 +123,7 @@ class Loader extends UIComponent<WithAsProp<LoaderProps>, LoaderState> { | |
|
||
renderComponent({ ElementType, classes, accessibility, variables, styles, unhandledProps }) { | ||
const { indicator, label, svg } = this.props | ||
const { visible } = this.state | ||
const { visible, labelId } = this.state | ||
|
||
const svgElement = Box.create(svg, { | ||
defaultProps: () => ({ className: Loader.slotClassNames.svg, styles: styles.svg }), | ||
|
@@ -135,7 +144,11 @@ class Loader extends UIComponent<WithAsProp<LoaderProps>, LoaderState> { | |
}), | ||
})} | ||
{Text.create(label, { | ||
defaultProps: () => ({ className: Loader.slotClassNames.label, styles: styles.label }), | ||
defaultProps: () => ({ | ||
className: Loader.slotClassNames.label, | ||
styles: styles.label, | ||
id: labelId, | ||
}), | ||
})} | ||
</ElementType> | ||
) | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Let's type this method as in
Dialog