Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sheet] Add required aria label #3852

Merged
merged 20 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f

### Breaking changes

- Remove `light` property from `Tooltip` as it now defaults to a light background ([#3803](https://github.com/Shopify/polaris-react/pull/3803)) ([#3803](https://github.com/Shopify/polaris-react/pull/3803))
- Made `title` property required in `Modal` for accessibility label, added `hideTitle` property ([#3803](https://github.com/Shopify/polaris-react/pull/3803)) ([#3803](https://github.com/Shopify/polaris-react/pull/3803))
- Remove `light` property from `Tooltip` as it now defaults to a light background ([#3846](https://github.com/Shopify/polaris-react/pull/3846))
- Made `title` property required in `Modal` for accessibility label, added `hideTitle` property ([#3803](https://github.com/Shopify/polaris-react/pull/3803))
Comment on lines +7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

馃敟?

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed the links where there twice, so I removed the duplicated links:

(#3803) (#3803)

- Added required `ariaLabel` property in `Sheet` ([#3852](https://github.com/Shopify/polaris-react/pull/3852))

### Enhancements

Expand Down
2 changes: 2 additions & 0 deletions src/components/Filters/Filters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ class FiltersInner extends Component<CombinedProps, State> {

const filtersContainerMarkup = isNavigationCollapsed ? (
<Sheet
accessibilityLabel={moreFiltersLabel}
open={open}
onClose={this.closeFilters}
onEntered={this.setReadyForFocus(true)}
Expand All @@ -403,6 +404,7 @@ class FiltersInner extends Component<CombinedProps, State> {
</Sheet>
) : (
<Sheet
accessibilityLabel={moreFiltersLabel}
open={open}
onClose={this.closeFilters}
onEntered={this.setReadyForFocus(true)}
Expand Down
6 changes: 5 additions & 1 deletion src/components/Sheet/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ function SheetExample() {
>
{salesChannelsCardMarkup}
</Card>
<Sheet open={sheetActive} onClose={toggleSheetActive}>
<Sheet
open={sheetActive}
onClose={toggleSheetActive}
ariaLabel="Manage sales channels"
>
<div
style={{
display: 'flex',
Expand Down
10 changes: 9 additions & 1 deletion src/components/Sheet/Sheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export interface SheetProps {
onEntered?(): void;
/** Callback when the sheet has started to exit */
onExit?(): void;
/** ARIA label for sheet */
accessibilityLabel: string;
}

export function Sheet({
Expand All @@ -46,6 +48,7 @@ export function Sheet({
onClose,
onEntered,
onExit,
accessibilityLabel,
}: SheetProps) {
const {isNavigationCollapsed} = useMediaQuery();
const container = useRef<HTMLDivElement>(null);
Expand All @@ -71,7 +74,12 @@ export function Sheet({
ref={container}
>
<TrapFocus trapping={open}>
<div role="dialog" tabIndex={-1} className={styles.Sheet}>
<div
role="dialog"
tabIndex={-1}
className={styles.Sheet}
aria-label={accessibilityLabel}
alex-page marked this conversation as resolved.
Show resolved Hide resolved
>
{children}
</div>
</TrapFocus>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Sheet/tests/Sheet.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('<Sheet />', () => {
const mockProps = {
open: true,
onClose: noop,
accessibilityLabel: 'abc',
accessibilityLabel: 'More filters',
};

it('renders its children', () => {
Expand Down