Skip to content

Commit

Permalink
[No Jira] Fix a11y issue for BpkSelectableChip (#2994)
Browse files Browse the repository at this point in the history 
Co-authored-by: anushree bagchi <anushreebagchi@skyscanner.net>
  • Loading branch information
@AnushreeBagchi
AnushreeBagchi and anushree bagchi authored Sep 5, 2023
1 parent 6623ab9 commit 0450ad3
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 45 deletions.
101 changes: 57 additions & 44 deletions packages/bpk-component-chip/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,62 +9,74 @@ Check the main [Readme](https://github.com/skyscanner/backpack#usage) for a comp
## Usage

```tsx
import BpkSelectableChip, { BpkDismissibleChip, BpkDropdownChip, CHIP_TYPES } from '@skyscanner/backpack-web/bpk-component-chip';
import BpkSelectableChip, {
BpkDismissibleChip,
BpkDropdownChip,
CHIP_TYPES,
} from '@skyscanner/backpack-web/bpk-component-chip';
import BeachIconSm from '@skyscanner/backpack-web/bpk-component-icon/sm/beach';

export default () => (

<div style={{ display: 'flex' }}> // IMPORTANT: Flex styles make sure chips align with each other
// Standard selectable chip.
<div style={{ display: 'flex' }}>
{' '}
// IMPORTANT: Flex styles make sure chips align with each other // Standard selectable
chip.
<BpkSelectableChip
accessibilityLabel="Press to toggle chip"
selected={false}
onClick={() => { /* Use state to set 'selected={true}' */ }}
onClick={() => {
/* Use state to set 'selected={true}' */
}}
>
Toggle me
</BpkSelectableChip>

// Selectable chip with an icon.
<BpkSelectableChip
accessibilityLabel="Press to toggle chip"
selected={false}
onClick={() => { /* Use state to set 'selected={true}' */ }}
onClick={() => {
/* Use state to set 'selected={true}' */
}}
leadingAccessoryView={<BeachIconSm />}
>
Toggle me
</BpkSelectableChip>

// Standard dropdown chip.
<BpkDropdownChip
accessibilityLabel="Press to toggle chip"
selected={false}
onClick={() => { /* Use state to set 'selected={true}' */ }}
onClick={() => {
/* Use state to set 'selected={true}' */
}}
>
Toggle me
</BpkDropdownChip>

// Dropdown chip with an icon.
<BpkDropdownChip
accessibilityLabel="Press to toggle chip"
selected={false}
onClick={() => { /* Use state to set 'selected={true}' */ }}
onClick={() => {
/* Use state to set 'selected={true}' */
}}
leadingAccessoryView={<BeachIconSm />}
>
Toggle me
</BpkDropdownChip>

// Standard dismissible chip.
<BpkDismissibleChip
accessibilityLabel="Press to dismiss chip"
onClick={() => { /* Use state to handle removing this chip. */ }}
onClick={() => {
/* Use state to handle removing this chip. */
}}
>
Dismiss me
</BpkDismissibleChip>

// Dismissible chip with an icon.
<BpkDismissibleChip
<BpkDismissibleChip
accessibilityLabel="Press to dismiss chip"
onClick={() => { /* Use state to handle removing this chip. */ }}
onClick={() => {
/* Use state to handle removing this chip. */
}}
leadingAccessoryView={<BeachIconSm />}
>
Dismiss me
Expand All @@ -77,42 +89,43 @@ export default () => (

### BpkSelectableChip

| Property | PropType | Required | Default Value |
| --------------------- | --------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| disabled | bool | false | false |
| leadingAccessoryView | node | false | null |
| selected | bool | false | false |
| trailingAccessoryView | node | false | null |
| type | oneOf(`CHIP_TYPES.onDark`, `CHIP_TYPES.default`, `CHIP_TYPES.onImage`) |
| Property | PropType | Required | Default Value |
| --------------------- | ---------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| disabled | bool | false | false |
| leadingAccessoryView | node | false | null |
| selected | bool | false | false |
| trailingAccessoryView | node | false | null |
| type | oneOf(`CHIP_TYPES.onDark`, `CHIP_TYPES.default`, `CHIP_TYPES.onImage`) | | |
| role | string | false | checkbox |

### BpkDropdownChip

| Property | PropType | Required | Default Value |
| --------------------- | --------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| disabled | bool | false | false |
| leadingAccessoryView | node | false | null |
| selected | bool | false | false |
| type | oneOf(`CHIP_TYPES.onDark`, `CHIP_TYPES.default`, `CHIP_TYPES.onImage`) |
| Property | PropType | Required | Default Value |
| -------------------- | ---------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| disabled | bool | false | false |
| leadingAccessoryView | node | false | null |
| selected | bool | false | false |
| type | oneOf(`CHIP_TYPES.onDark`, `CHIP_TYPES.default`, `CHIP_TYPES.onImage`) |

### BpkDismissibleChip

Dismissible chips are selectable chips that have been preconfigured to have a 'close' icon trailing accessory view and cannot be selected, so they have the same props as `BpkSelectableChip`, minus `trailingAccessoryView` and `selected`.

| Property | PropType | Required | Default Value |
| -------------------- | --------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| leadingAccessoryView | node | false | null |
| Property | PropType | Required | Default Value |
| -------------------- | ---------------------------------------------------------------------- | -------- | ------------- |
| accessibilityLabel | string | true | - |
| children | node | true | - |
| onClick | func | true | - |
| className | string | false | null |
| leadingAccessoryView | node | false | null |
| type | oneOf(`CHIP_TYPES.onDark`, `CHIP_TYPES.default`, `CHIP_TYPES.onImage`) |

## Theme Props
Expand Down
2 changes: 1 addition & 1 deletion packages/bpk-component-chip/src/BpkSelectableChip.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const BpkSelectableChip = ({

return (
<button
aria-checked={role === 'button' ? undefined : selected}
aria-checked={role === 'button' || role === 'tab' ? undefined : selected}
className={classNames}
disabled={disabled}
role={role}
Expand Down
16 changes: 16 additions & 0 deletions packages/bpk-component-chip/src/accessibility-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,20 @@ describe('BpkSelectableChip accessibility tests', () => {
const results = await axe(container);
expect(results).toHaveNoViolations();
});

it('should not have programmatically-detectable accessibility issues when role=tab', async () => {
const { container } = render(
<div role="tablist">
<BpkSelectableChip
onClick={() => null}
accessibilityLabel="Tab"
role="tab"
>
Toggle me
</BpkSelectableChip>
</div>,
);
const results = await axe(container);
expect(results).toHaveNoViolations();
});
});

0 comments on commit 0450ad3

Please sign in to comment.