From d23de70f3616ddd4d11d7088702065f7628c1f70 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 11:03:29 -0700 Subject: [PATCH 1/6] Pass the EuiSelectable `height` to the scrolling list element --- .../selectable_list.test.tsx.snap | 2 +- .../selectable_list/_selectable_list.scss | 1 + .../selectable_list/selectable_list.test.tsx | 35 ++++++++++++++----- .../selectable_list/selectable_list.tsx | 5 ++- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap index a388f1c50ad..c5f88e8393f 100644 --- a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap +++ b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap @@ -1456,7 +1456,7 @@ exports[`EuiSelectableListItem props height is full 1`] = ` `; -exports[`EuiSelectableListItem props isVirtualized can be false 1`] = ` +exports[`EuiSelectableListItem props isVirtualized={false} renders 1`] = `
ul:focus { diff --git a/src/components/selectable/selectable_list/selectable_list.test.tsx b/src/components/selectable/selectable_list/selectable_list.test.tsx index 847ed388948..897721906bd 100644 --- a/src/components/selectable/selectable_list/selectable_list.test.tsx +++ b/src/components/selectable/selectable_list/selectable_list.test.tsx @@ -227,16 +227,33 @@ describe('EuiSelectableListItem', () => { expect(container.firstChild).toMatchSnapshot(); }); - test('isVirtualized can be false', () => { - const { container } = render( - - ); + describe('isVirtualized={false}', () => { + it('renders', () => { + const { container } = render( + + ); - expect(container.firstChild).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); + }); + + it('maintains the passed height when false', () => { + const { container } = render( + + ); + + expect(container.querySelector('.euiSelectableList__list')).toHaveStyle( + 'block-size: 300px' + ); + }); }); test('searchable enables correct screen reader instructions', () => { diff --git a/src/components/selectable/selectable_list/selectable_list.tsx b/src/components/selectable/selectable_list/selectable_list.tsx index 9812d3a9e77..e1e1a069f08 100644 --- a/src/components/selectable/selectable_list/selectable_list.tsx +++ b/src/components/selectable/selectable_list/selectable_list.tsx @@ -609,10 +609,12 @@ export class EuiSelectableList extends Component< ...rest } = this.props; + const heightIsFull = forcedHeight === 'full'; + const classes = classNames( 'euiSelectableList', { - 'euiSelectableList-fullHeight': forcedHeight === 'full', + 'euiSelectableList-fullHeight': heightIsFull, 'euiSelectableList-bordered': bordered, }, className @@ -625,6 +627,7 @@ export class EuiSelectableList extends Component< ) : (
    From f34cbd719b4c97ce11b0c692dbfb475da83e50c4 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 11:18:46 -0700 Subject: [PATCH 2/6] Fix keyboard navigation accessibility to match virtualized UX --- src/components/selectable/selectable.spec.tsx | 16 ++++++++++++++++ .../selectable_list/selectable_list.tsx | 15 ++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/components/selectable/selectable.spec.tsx b/src/components/selectable/selectable.spec.tsx index f477641bbd1..9ac3ee00c80 100644 --- a/src/components/selectable/selectable.spec.tsx +++ b/src/components/selectable/selectable.spec.tsx @@ -253,6 +253,22 @@ describe('EuiSelectable', () => { }); }); + describe('isVirtualization={false}', () => { + it('correctly scrolls keyboard navigated elements into view', () => { + cy.realMount( + + ); + cy.get('.euiSelectableList__list').invoke('scrollTop').should('eq', 0); + + cy.realPress('Tab'); + cy.realPress('ArrowUp'); + cy.get('.euiSelectableList__list').invoke('scrollTop').should('eq', 48); + }); + }); + describe('truncation', () => { const sharedProps = { style: { width: 240 }, diff --git a/src/components/selectable/selectable_list/selectable_list.tsx b/src/components/selectable/selectable_list/selectable_list.tsx index e1e1a069f08..d888897a156 100644 --- a/src/components/selectable/selectable_list/selectable_list.tsx +++ b/src/components/selectable/selectable_list/selectable_list.tsx @@ -244,7 +244,8 @@ export class EuiSelectableList extends Component< }; componentDidUpdate(prevProps: EuiSelectableListProps) { - const { activeOptionIndex, visibleOptions, options } = this.props; + const { isVirtualized, activeOptionIndex, visibleOptions, options } = + this.props; if (this.listBoxRef && this.props.searchable !== true) { this.listBoxRef.setAttribute( @@ -253,8 +254,16 @@ export class EuiSelectableList extends Component< ); } - if (this.listRef && typeof activeOptionIndex !== 'undefined') { - this.listRef.scrollToItem(activeOptionIndex, 'auto'); + if (typeof activeOptionIndex !== 'undefined') { + if (isVirtualized) { + this.listRef?.scrollToItem(activeOptionIndex, 'auto'); + } else { + const activeOptionId = `#${this.props.makeOptionId(activeOptionIndex)}`; + const activeOptionEl = this.listBoxRef?.querySelector(activeOptionId); + if (activeOptionEl) { + activeOptionEl.scrollIntoView({ block: 'nearest' }); + } + } } if ( From c76aba5e379de1d3a43734cb9c7c1d8b4fdfb936 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 11:26:52 -0700 Subject: [PATCH 3/6] [perf] Add `prevProps.activeOptionIndex` check to only run on change and not every rerender + bonus props destructure --- .../selectable_list.test.tsx.snap | 1 - .../selectable_list/selectable_list.tsx | 32 +++++++++++-------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap index c5f88e8393f..e338cf3b75d 100644 --- a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap +++ b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap @@ -10,7 +10,6 @@ exports[`EuiSelectableListItem group labels handles updating aria attrs correctl tabindex="-1" >
      extends Component< const { isVirtualized, activeOptionIndex, visibleOptions, options } = this.props; - if (this.listBoxRef && this.props.searchable !== true) { - this.listBoxRef.setAttribute( - 'aria-activedescendant', - `${this.props.makeOptionId(activeOptionIndex)}` - ); - } + if (prevProps.activeOptionIndex !== activeOptionIndex) { + const { makeOptionId } = this.props; + + if (this.listBoxRef && this.props.searchable !== true) { + this.listBoxRef.setAttribute( + 'aria-activedescendant', + makeOptionId(activeOptionIndex) + ); + } - if (typeof activeOptionIndex !== 'undefined') { - if (isVirtualized) { - this.listRef?.scrollToItem(activeOptionIndex, 'auto'); - } else { - const activeOptionId = `#${this.props.makeOptionId(activeOptionIndex)}`; - const activeOptionEl = this.listBoxRef?.querySelector(activeOptionId); - if (activeOptionEl) { - activeOptionEl.scrollIntoView({ block: 'nearest' }); + if (typeof activeOptionIndex !== 'undefined') { + if (isVirtualized) { + this.listRef?.scrollToItem(activeOptionIndex, 'auto'); + } else { + const activeOptionId = `#${makeOptionId(activeOptionIndex)}`; + const activeOptionEl = this.listBoxRef?.querySelector(activeOptionId); + if (activeOptionEl) { + activeOptionEl.scrollIntoView({ block: 'nearest' }); + } } } } From f9950c5f7680521785f41ee40ba8dc422488ba35 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 11:34:59 -0700 Subject: [PATCH 4/6] Update docs to remove note about removed scrolling --- src-docs/src/views/selectable/selectable_example.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src-docs/src/views/selectable/selectable_example.js b/src-docs/src/views/selectable/selectable_example.js index fe757177681..9f6dae7c1b1 100644 --- a/src-docs/src/views/selectable/selectable_example.js +++ b/src-docs/src/views/selectable/selectable_example.js @@ -479,11 +479,14 @@ export const SelectableExample = {

      If listProps.isVirtualized is set to{' '} - false, each row will fit its contents and removes - all scrolling. Therefore, we recommend having a large enough - container to accommodate all options. You can also remove truncation - by setting {'textWrap="wrap"'} when - virtualization is off. + false, each row will fit its content. You can + also remove truncation by setting{' '} + {'textWrap="wrap"'} when virtualization is off. + Note that while this is useful for dynamic options, it can also be + computationally expensive as all off-screen options will be + rendered to the DOM. We do not recommend turning off virtualization + for high numbers of options, but if you absolutely must do so, we + suggest using methods such as async loading more options.

      Custom content

      From 08cc7d12461c5ce94cfb6cc79e4c7a232c05533b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 11:53:46 -0700 Subject: [PATCH 5/6] changelog --- changelogs/upcoming/7609.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/upcoming/7609.md diff --git a/changelogs/upcoming/7609.md b/changelogs/upcoming/7609.md new file mode 100644 index 00000000000..cbab32b34fc --- /dev/null +++ b/changelogs/upcoming/7609.md @@ -0,0 +1 @@ +- Updated `EuiSelectable` to support scrolling list containers when `listProps.isVirtualization` is set to `false` From 4715674f6e5351600e955115141d525792497d2f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 21 Mar 2024 12:05:21 -0700 Subject: [PATCH 6/6] Fix incredibly bizarre cross-browser CSS bug - chrome, why?? --- src/components/selectable/selectable_list/_selectable_list.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/selectable/selectable_list/_selectable_list.scss b/src/components/selectable/selectable_list/_selectable_list.scss index 82119a8d912..bb384224d1b 100644 --- a/src/components/selectable/selectable_list/_selectable_list.scss +++ b/src/components/selectable/selectable_list/_selectable_list.scss @@ -21,6 +21,7 @@ @include euiOverflowShadow; @include euiScrollBar; overflow: auto; + position: relative; // Chrome/Edge loses its mind without this and doesn't render `mask-image` correctly &:focus, & > ul:focus {