Skip to content

Commit

Permalink
[EuiSelectable] Fix text truncation when a scrollbar is present (#7392)
Browse files Browse the repository at this point in the history
  • Loading branch information
cee-chen authored Dec 3, 2023
1 parent 664ed2b commit b76ae3f
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
3 changes: 3 additions & 0 deletions changelogs/upcoming/7392.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed a bug with `EuiSelectable`s with custom `truncationProps`, where scrollbar widths were not being accounted for
17 changes: 17 additions & 0 deletions src/components/selectable/selectable.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,23 @@ describe('EuiSelectable', () => {
);
});

it('correctly accounts for scrollbar width', () => {
const multipleOptions = Array.from({ length: 5 }).map(
() => sharedProps.options[0]
);
cy.realMount(
<EuiSelectableListboxOnly
{...truncationProps}
height={100}
options={multipleOptions}
/>
);

cy.get('[data-test-subj="truncatedText"]')
.first()
.should('have.text', 'Lorem ipsum …iscing elit.');
});

it('correctly accounts for the keyboard focus badge', () => {
cy.realMount(<EuiSelectableListboxOnly {...truncationProps} />);

Expand Down
16 changes: 16 additions & 0 deletions src/components/selectable/selectable_list/selectable_list.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,13 @@ describe('EuiSelectableListItem', () => {
});

describe('truncation performance optimization', () => {
// Mock requestAnimationFrame
beforeEach(() => {
jest
.spyOn(window, 'requestAnimationFrame')
.mockImplementation((cb: Function) => cb());
});

it('does not render EuiTextTruncate if not virtualized and text is wrapping', () => {
const { container } = render(
<EuiSelectableList
Expand Down Expand Up @@ -395,6 +402,12 @@ describe('EuiSelectableListItem', () => {
});

it('attempts to use a default optimized option width calculated from the wrapping EuiAutoSizer', () => {
// jsdom doesn't return valid element offsetWidths, so we have to mock it here
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', {
configurable: true,
value: 600,
});

const { container } = render(
<EuiSelectableList
options={options}
Expand All @@ -409,6 +422,9 @@ describe('EuiSelectableListItem', () => {
expect(
container.querySelector('[data-resize-observer]')
).not.toBeInTheDocument();

// Reset jsdom mock
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { value: 0 });
});

it('falls back to individual resize observers if options have append/prepend nodes', () => {
Expand Down
24 changes: 16 additions & 8 deletions src/components/selectable/selectable_list/selectable_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -479,15 +479,23 @@ export class EuiSelectableList<T> extends Component<
const checkedIconOffset = this.props.showIcons === false ? 0 : 28; // Defaults to true
this.focusBadgeOffset = this.props.onFocusBadge === false ? 0 : 46;

this.setState({
defaultOptionWidth: containerWidth - paddingOffset - checkedIconOffset,
});
// Wait a tick for the listbox ref to update before proceeding
requestAnimationFrame(() => {
const scrollbarOffset = this.listBoxRef
? containerWidth - this.listBoxRef.offsetWidth
: 0;

// Potentially force list rows to rerender on dynamic resize as well,
// but try to do it as lightly as possible
if (truncationProps || (searchable && searchValue)) {
this.forceVirtualizedListRowRerender();
}
this.setState({
defaultOptionWidth:
containerWidth - scrollbarOffset - paddingOffset - checkedIconOffset,
});

// Potentially force list rows to rerender on dynamic resize as well,
// but try to do it as lightly as possible
if (truncationProps || (searchable && searchValue)) {
this.forceVirtualizedListRowRerender();
}
});
};

getTruncationProps = (option: EuiSelectableOption, isFocused: boolean) => {
Expand Down

0 comments on commit b76ae3f

Please sign in to comment.