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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use a centralized announcer to make aria-live announcements #2128

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pages/live-region-content-test.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function LiveRegionContentTestPage() {
<strong>Live region</strong>

<div style={{ padding: 8, border: '1px solid black' }}>
<LiveRegion visible={true}>
<LiveRegion>
<article>
<div>Before list</div>
<ul>
Expand Down
2 changes: 1 addition & 1 deletion pages/live-region.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default function LiveRegionXSS() {
return (
<>
<h1>Live region</h1>
<LiveRegion delay={0}>
<LiveRegion hidden={true}>
{`<p>Testing</p>`}
<p>Testing</p>
</LiveRegion>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ exports[`test-utils selectors 1`] = `
"awsui_root_1kjc7",
"awsui_root_1qprf",
"awsui_root_1t44z",
"awsui_root_3bgfn",
"awsui_root_qwoo0",
"awsui_root_vrgzu",
"awsui_selectable-item_15o6u",
Expand Down
6 changes: 3 additions & 3 deletions src/attribute-editor/additional-info.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
import React from 'react';

import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';

import styles from './styles.css.js';

Expand All @@ -12,9 +12,9 @@ interface AdditionalInfoProps {
}

export const AdditionalInfo = ({ children, id }: AdditionalInfoProps) => (
<LiveRegion visible={true} tagName="div" data-testid="info-live-region">
<InternalLiveRegion data-testid="info-live-region">
<div id={id} className={styles['additional-info']}>
{children}
</div>
</LiveRegion>
</InternalLiveRegion>
);
12 changes: 9 additions & 3 deletions src/attribute-editor/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import InternalBox from '../box/internal';
import { ButtonProps } from '../button/interfaces';
import { InternalButton } from '../button/internal';
import { getBaseProps } from '../internal/base-component';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { useContainerBreakpoints } from '../internal/hooks/container-queries';
import { InternalBaseComponentProps } from '../internal/hooks/use-base-component';
import { useMergeRefs } from '../internal/hooks/use-merge-refs';
Expand Down Expand Up @@ -110,9 +110,15 @@ const InternalAttributeEditor = React.forwardRef(
>
{addButtonText}
</InternalButton>
<LiveRegion data-testid="removal-announcement" delay={5} key={items.length}>
<InternalLiveRegion
data-testid="removal-announcement"
tagName="span"
hidden={true}
delay={5}
key={items.length}
>
{removalAnnouncement}
</LiveRegion>
</InternalLiveRegion>
{!!additionalInfo && <AdditionalInfo id={infoAriaDescribedBy}>{additionalInfo}</AdditionalInfo>}
</div>
);
Expand Down
7 changes: 5 additions & 2 deletions src/button-group/icon-button-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { warnOnce } from '@cloudscape-design/component-toolkit/internal';

import { ButtonProps } from '../button/interfaces.js';
import { InternalButton } from '../button/internal.js';
import LiveRegion from '../internal/components/live-region/index.js';
import InternalLiveRegion from '../internal/components/live-region/internal';
import Tooltip from '../internal/components/tooltip/index.js';
import { CancelableEventHandler, ClickDetail } from '../internal/events/index.js';
import { ButtonGroupProps } from './interfaces.js';
Expand Down Expand Up @@ -60,7 +60,10 @@ const IconButtonItem = forwardRef(
<Tooltip
trackRef={containerRef}
trackKey={item.id}
value={(showFeedback && <LiveRegion visible={true}>{item.popoverFeedback}</LiveRegion>) || item.text}
value={
(showFeedback && <InternalLiveRegion tagName="span">{item.popoverFeedback}</InternalLiveRegion>) ||
item.text
}
className={clsx(testUtilStyles.tooltip, testUtilStyles['button-group-tooltip'])}
/>
)}
Expand Down
14 changes: 14 additions & 0 deletions src/button/__tests__/button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { act, fireEvent, render } from '@testing-library/react';
import Button, { ButtonProps } from '../../../lib/components/button';
import InternalButton from '../../../lib/components/button/internal';
import createWrapper, { ButtonWrapper } from '../../../lib/components/test-utils/dom';
import LiveRegionWrapper from '../../../lib/components/test-utils/selectors/internal/live-region';
import { buttonRelExpectations, buttonTargetExpectations } from '../../__tests__/target-rel-test-helper';
import { renderWithSingleTabStopNavigation } from '../../internal/context/__tests__/utils';

Expand Down Expand Up @@ -60,6 +61,19 @@ describe('Button Component', () => {
expect(document.activeElement).toBe(wrapper.findButton()!.getElement());
});

describe('loadingText property', () => {
test('renders loadingText in a LiveRegion', () => {
renderButton({ children: 'Button', loading: true, loadingText: 'Loading' });
expect(createWrapper().findByClassName(LiveRegionWrapper.rootSelector)!.getElement()).toHaveTextContent(
'Loading'
);
});
test('does not render loadingText if loading is false', () => {
renderButton({ children: 'Button', loading: false, loadingText: 'Loading' });
expect(createWrapper().findByClassName(LiveRegionWrapper.rootSelector)).toBeNull();
});
});

describe('disabled property', () => {
test('renders button with normal styling by default', () => {
const wrapper = renderButton();
Expand Down
14 changes: 11 additions & 3 deletions src/button/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
getSubStepAllSelector,
getTextFromSelector,
} from '../internal/analytics/selectors';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import Tooltip from '../internal/components/tooltip/index.js';
import { useButtonContext } from '../internal/context/button-context';
import { useSingleTabStopNavigation } from '../internal/context/single-tab-stop-navigation-context';
Expand Down Expand Up @@ -250,7 +250,11 @@
>
{buttonContent}
</a>
{loading && loadingText && <LiveRegion>{loadingText}</LiveRegion>}
{loading && loadingText && (
<InternalLiveRegion tagName="span" hidden={true}>

Check warning on line 254 in src/button/internal.tsx

View check run for this annotation

Codecov / codecov/patch

src/button/internal.tsx#L254

Added line #L254 was not covered by tests
{loadingText}
</InternalLiveRegion>
)}
</>
);
}
Expand Down Expand Up @@ -282,7 +286,11 @@
</>
)}
</button>
{loading && loadingText && <LiveRegion>{loadingText}</LiveRegion>}
{loading && loadingText && (
<InternalLiveRegion tagName="span" hidden={true}>
{loadingText}
</InternalLiveRegion>
)}
</>
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/cards/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { InternalContainerAsSubstep } from '../container/internal';
import { useInternalI18n } from '../i18n/context';
import { AnalyticsFunnelSubStep } from '../internal/analytics/components/analytics-funnel';
import { getBaseProps } from '../internal/base-component';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { CollectionLabelContext } from '../internal/context/collection-label-context';
import { LinkDefaultVariantContext } from '../internal/context/link-default-variant-context';
import useBaseComponent from '../internal/hooks/use-base-component';
Expand Down Expand Up @@ -132,7 +132,7 @@ const Cards = React.forwardRef(function <T = any>(
status = (
<div className={styles.loading}>
<InternalStatusIndicator type="loading">
<LiveRegion visible={true}>{loadingText}</LiveRegion>
<InternalLiveRegion tagName="span">{loadingText}</InternalLiveRegion>
</InternalStatusIndicator>
</div>
);
Expand Down Expand Up @@ -178,11 +178,11 @@ const Cards = React.forwardRef(function <T = any>(
)}
>
{!!renderAriaLive && !!firstIndex && (
<LiveRegion>
<InternalLiveRegion hidden={true} tagName="span">
<span>
{renderAriaLive({ totalItemsCount, firstIndex, lastIndex: firstIndex + items.length - 1 })}
</span>
</LiveRegion>
</InternalLiveRegion>
)}
{status ?? (
<CardsList
Expand Down
6 changes: 4 additions & 2 deletions src/code-editor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useCurrentMode } from '@cloudscape-design/component-toolkit/internal';

import { useInternalI18n } from '../i18n/context';
import { getBaseProps } from '../internal/base-component';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { useFormFieldContext } from '../internal/context/form-field-context';
import { fireNonCancelableEvent } from '../internal/events';
import useForwardFocus from '../internal/hooks/forward-focus';
Expand Down Expand Up @@ -196,7 +196,9 @@ const CodeEditor = forwardRef((props: CodeEditorProps, ref: React.Ref<CodeEditor
>
{loading && (
<LoadingScreen>
<LiveRegion visible={true}>{i18n('i18nStrings.loadingState', i18nStrings?.loadingState)}</LiveRegion>
<InternalLiveRegion tagName="span">
{i18n('i18nStrings.loadingState', i18nStrings?.loadingState)}
</InternalLiveRegion>
</LoadingScreen>
)}

Expand Down
6 changes: 3 additions & 3 deletions src/code-editor/status-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import clsx from 'clsx';

import { InternalButton } from '../button/internal';
import { useInternalI18n } from '../i18n/context.js';
import LiveRegion from '../internal/components/live-region/index';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { CodeEditorProps } from './interfaces';
import { TabButton } from './tab-button';
import { getStatusButtonId, PaneStatus } from './util';
Expand Down Expand Up @@ -107,10 +107,10 @@ export function StatusBar({
isRefresh={isRefresh}
/>
</div>
<LiveRegion assertive={true}>
<InternalLiveRegion assertive={true} hidden={true} tagName="span">
<span>{errorText} </span>
<span>{warningText}</span>
</LiveRegion>
</InternalLiveRegion>
</div>

<div className={styles['status-bar__right']}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ export default class ContentDisplayPageObject extends CollectionPreferencesPageO
}

async expectAnnouncement(announcement: string) {
const liveRegion = await this.browser.$(
this.wrapper.findModal().findContentDisplayPreference().find('[aria-live="assertive"]').toSelector()
);
const liveRegion = await this.browser.$('[aria-live="assertive"]');
// Using getHTML because getText returns an empty string if the live region is outside the viewport.
// See https://webdriver.io/docs/api/element/getText/
return expect(liveRegion.getHTML()).resolves.toContain(announcement);
return this.waitForAssertion(() => expect(liveRegion.getHTML()).resolves.toContain(announcement));
}

findDragHandle(index = 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/date-picker/__integ__/date-picker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('Date Picker', () => {
await page.initLiveAnnouncementsObserver();
await page.setInputValue('2024/02/20', false);
await page.clickOpenCalendar();
await expect(page.getLiveAnnouncements()).resolves.toContain('February 2024');
await page.waitForAssertion(() => expect(page.getLiveAnnouncements()).resolves.toContain('February 2024'));
})
);
});
1 change: 1 addition & 0 deletions src/date-picker/__integ__/month-granularity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe('Date picker at month granularity', () => {
await page.initLiveAnnouncementsObserver();
await page.waitForLoad();
await page.clickOpenCalendar();
await new Promise(resolve => setTimeout(resolve, 2000)); // Wait for live region timeout
await expect(page.getLiveAnnouncements()).resolves.toContain('2024');
})
);
Expand Down
6 changes: 3 additions & 3 deletions src/date-picker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { InputProps } from '../input/interfaces';
import { getBaseProps } from '../internal/base-component';
import Dropdown from '../internal/components/dropdown';
import FocusLock from '../internal/components/focus-lock';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { fireNonCancelableEvent } from '../internal/events';
import checkControlled from '../internal/hooks/check-controlled';
import useForwardFocus from '../internal/hooks/forward-focus';
Expand Down Expand Up @@ -213,9 +213,9 @@ const DatePicker = React.forwardRef(
previousMonthAriaLabel: i18nStrings?.previousMonthAriaLabel ?? previousMonthAriaLabel,
}}
/>
<LiveRegion id={calendarDescriptionId}>
<InternalLiveRegion id={calendarDescriptionId} hidden={true} tagName="span">
{getBaseDateLabel({ date: baseDate, granularity, locale: normalizedLocale })}
</LiveRegion>
</InternalLiveRegion>
</div>
</FocusLock>
)}
Expand Down
6 changes: 4 additions & 2 deletions src/date-range-picker/__tests__/date-range-picker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ import { warnOnce } from '@cloudscape-design/component-toolkit/internal';
import DateRangePicker, { DateRangePickerProps } from '../../../lib/components/date-range-picker';
import FormField from '../../../lib/components/form-field';
import TestI18nProvider from '../../../lib/components/i18n/testing';
import { LiveRegionController } from '../../../lib/components/internal/components/live-region/controller.js';
import { NonCancelableEventHandler } from '../../../lib/components/internal/events';
import createWrapper from '../../../lib/components/test-utils/dom';
import DateRangePickerWrapper from '../../../lib/components/test-utils/dom/date-range-picker';
import { changeMode } from './change-mode';
import { i18nStrings } from './i18n-strings';
import { isValidRange } from './is-valid-range';

import styles from '../../../lib/components/date-range-picker/styles.css.js';
import segmentedStyles from '../../../lib/components/segmented-control/styles.css.js';

LiveRegionController.defaultMinDelay = 0;

jest.mock('@cloudscape-design/component-toolkit/internal', () => ({
...jest.requireActual('@cloudscape-design/component-toolkit/internal'),
warnOnce: jest.fn(),
Expand Down Expand Up @@ -256,7 +258,7 @@ describe('Date range picker', () => {

wrapper.findDropdown()!.findApplyButton().click();
expect(wrapper.findDropdown()!.findValidationError()?.getElement()).toHaveTextContent('10 is not allowed.');
expect(wrapper.findDropdown()!.findByClassName(styles['validation-section'])!.find('[aria-live]')).not.toBe(null);
expect(createWrapper().find('[aria-live]')!.getElement()).toHaveTextContent('10 is not allowed.');
});

test('after rendering the error once, displays subsequent errors in real time', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/date-range-picker/calendar/header/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { add } from 'date-fns';

import { renderMonthAndYear } from '../../../calendar/utils/intl';
import { useInternalI18n } from '../../../i18n/context.js';
import LiveRegion from '../../../internal/components/live-region';
import InternalLiveRegion from '../../../internal/components/live-region/internal';
import { NextMonthButton, PrevMonthButton } from './header-button';

import styles from '../../styles.css.js';
Expand Down Expand Up @@ -57,7 +57,7 @@ export default function CalendarHeader({
onChangeMonth={onChangeMonth}
/>
</div>
<LiveRegion>{isSingleGrid ? currentMonthLabel : `${prevMonthLabel}, ${currentMonthLabel}`}</LiveRegion>
<InternalLiveRegion message={isSingleGrid ? currentMonthLabel : `${prevMonthLabel}, ${currentMonthLabel}`} />
</>
);
}
7 changes: 5 additions & 2 deletions src/date-range-picker/calendar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { getDateLabel, renderTimeLabel } from '../../calendar/utils/intl';
import { getBaseDay } from '../../calendar/utils/navigation';
import { useInternalI18n } from '../../i18n/context.js';
import { BaseComponentProps } from '../../internal/base-component';
import LiveRegion from '../../internal/components/live-region';
import InternalLiveRegion from '../../internal/components/live-region/internal';
import { useMobile } from '../../internal/hooks/use-mobile/index.js';
import { useUniqueId } from '../../internal/hooks/use-unique-id';
import { formatDateTime, parseDate, splitDateTime } from '../../internal/utils/date-time';
Expand Down Expand Up @@ -271,7 +271,10 @@ export default function DateRangePickerCalendar({
{customAbsoluteRangeControl && <div>{customAbsoluteRangeControl(value, interceptedSetValue)}</div>}
</SpaceBetween>
</div>
<LiveRegion className={styles['calendar-aria-live']}>{announcement}</LiveRegion>
{/* Can't use message here because the contents are checked in tests */}
<InternalLiveRegion className={styles['calendar-aria-live']} hidden={true} tagName="span">
{announcement}
</InternalLiveRegion>
</>
);
}
6 changes: 4 additions & 2 deletions src/date-range-picker/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ButtonProps } from '../button/interfaces';
import { InternalButton } from '../button/internal';
import { useInternalI18n } from '../i18n/context';
import FocusLock from '../internal/components/focus-lock';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import InternalSpaceBetween from '../space-between/internal';
import Calendar from './calendar';
import { DateRangePickerProps } from './interfaces';
Expand Down Expand Up @@ -222,7 +222,9 @@ export function DateRangePickerDropdown({
>
<span className={styles['validation-error']}>{validationResult.errorMessage}</span>
</InternalAlert>
<LiveRegion>{validationResult.errorMessage}</LiveRegion>
<InternalLiveRegion hidden={true} tagName="span">
{validationResult.errorMessage}
</InternalLiveRegion>
</>
)}
</InternalBox>
Expand Down
6 changes: 4 additions & 2 deletions src/drawer/implementation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import clsx from 'clsx';
import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
import { useInternalI18n } from '../i18n/context';
import { getBaseProps } from '../internal/base-component';
import LiveRegion from '../internal/components/live-region';
import InternalLiveRegion from '../internal/components/live-region/internal';
import { InternalBaseComponentProps } from '../internal/hooks/use-base-component';
import { createWidgetizedComponent } from '../internal/widgets';
import InternalStatusIndicator from '../status-indicator/internal';
Expand Down Expand Up @@ -34,7 +34,9 @@ export function DrawerImplementation({
return loading ? (
<div {...containerProps} ref={__internalRootRef}>
<InternalStatusIndicator type="loading">
<LiveRegion visible={true}>{i18n('i18nStrings.loadingText', i18nStrings?.loadingText)}</LiveRegion>
<InternalLiveRegion tagName="span">
{i18n('i18nStrings.loadingText', i18nStrings?.loadingText)}
</InternalLiveRegion>
</InternalStatusIndicator>
</div>
) : (
Expand Down
Loading
Loading