Skip to content

Commit 52e4eae

Browse files
AlexGPlaykertal
AlexGPlay
and
kertal
authored
[Discover] Add skip to next section in field list grouped (#221792)
## Summary Closes #155015 Closes #156127 Adds a button that allows keyboard users to jump to the next section instead of having to tab through all the fields. https://github.com/user-attachments/assets/f7f53243-3c6a-45ec-9c3d-efa67a5e9afa ### 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 08eb393 commit 52e4eae

File tree

4 files changed

+322
-128
lines changed

4 files changed

+322
-128
lines changed

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

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { ExistenceFetchStatus } from '../../types';
2222
import { FieldsAccordion } from './fields_accordion';
2323
import { NoFieldsCallout } from './no_fields_callout';
2424
import { useGroupedFields, type GroupedFieldsParams } from '../../hooks/use_grouped_fields';
25+
import { render, screen, within } from '@testing-library/react';
26+
import userEvent from '@testing-library/user-event';
2527

2628
jest.mock('lodash', () => {
2729
const original = jest.requireActual('lodash');
@@ -76,6 +78,27 @@ describe('UnifiedFieldList FieldListGrouped + useGroupedFields()', () => {
7678
hookParams: Omit<GroupedFieldsParams<DataViewField>, 'services'>;
7779
}
7880

81+
function mountWithRTL({ listProps, hookParams }: WrapperProps) {
82+
const Wrapper: React.FC<WrapperProps> = (props) => {
83+
const {
84+
fieldListFiltersProps,
85+
fieldListGroupedProps: { fieldGroups },
86+
} = useGroupedFields({
87+
...props.hookParams,
88+
services: mockedServices,
89+
});
90+
91+
return (
92+
<>
93+
<FieldListFilters {...fieldListFiltersProps} />
94+
<FieldListGrouped {...props.listProps} fieldGroups={fieldGroups} />
95+
</>
96+
);
97+
};
98+
99+
render(<Wrapper hookParams={hookParams} listProps={listProps} />);
100+
}
101+
79102
async function mountGroupedList({ listProps, hookParams }: WrapperProps): Promise<ReactWrapper> {
80103
const Wrapper: React.FC<WrapperProps> = (props) => {
81104
const {
@@ -124,7 +147,7 @@ describe('UnifiedFieldList FieldListGrouped + useGroupedFields()', () => {
124147
fieldsExistenceStatus: ExistenceFetchStatus.unknown,
125148
},
126149
hookParams: {
127-
dataViewId: dataView.id!,
150+
dataViewId: dataView.id ?? null,
128151
allFields,
129152
},
130153
});
@@ -466,6 +489,81 @@ describe('UnifiedFieldList FieldListGrouped + useGroupedFields()', () => {
466489
);
467490
});
468491

492+
describe('Skip Link Functionality', () => {
493+
it('renders the skip link when there is a next section', async () => {
494+
mountWithRTL({
495+
listProps: {
496+
...defaultProps,
497+
fieldsExistenceStatus: ExistenceFetchStatus.succeeded,
498+
},
499+
hookParams: {
500+
dataViewId: dataView.id!,
501+
allFields,
502+
},
503+
});
504+
505+
// Check that the first accordion (Available Fields) has a skip link
506+
const availableFieldsAccordion = screen.getByTestId('fieldListGroupedAvailableFields');
507+
const skipLinks = within(availableFieldsAccordion).getAllByRole('link', {
508+
name: /go to meta fields/i,
509+
});
510+
511+
// Since we have multiple sections, we should have at least one skip link
512+
expect(skipLinks.length).toBe(1);
513+
});
514+
515+
it('does not render a skip link in the last section', async () => {
516+
mountWithRTL({
517+
listProps: {
518+
...defaultProps,
519+
fieldsExistenceStatus: ExistenceFetchStatus.succeeded,
520+
},
521+
hookParams: {
522+
dataViewId: dataView.id!,
523+
allFields,
524+
},
525+
});
526+
527+
// Find the last accordion (should be Meta Fields)
528+
const metaFieldsAccordion = screen.getByTestId('fieldListGroupedMetaFields');
529+
530+
// The last section shouldn't have a skip link
531+
const skipLinksInLastAccordion = within(metaFieldsAccordion).queryAllByRole('link', {
532+
name: /go to/i,
533+
});
534+
535+
expect(skipLinksInLastAccordion.length).toBe(0);
536+
});
537+
538+
it('sets focus on the next section when skip link is clicked', async () => {
539+
const user = userEvent.setup();
540+
541+
mountWithRTL({
542+
listProps: {
543+
...defaultProps,
544+
fieldsExistenceStatus: ExistenceFetchStatus.succeeded,
545+
},
546+
hookParams: {
547+
dataViewId: dataView.id!,
548+
allFields,
549+
},
550+
});
551+
552+
// Find the skip link in the Available Fields accordion
553+
const availableFieldsAccordion = screen.getByTestId('fieldListGroupedAvailableFields');
554+
const skipLink = within(availableFieldsAccordion).getByRole('link', {
555+
name: /go to meta fields/i,
556+
});
557+
558+
// Click the skip link
559+
await user.click(skipLink);
560+
561+
// Verify that the Meta Fields accordion is now focused
562+
const metaFieldsButton = screen.getByRole('button', { name: /meta fields/i });
563+
expect(metaFieldsButton).toHaveFocus();
564+
});
565+
});
566+
469567
it('persists sections state in local storage', async () => {
470568
const wrapper = await mountGroupedList({
471569
listProps: {

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

Lines changed: 77 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { partition, throttle } from 'lodash';
1111
import React, { Fragment, useCallback, useEffect, useMemo, useState } from 'react';
1212
import useLocalStorage from 'react-use/lib/useLocalStorage';
1313
import { i18n } from '@kbn/i18n';
14-
import { EuiScreenReaderOnly, EuiSpacer } from '@elastic/eui';
14+
import { EuiScreenReaderOnly, EuiSpacer, EuiSkipLink, useGeneratedHtmlId } from '@elastic/eui';
1515
import { type DataViewField } from '@kbn/data-views-plugin/common';
1616
import { NoFieldsCallout } from './no_fields_callout';
1717
import { FieldsAccordion, type FieldsAccordionProps, getFieldKey } from './fields_accordion';
@@ -59,6 +59,8 @@ function InnerFieldListGrouped<T extends FieldListItem = DataViewField>({
5959
const hasSyncedExistingFields =
6060
fieldsExistenceStatus && fieldsExistenceStatus !== ExistenceFetchStatus.unknown;
6161

62+
const buttonIdPrefix = useGeneratedHtmlId({ prefix: 'fieldListGroupedButton' });
63+
6264
const [fieldGroupsToShow, fieldGroupsToCollapse] = partition(
6365
Object.entries(fieldGroups),
6466
([, { showInAccordion }]) => showInAccordion
@@ -243,66 +245,83 @@ function InnerFieldListGrouped<T extends FieldListItem = DataViewField>({
243245
<EuiSpacer size="s" />
244246
</>
245247
)}
246-
{fieldGroupsToShow.map(([key, fieldGroup], index) => {
247-
const hidden = Boolean(fieldGroup.hideIfEmpty) && !fieldGroup.fields.length;
248-
if (hidden) {
249-
return null;
250-
}
251-
return (
252-
<Fragment key={key}>
253-
<FieldsAccordion<T>
254-
id={`${dataTestSubject}${key}`}
255-
initialIsOpen={Boolean(accordionState[key])}
256-
label={fieldGroup.title}
257-
helpTooltip={fieldGroup.helpText}
258-
hideDetails={fieldGroup.hideDetails}
259-
hasLoaded={hasSyncedExistingFields}
260-
fieldsCount={fieldGroup.fields.length}
261-
isFiltered={fieldGroup.fieldCount !== fieldGroup.fields.length}
262-
fieldSearchHighlight={fieldGroup.fieldSearchHighlight}
263-
paginatedFields={paginatedFields[key]}
264-
groupIndex={index + 1}
265-
groupName={key as FieldsGroupNames}
266-
onToggle={(open) => {
267-
setAccordionState((s) => ({
268-
...s,
269-
[key]: open,
270-
}));
271-
const displayedFieldLength = getDisplayedFieldsLength(fieldGroups, {
272-
...accordionState,
273-
[key]: open,
274-
});
275-
setPageSize(
276-
Math.max(
277-
PAGINATION_SIZE,
278-
Math.min(Math.ceil(pageSize * 1.5), displayedFieldLength)
279-
)
280-
);
281-
if (localStorageKeyPrefix) {
282-
storeInitiallyOpenSections({
283-
...storedInitiallyOpenSections,
248+
{fieldGroupsToShow
249+
.filter(([_, fieldGroup]) => {
250+
if (fieldGroup.fields.length) return true;
251+
return !Boolean(fieldGroup.hideIfEmpty);
252+
})
253+
.map(([key, fieldGroup], index, _fieldGroupsToShow) => {
254+
const nextFieldGroup = _fieldGroupsToShow.at(index + 1);
255+
256+
return (
257+
<Fragment key={key}>
258+
<FieldsAccordion<T>
259+
id={`${dataTestSubject}${key}`}
260+
buttonId={`${buttonIdPrefix}${key}`}
261+
initialIsOpen={Boolean(accordionState[key])}
262+
label={fieldGroup.title}
263+
helpTooltip={fieldGroup.helpText}
264+
hideDetails={fieldGroup.hideDetails}
265+
hasLoaded={hasSyncedExistingFields}
266+
fieldsCount={fieldGroup.fields.length}
267+
isFiltered={fieldGroup.fieldCount !== fieldGroup.fields.length}
268+
fieldSearchHighlight={fieldGroup.fieldSearchHighlight}
269+
paginatedFields={paginatedFields[key]}
270+
groupIndex={index + 1}
271+
groupName={key as FieldsGroupNames}
272+
onToggle={(open) => {
273+
setAccordionState((s) => ({
274+
...s,
275+
[key]: open,
276+
}));
277+
const displayedFieldLength = getDisplayedFieldsLength(fieldGroups, {
278+
...accordionState,
284279
[key]: open,
285280
});
281+
setPageSize(
282+
Math.max(
283+
PAGINATION_SIZE,
284+
Math.min(Math.ceil(pageSize * 1.5), displayedFieldLength)
285+
)
286+
);
287+
if (localStorageKeyPrefix) {
288+
storeInitiallyOpenSections({
289+
...storedInitiallyOpenSections,
290+
[key]: open,
291+
});
292+
}
293+
}}
294+
showExistenceFetchError={fieldsExistenceStatus === ExistenceFetchStatus.failed}
295+
showExistenceFetchTimeout={fieldsExistenceStatus === ExistenceFetchStatus.failed} // TODO: deprecate timeout logic?
296+
renderCallout={() => (
297+
<NoFieldsCallout
298+
isAffectedByGlobalFilter={fieldGroup.isAffectedByGlobalFilter}
299+
isAffectedByTimerange={fieldGroup.isAffectedByTimeFilter}
300+
isAffectedByFieldFilter={fieldGroup.fieldCount !== fieldGroup.fields.length}
301+
fieldsExistInIndex={!!fieldsExistInIndex}
302+
defaultNoFieldsMessage={fieldGroup.defaultNoFieldsMessage}
303+
data-test-subj={`${dataTestSubject}${key}NoFieldsCallout`}
304+
/>
305+
)}
306+
renderFieldItem={renderFieldItem}
307+
extraAction={
308+
nextFieldGroup ? (
309+
<EuiSkipLink
310+
overrideLinkBehavior
311+
destinationId={`${buttonIdPrefix}${nextFieldGroup[0]}`}
312+
>
313+
{i18n.translate('unifiedFieldList.fieldListGrouped.goToNextGroupLink', {
314+
defaultMessage: 'Go to {nextFieldGroup}',
315+
values: { nextFieldGroup: nextFieldGroup[1].title },
316+
})}
317+
</EuiSkipLink>
318+
) : null
286319
}
287-
}}
288-
showExistenceFetchError={fieldsExistenceStatus === ExistenceFetchStatus.failed}
289-
showExistenceFetchTimeout={fieldsExistenceStatus === ExistenceFetchStatus.failed} // TODO: deprecate timeout logic?
290-
renderCallout={() => (
291-
<NoFieldsCallout
292-
isAffectedByGlobalFilter={fieldGroup.isAffectedByGlobalFilter}
293-
isAffectedByTimerange={fieldGroup.isAffectedByTimeFilter}
294-
isAffectedByFieldFilter={fieldGroup.fieldCount !== fieldGroup.fields.length}
295-
fieldsExistInIndex={!!fieldsExistInIndex}
296-
defaultNoFieldsMessage={fieldGroup.defaultNoFieldsMessage}
297-
data-test-subj={`${dataTestSubject}${key}NoFieldsCallout`}
298-
/>
299-
)}
300-
renderFieldItem={renderFieldItem}
301-
/>
302-
<EuiSpacer size="m" />
303-
</Fragment>
304-
);
305-
})}
320+
/>
321+
<EuiSpacer size="m" />
322+
</Fragment>
323+
);
324+
})}
306325
</div>
307326
</div>
308327
);

0 commit comments

Comments
 (0)