Switch: Improve screen reader behaviour (#95178)

* add role="switch" and only set a label if one is passed in

* fix unit tests

* fix more unit tests
This commit is contained in:
Ashley Harrison 2024-10-24 11:26:04 +01:00 committed by GitHub
parent 142797032b
commit 4a9ba41f81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 39 additions and 38 deletions

View File

@ -391,7 +391,7 @@ describe('Switch, as AutoSaveField child, ', () => {
setupSwitch();
//Is there another way to find the switch element? Filtering by name doesn't work
expect(
screen.getByRole('checkbox', {
screen.getByRole('switch', {
checked: false,
})
).toBeInTheDocument();

View File

@ -29,13 +29,13 @@ export const Controlled: StoryFn<typeof Switch> = (args) => {
<div>
<div style={{ marginBottom: '32px' }}>
<Field label="Normal switch" description="For horizontal forms" invalid={args.invalid}>
<Switch value={args.value} disabled={args.disabled} />
<Switch value={args.value} disabled={args.disabled} id="normal-switch" />
</Field>
</div>
<div style={{ marginBottom: '32px' }}>
<InlineFieldRow>
<InlineField label="My switch" invalid={args.invalid} disabled={args.disabled}>
<InlineSwitch value={args.value} />
<InlineSwitch value={args.value} id="my-switch" />
</InlineField>
</InlineFieldRow>
</div>

View File

@ -27,6 +27,7 @@ export const Switch = forwardRef<HTMLInputElement, Props>(
<div className={cx(styles.switch, invalid && styles.invalid)}>
<input
type="checkbox"
role="switch"
disabled={disabled}
checked={value}
onChange={(event) => {
@ -36,7 +37,7 @@ export const Switch = forwardRef<HTMLInputElement, Props>(
{...inputProps}
ref={ref}
/>
<label htmlFor={switchIdRef.current} aria-label={label ?? 'Toggle switch'}>
<label htmlFor={switchIdRef.current} aria-label={label}>
<Icon name="check" size="xs" />
</label>
</div>

View File

@ -1,6 +1,6 @@
import { noop } from 'lodash';
import { render } from 'test/test-utils';
import { byRole } from 'testing-library-selector';
import { byLabelText, byRole } from 'testing-library-selector';
import { Button } from '@grafana/ui';
import { setupMswServer } from 'app/features/alerting/unified/mockApi';
@ -16,7 +16,7 @@ import { AmRoutesExpandedForm } from './EditNotificationPolicyForm';
const ui = {
error: byRole('alert'),
overrideTimingsCheckbox: byRole('checkbox', { name: /Override general timings/ }),
overrideTimingsSwitch: byLabelText(/Override general timings/),
submitBtn: byRole('button', { name: /Update default policy/ }),
groupWaitInput: byRole('textbox', { name: /Group wait/ }),
groupIntervalInput: byRole('textbox', { name: /Group interval/ }),
@ -42,7 +42,7 @@ describe('EditNotificationPolicyForm', function () {
repeat_interval: '1w2d6h',
});
expect(ui.overrideTimingsCheckbox.get()).toBeChecked();
expect(ui.overrideTimingsSwitch.get()).toBeChecked();
expect(ui.groupWaitInput.get()).toHaveValue('1m30s');
expect(ui.groupIntervalInput.get()).toHaveValue('2d4h30m35s');
expect(ui.repeatIntervalInput.get()).toHaveValue('1w2d6h');
@ -58,7 +58,7 @@ describe('EditNotificationPolicyForm', function () {
onSubmit
);
await user.click(ui.overrideTimingsCheckbox.get());
await user.click(ui.overrideTimingsSwitch.get());
await user.type(ui.groupWaitInput.get(), '5m25s');
await user.type(ui.groupIntervalInput.get(), '35m40s');
@ -88,7 +88,7 @@ describe('EditNotificationPolicyForm', function () {
onSubmit
);
await user.click(ui.overrideTimingsCheckbox.get());
await user.click(ui.overrideTimingsSwitch.get());
await user.type(ui.groupWaitInput.get(), '5m25s');
await user.type(ui.groupIntervalInput.get(), '35m40s');

View File

@ -91,8 +91,8 @@ describe('ProviderConfigForm', () => {
expect(screen.getByRole('textbox', { name: /Client ID/i })).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: /Client secret/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Scopes/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Allow Sign Up/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Auto login/i })).toBeInTheDocument();
expect(screen.getByLabelText(/Allow Sign Up/i)).toBeInTheDocument();
expect(screen.getByLabelText(/Auto login/i)).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: /Sign out redirect URL/i })).toBeInTheDocument();
expect(screen.getByRole('button', { name: /Save/i })).toBeInTheDocument();
expect(screen.getByRole('link', { name: /Discard/i })).toBeInTheDocument();
@ -102,8 +102,8 @@ describe('ProviderConfigForm', () => {
const { user } = setup(<ProviderConfigForm config={testConfig} provider={testConfig.provider} />);
await user.click(screen.getByText('User mapping'));
expect(screen.getByRole('textbox', { name: /Role attribute path/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Role attribute strict mode/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Skip organization role sync/i })).toBeInTheDocument();
expect(screen.getByLabelText(/Role attribute strict mode/i)).toBeInTheDocument();
expect(screen.getByLabelText(/Skip organization role sync/i)).toBeInTheDocument();
});
it('renders all extra security fields correctly', async () => {
@ -114,7 +114,7 @@ describe('ProviderConfigForm', () => {
expect(screen.getByRole('combobox', { name: /Team Ids/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Use PKCE/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Use refresh token/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /TLS skip verify/i })).toBeInTheDocument();
expect(screen.getByLabelText(/TLS skip verify/i)).toBeInTheDocument();
});
it('should save and enable on form submit', async () => {
@ -124,11 +124,11 @@ describe('ProviderConfigForm', () => {
await user.type(screen.getByLabelText(/Client secret/i), 'test-client-secret');
// Type a scope and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Scopes/i }), 'user:email{enter}');
await user.click(screen.getByRole('checkbox', { name: /Auto login/i }));
await user.click(screen.getByLabelText(/Auto login/i));
await user.click(screen.getByText('User mapping'));
await user.type(screen.getByRole('textbox', { name: /Role attribute path/i }), 'new-attribute-path');
await user.click(screen.getByRole('checkbox', { name: /Role attribute strict mode/i }));
await user.click(screen.getByLabelText(/Role attribute strict mode/i));
await user.type(screen.getByRole('combobox', { name: /Organization mapping/i }), 'Group A:1:Editor{enter}');
await user.type(screen.getByRole('combobox', { name: /Organization mapping/i }), 'Group B:2:Admin{enter}');
@ -185,7 +185,7 @@ describe('ProviderConfigForm', () => {
await user.type(screen.getByLabelText(/Client secret/i), 'test-client-secret');
// Type a scope and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Scopes/i }), 'user:email{enter}');
await user.click(screen.getByRole('checkbox', { name: /Auto login/i }));
await user.click(screen.getByLabelText(/Auto login/i));
await user.click(screen.getByText('Save'));
await waitFor(() => {

View File

@ -20,6 +20,6 @@ describe('<BasicSettings>', () => {
setup();
expect(screen.getByTestId(selectors.pages.DataSource.name)).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Default/ })).toBeInTheDocument();
expect(screen.getByLabelText(/Default/)).toBeInTheDocument();
});
});

View File

@ -25,9 +25,9 @@ describe('RichHistorySettings', () => {
});
it('should render component with correctly checked starredTabAsFirstTab and uncheched toggleactiveDatasourcesOnly settings', () => {
setup();
const checkboxes = screen.getAllByRole('checkbox');
expect(checkboxes.length).toBe(2);
expect(checkboxes[0]).toHaveAttribute('checked');
expect(checkboxes[1]).not.toHaveAttribute('checked');
const switches = screen.getAllByRole('switch');
expect(switches.length).toBe(2);
expect(switches[0]).toHaveAttribute('checked');
expect(switches[1]).not.toHaveAttribute('checked');
});
});

View File

@ -125,7 +125,7 @@ describe('TraceViewContainer', () => {
await user.click(tagOption);
expect(screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' }).length).toBe(3);
const matchesSwitch = screen.getByRole('checkbox', { name: 'Show matches only switch' });
const matchesSwitch = screen.getByRole('switch', { name: 'Show matches only switch' });
expect(matchesSwitch).toBeInTheDocument();
await user.click(matchesSwitch);
expect(screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' }).length).toBe(1);

View File

@ -54,7 +54,7 @@ describe('<TracePageSearchBar>', () => {
it('renders show span filter matches only switch', async () => {
render(<TracePageSearchBarWithProps matches={[]} />);
const matchesSwitch = screen.getByRole('checkbox', { name: 'Show matches only switch' });
const matchesSwitch = screen.getByRole('switch', { name: 'Show matches only switch' });
expect(matchesSwitch).toBeInTheDocument();
});
});

View File

@ -260,7 +260,7 @@ describe('SpanFilters', () => {
await selectAndCheckValue(user, tagKey, 'TagKey0');
await selectAndCheckValue(user, tagValue, 'TagValue0');
const matchesSwitch = screen.getByRole('checkbox', { name: 'Show matches only switch' });
const matchesSwitch = screen.getByRole('switch', { name: 'Show matches only switch' });
expect(matchesSwitch).not.toBeChecked();
await user.click(matchesSwitch);
expect(matchesSwitch).toBeChecked();

View File

@ -75,9 +75,9 @@ export const switchToQueryHistoryTab = async (name: 'Settings' | 'Query History'
};
export const selectStarredTabFirst = async () => {
const checkbox = withinQueryHistory().getByRole('checkbox', {
name: /Change the default active tab from “Query history” to “Starred”/,
});
const checkbox = withinQueryHistory().getByLabelText(
/Change the default active tab from “Query history” to “Starred”/
);
await userEvent.click(checkbox);
};

View File

@ -4,10 +4,10 @@ import userEvent from '@testing-library/user-event';
import { LogContextButtons } from './LogContextButtons';
describe('LogContextButtons', () => {
it('should call onChangeWrapLines when the checkbox is used, case 1', async () => {
it('should call onChangeWrapLines when the switch is used, case 1', async () => {
const onChangeWrapLines = jest.fn();
render(<LogContextButtons onChangeWrapLines={onChangeWrapLines} onScrollCenterClick={jest.fn()} />);
const wrapLinesBox = screen.getByRole('checkbox', {
const wrapLinesBox = screen.getByRole('switch', {
name: 'Wrap lines',
});
await userEvent.click(wrapLinesBox);
@ -15,10 +15,10 @@ describe('LogContextButtons', () => {
expect(onChangeWrapLines).toHaveBeenCalledWith(true);
});
it('should call onChangeWrapLines when the checkbox is used, case 2', async () => {
it('should call onChangeWrapLines when the switch is used, case 2', async () => {
const onChangeWrapLines = jest.fn();
render(<LogContextButtons onChangeWrapLines={onChangeWrapLines} onScrollCenterClick={jest.fn()} wrapLines />);
const wrapLinesBox = screen.getByRole('checkbox', {
const wrapLinesBox = screen.getByRole('switch', {
name: 'Wrap lines',
});
await userEvent.click(wrapLinesBox);

View File

@ -28,7 +28,7 @@ describe('DataLinks tests', () => {
setup({ value: testValue });
expect(await screen.findAllByRole('button', { name: 'Remove field' })).toHaveLength(2);
expect(await screen.findAllByRole('checkbox', { name: 'Internal link' })).toHaveLength(2);
expect(await screen.findAllByLabelText('Internal link')).toHaveLength(2);
});
it('should call onChange to add a new field when the add button is clicked', async () => {

View File

@ -232,7 +232,7 @@ describe('LokiContextUi', () => {
window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true');
render(<LokiContextUi {...newProps} />);
await waitFor(() => {
expect((screen.getByRole('checkbox') as HTMLInputElement).checked).toBe(true);
expect((screen.getByRole('switch') as HTMLInputElement).checked).toBe(true);
});
});
@ -248,7 +248,7 @@ describe('LokiContextUi', () => {
window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'false');
render(<LokiContextUi {...newProps} />);
await waitFor(() => {
expect((screen.getByRole('checkbox') as HTMLInputElement).checked).toBe(false);
expect((screen.getByRole('switch') as HTMLInputElement).checked).toBe(false);
});
});
@ -265,7 +265,7 @@ describe('LokiContextUi', () => {
window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true');
render(<LokiContextUi {...newProps} />);
await waitFor(() => {
expect(screen.getByRole('checkbox')).toBeInTheDocument();
expect(screen.getByRole('switch')).toBeInTheDocument();
});
});
@ -282,7 +282,7 @@ describe('LokiContextUi', () => {
window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true');
render(<LokiContextUi {...newProps} />);
await waitFor(() => {
expect(screen.queryByRole('checkbox')).toBeNull();
expect(screen.queryByRole('switch')).toBeNull();
});
});

View File

@ -16,7 +16,7 @@ describe('AlertingSettings', () => {
it('should update alerting settings', async () => {
const onChange = jest.fn();
render(<AlertingSettings options={options} onOptionsChange={onChange} />);
await userEvent.click(screen.getByLabelText('Toggle switch'));
await userEvent.click(screen.getByRole('switch'));
expect(onChange).toHaveBeenCalledTimes(1);
});
});