Skip to content

Commit 0f4129f

Browse files
AlexGPlaykertal
andauthored
[Discover] Fix classic and ES|QL switch button a11y issues (#216471) (#221246)
## Summary Closes #216471 - Changed Monaco props in ES|QL editor to have accesibility support | Before | After | |--------|------| | <img width="488" alt="image (3)" src="https://github.com/user-attachments/assets/e7771a34-0ef2-408f-9b84-1165306ed241" /> | <img width="493" alt="image (2)" src="https://github.com/user-attachments/assets/1d13b700-f3cc-43e0-8a2f-0a1a9c7cd729" /> | - Changed field list grouped to only have aria-live enabled when the search input is focused | Scenario | Before | After | |----------|--------|------| | Classic to ESQL | <img width="487" alt="image (5)" src="https://github.com/user-attachments/assets/de18d15d-cadb-4ea2-af02-285ab196db64" /> | <img width="486" alt="image" src="https://github.com/user-attachments/assets/9a3c55f1-5ae0-44ef-8029-344543a32880" /> | | ESQL to Classic | <img width="489" alt="image (4)" src="https://github.com/user-attachments/assets/ebfd6f56-4f02-46e5-aa79-1fb0f515cc97" /> | <img width="486" alt="image (7)" src="https://github.com/user-attachments/assets/8a260a4f-2ad2-4ff9-af1e-7f7767c8df70" /> | ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: Matthias Wilhelm <[email protected]>
1 parent 08a3dd3 commit 0f4129f

File tree

7 files changed

+98
-35
lines changed

7 files changed

+98
-35
lines changed

src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ export const ESQLEditor = memo(function ESQLEditor({
717717
hover: {
718718
above: false,
719719
},
720-
accessibilitySupport: 'off',
720+
accessibilitySupport: 'auto',
721721
autoIndent: 'keep',
722722
automaticLayout: true,
723723
fixedOverflowWidgets: true,

src/platform/packages/shared/kbn-unified-field-list/src/components/field_list_filters/field_list_filters.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export interface FieldListFiltersProps<T extends FieldListItem> {
2424
getCustomFieldType?: FieldTypeFilterProps<T>['getCustomFieldType'];
2525
onSupportedFieldFilter?: FieldTypeFilterProps<T>['onSupportedFieldFilter'];
2626
onChangeFieldTypes: FieldTypeFilterProps<T>['onChange'];
27+
onFieldNameSearchFocus?: () => void;
28+
onFieldNameSearchBlur?: () => void;
2729
compressed?: FieldNameSearchProps['compressed'];
2830
nameFilter: FieldNameSearchProps['nameFilter'];
2931
screenReaderDescriptionId?: FieldNameSearchProps['screenReaderDescriptionId'];
@@ -59,6 +61,8 @@ function InnerFieldListFilters<T extends FieldListItem = DataViewField>({
5961
nameFilter,
6062
screenReaderDescriptionId,
6163
onChangeNameFilter,
64+
onFieldNameSearchBlur,
65+
onFieldNameSearchFocus,
6266
}: FieldListFiltersProps<T>) {
6367
return (
6468
<FieldNameSearch
@@ -80,6 +84,8 @@ function InnerFieldListFilters<T extends FieldListItem = DataViewField>({
8084
nameFilter={nameFilter}
8185
screenReaderDescriptionId={screenReaderDescriptionId}
8286
onChange={onChangeNameFilter}
87+
onFocus={onFieldNameSearchFocus}
88+
onBlur={onFieldNameSearchBlur}
8389
/>
8490
);
8591
}

src/platform/packages/shared/kbn-unified-field-list/src/components/field_list_filters/field_name_search.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export interface FieldNameSearchProps {
2222
nameFilter: string;
2323
screenReaderDescriptionId?: string;
2424
onChange: (nameFilter: string) => unknown;
25+
onFocus?: () => void;
26+
onBlur?: () => void;
2527
}
2628

2729
/**
@@ -41,6 +43,8 @@ export const FieldNameSearch: React.FC<FieldNameSearchProps> = ({
4143
nameFilter,
4244
screenReaderDescriptionId,
4345
onChange,
46+
onFocus,
47+
onBlur,
4448
}) => {
4549
const searchPlaceholder = i18n.translate('unifiedFieldList.fieldNameSearch.filterByNameLabel', {
4650
defaultMessage: 'Search field names',
@@ -61,6 +65,8 @@ export const FieldNameSearch: React.FC<FieldNameSearchProps> = ({
6165
onChange={(e) => {
6266
handleInputChange(e.target.value);
6367
}}
68+
onFocus={onFocus}
69+
onBlur={onBlur}
6470
placeholder={searchPlaceholder}
6571
value={inputValue}
6672
append={append}

src/platform/packages/shared/kbn-unified-field-list/src/components/field_list_grouped/field_list_grouped.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export interface FieldListGroupedProps<T extends FieldListItem> {
4141
scrollToTopResetCounter: number;
4242
screenReaderDescriptionId?: string;
4343
localStorageKeyPrefix?: string; // Your app name: "discover", "lens", etc. If not provided, sections state would not be persisted.
44+
muteScreenReader?: boolean; // Changes aria-live from "polite" to "off" - it's useful when the numbers change due to something not directly related to the field list and we want to avoid announcing it.
4445
'data-test-subj'?: string;
4546
}
4647

@@ -51,6 +52,7 @@ function InnerFieldListGrouped<T extends FieldListItem = DataViewField>({
5152
renderFieldItem,
5253
scrollToTopResetCounter,
5354
screenReaderDescriptionId,
55+
muteScreenReader,
5456
localStorageKeyPrefix,
5557
'data-test-subj': dataTestSubject = 'fieldListGrouped',
5658
}: FieldListGroupedProps<T>) {
@@ -141,7 +143,7 @@ function InnerFieldListGrouped<T extends FieldListItem = DataViewField>({
141143
{Boolean(screenReaderDescriptionId) && (
142144
<EuiScreenReaderOnly>
143145
<div
144-
aria-live="polite"
146+
aria-live={muteScreenReader ? 'off' : 'polite'}
145147
id={screenReaderDescriptionId}
146148
data-test-subj={`${dataTestSubject}__ariaDescription`}
147149
>

src/platform/packages/shared/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
181181
const [multiFieldsMap, setMultiFieldsMap] = useState<
182182
Map<string, Array<{ field: DataViewField; isSelected: boolean }>> | undefined
183183
>(undefined);
184+
const [isFieldNameSearchFocused, setIsFieldNameSearchFocused] = useState(false);
184185

185186
useEffect(() => {
186187
const result = getSelectedFields({
@@ -393,7 +394,12 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
393394
<EuiFlexItem grow={false}>{sidebarToggleButton}</EuiFlexItem>
394395
)}
395396
<EuiFlexItem>
396-
<FieldListFilters {...fieldListFiltersProps} compressed={compressed} />
397+
<FieldListFilters
398+
{...fieldListFiltersProps}
399+
compressed={compressed}
400+
onFieldNameSearchBlur={() => setIsFieldNameSearchFocused(false)}
401+
onFieldNameSearchFocus={() => setIsFieldNameSearchFocused(true)}
402+
/>
397403
</EuiFlexItem>
398404
</EuiFlexGroup>
399405
}
@@ -404,6 +410,7 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
404410
{...fieldListGroupedProps}
405411
renderFieldItem={renderFieldItem}
406412
localStorageKeyPrefix={stateService.creationOptions.localStorageKeyPrefix}
413+
muteScreenReader={!isFieldNameSearchFocused}
407414
/>
408415
) : (
409416
<EuiFlexItem grow />

src/platform/plugins/shared/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10+
import { render, screen, act as rtlAct } from '@testing-library/react';
11+
import userEvent from '@testing-library/user-event';
12+
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
1013
import { BehaviorSubject } from 'rxjs';
1114
import type { ReactWrapper } from 'enzyme';
1215
import { findTestSubject } from '@elastic/eui/lib/test';
@@ -34,6 +37,9 @@ import type { DiscoverCustomizationId } from '../../../../customizations/customi
3437
import type { FieldListCustomization, SearchBarCustomization } from '../../../../customizations';
3538
import { RuntimeStateProvider } from '../../state_management/redux';
3639
import { DiscoverMainProvider } from '../../state_management/discover_state_provider';
40+
import type { DataView } from '@kbn/data-views-plugin/common';
41+
42+
type TestWrapperProps = DiscoverSidebarResponsiveProps & { selectedDataView: DataView };
3743

3844
const mockSearchBarCustomization: SearchBarCustomization = {
3945
id: 'search_bar',
@@ -143,7 +149,7 @@ jest.mock('@kbn/discover-utils/src/utils/calc_field_counts', () => ({
143149

144150
jest.spyOn(ExistingFieldsServiceApi, 'loadFieldExisting');
145151

146-
function getCompProps(options?: { hits?: DataTableRecord[] }): DiscoverSidebarResponsiveProps {
152+
function getCompProps(options?: { hits?: DataTableRecord[] }): TestWrapperProps {
147153
const dataView = stubLogstashDataView;
148154
dataView.toSpec = jest.fn(() => ({}));
149155

@@ -186,12 +192,16 @@ function getStateContainer({ query }: { query?: Query | AggregateQuery }) {
186192
return stateContainer;
187193
}
188194

189-
async function mountComponent(
190-
props: DiscoverSidebarResponsiveProps,
195+
type EnzymeReturnType = ReactWrapper<TestWrapperProps>;
196+
type MountReturn<WithRTL extends boolean> = WithRTL extends true ? undefined : EnzymeReturnType;
197+
198+
async function mountComponent<WithReactTestingLibrary extends boolean = false>(
199+
props: TestWrapperProps,
191200
appStateParams: { query?: Query | AggregateQuery } = {},
192-
services?: DiscoverServices
193-
): Promise<ReactWrapper<DiscoverSidebarResponsiveProps>> {
194-
let comp: ReactWrapper<DiscoverSidebarResponsiveProps>;
201+
services?: DiscoverServices,
202+
withReactTestingLibrary?: WithReactTestingLibrary
203+
): Promise<MountReturn<WithReactTestingLibrary>> {
204+
let comp: ReactWrapper<TestWrapperProps>;
195205
const stateContainer = getStateContainer(appStateParams);
196206
const mockedServices = services ?? createMockServices();
197207
mockedServices.data.dataViews.getIdsWithTitle = jest.fn(async () =>
@@ -206,30 +216,37 @@ async function mountComponent(
206216
.fn()
207217
.mockImplementation(() => stateContainer.appState.getState());
208218

219+
const component = (
220+
<EuiProvider>
221+
<KibanaContextProvider services={mockedServices}>
222+
<DiscoverMainProvider value={stateContainer}>
223+
<RuntimeStateProvider currentDataView={props.selectedDataView} adHocDataViews={[]}>
224+
<DiscoverSidebarResponsive {...props} />
225+
</RuntimeStateProvider>
226+
</DiscoverMainProvider>
227+
</KibanaContextProvider>
228+
</EuiProvider>
229+
);
230+
231+
if (withReactTestingLibrary) {
232+
await rtlAct(() => render(<IntlProvider locale="en">{component}</IntlProvider>));
233+
return undefined as MountReturn<WithReactTestingLibrary>;
234+
}
235+
209236
await act(async () => {
210-
comp = mountWithIntl(
211-
<EuiProvider>
212-
<KibanaContextProvider services={mockedServices}>
213-
<DiscoverMainProvider value={stateContainer}>
214-
<RuntimeStateProvider currentDataView={props.selectedDataView!} adHocDataViews={[]}>
215-
<DiscoverSidebarResponsive {...props} />{' '}
216-
</RuntimeStateProvider>
217-
</DiscoverMainProvider>
218-
</KibanaContextProvider>
219-
</EuiProvider>
220-
);
237+
comp = mountWithIntl(component);
221238
// wait for lazy modules
222239
await new Promise((resolve) => setTimeout(resolve, 0));
223240
comp.update();
224241
});
225242

226243
comp!.update();
227244

228-
return comp!;
245+
return comp! as unknown as MountReturn<WithReactTestingLibrary>;
229246
}
230247

231248
describe('discover responsive sidebar', function () {
232-
let props: DiscoverSidebarResponsiveProps;
249+
let props: TestWrapperProps;
233250

234251
beforeEach(async () => {
235252
(ExistingFieldsServiceApi.loadFieldExisting as jest.Mock).mockImplementation(async () => ({
@@ -328,17 +345,42 @@ describe('discover responsive sidebar', function () {
328345
expect(ExistingFieldsServiceApi.loadFieldExisting).toHaveBeenCalledTimes(1);
329346
});
330347

331-
it('should set a11y attributes for the search input in the field list', async function () {
332-
const comp = await mountComponent(props);
348+
describe('when the input is not focused', () => {
349+
it('should set a11y attributes for the search input in the field list', async function () {
350+
// When
351+
await mountComponent(props, undefined, undefined, true);
333352

334-
const a11yDescription = findTestSubject(comp, 'fieldListGrouped__ariaDescription');
335-
expect(a11yDescription.prop('aria-live')).toBe('polite');
336-
expect(a11yDescription.text()).toBe(
337-
'1 selected field. 4 popular fields. 3 available fields. 20 empty fields. 2 meta fields.'
338-
);
353+
// Then
354+
const a11yDescription = screen.getByTestId('fieldListGrouped__ariaDescription');
355+
expect(a11yDescription).toHaveAttribute('aria-live', 'off');
356+
expect(a11yDescription).toHaveTextContent(
357+
'1 selected field. 4 popular fields. 3 available fields. 20 empty fields. 2 meta fields.'
358+
);
359+
360+
const searchInput = screen.getByTestId('fieldListFiltersFieldSearch');
361+
expect(searchInput).toHaveAttribute('aria-describedby', a11yDescription.id);
362+
});
363+
});
339364

340-
const searchInput = findTestSubject(comp, 'fieldListFiltersFieldSearch');
341-
expect(searchInput.first().prop('aria-describedby')).toBe(a11yDescription.prop('id'));
365+
describe('when the input is focused', () => {
366+
it('should set a11y attributes for the search input in the field list', async function () {
367+
// Given
368+
const user = userEvent.setup();
369+
370+
// When
371+
await mountComponent(props, undefined, undefined, true);
372+
373+
// Then
374+
const searchInput = screen.getByTestId('fieldListFiltersFieldSearch');
375+
const a11yDescription = screen.getByTestId('fieldListGrouped__ariaDescription');
376+
await user.click(searchInput);
377+
expect(searchInput).toHaveAttribute('aria-describedby', a11yDescription.id);
378+
379+
expect(a11yDescription).toHaveAttribute('aria-live', 'polite');
380+
expect(a11yDescription).toHaveTextContent(
381+
'1 selected field. 4 popular fields. 3 available fields. 20 empty fields. 2 meta fields.'
382+
);
383+
});
342384
});
343385

344386
it('should not have selected fields if no columns selected', async function () {
@@ -385,7 +427,7 @@ describe('discover responsive sidebar', function () {
385427
});
386428

387429
it('should not calculate counts if documents are not fetched yet', async function () {
388-
const propsWithoutDocuments: DiscoverSidebarResponsiveProps = {
430+
const propsWithoutDocuments: TestWrapperProps = {
389431
...props,
390432
documents$: new BehaviorSubject({
391433
fetchStatus: FetchStatus.UNINITIALIZED,
@@ -662,7 +704,7 @@ describe('discover responsive sidebar', function () {
662704

663705
it('should render buttons in data view picker correctly', async () => {
664706
const services = createMockServices();
665-
const propsWithPicker: DiscoverSidebarResponsiveProps = {
707+
const propsWithPicker: TestWrapperProps = {
666708
...props,
667709
fieldListVariant: 'button-and-flyout-always',
668710
};
@@ -694,7 +736,7 @@ describe('discover responsive sidebar', function () {
694736
const services = createMockServices();
695737
services.dataViewEditor.userPermissions.editDataView = jest.fn(() => false);
696738
services.dataViewFieldEditor.userPermissions.editIndexPattern = jest.fn(() => false);
697-
const propsWithPicker: DiscoverSidebarResponsiveProps = {
739+
const propsWithPicker: TestWrapperProps = {
698740
...props,
699741
fieldListVariant: 'button-and-flyout-always',
700742
};

src/platform/test/functional/page_objects/unified_field_list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export class UnifiedFieldListPageObject extends FtrService {
2525

2626
public async clearFieldSearchInput() {
2727
const fieldSearch = await this.testSubjects.find('fieldListFiltersFieldSearch');
28-
await fieldSearch.clearValue();
28+
await fieldSearch.clearValueWithKeyboard();
2929
}
3030

3131
public async getAllFieldNames() {

0 commit comments

Comments
 (0)