Skip to content

Commit

Permalink
New and improved useOverflow algorithm (#2154)
Browse files Browse the repository at this point in the history
  • Loading branch information
@r100-stack
r100-stack authored Nov 8, 2024
1 parent c355680 commit 9cdf466
Show file tree
Hide file tree
Showing 15 changed files with 433 additions and 725 deletions.
5 changes: 5 additions & 0 deletions .changeset/sour-apes-visit.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-react': patch
---

Fixed bugs and improved performance in components with overflow support (e.g. `ComboBox`, `Select`, `ButtonGroup`, `Breadcrumbs`, etc.)
64 changes: 2 additions & 62 deletions packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import * as React from 'react';
import { fireEvent, render, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';

import { Breadcrumbs } from './Breadcrumbs.js';
import { SvgChevronRight, SvgMore } from '../../utils/index.js';
import * as UseOverflow from '../../utils/hooks/useOverflow.js';
import { IconButton } from '../Buttons/IconButton.js';
import { SvgChevronRight } from '../../utils/index.js';
import { Button } from '../Buttons/Button.js';
import { userEvent } from '@testing-library/user-event';

Expand Down Expand Up @@ -56,13 +54,6 @@ const assertBaseElement = (
});
};

const useOverflowMock = vi
.spyOn(UseOverflow, 'useOverflow')
.mockImplementation((itemsCount) => [vi.fn(), itemsCount]);
beforeEach(() => {
useOverflowMock.mockImplementation((itemsCount) => [vi.fn(), itemsCount]);
});

it('should render all elements in default state', () => {
const { container } = renderComponent();
assertBaseElement(container);
Expand Down Expand Up @@ -181,57 +172,6 @@ it('should accept currentIndex prop', () => {
assertBaseElement(container, { currentIndex: 1 });
});

it('should overflow when there is not enough space', () => {
useOverflowMock.mockReturnValue([vi.fn(), 2]);
const { container } = renderComponent();

expect(container.querySelector('.iui-breadcrumbs')).toBeTruthy();
expect(container.querySelector('.iui-breadcrumbs-list')).toBeTruthy();

const breadcrumbs = container.querySelectorAll('.iui-breadcrumbs-item');
expect(breadcrumbs.length).toEqual(3);
expect(breadcrumbs[0].textContent).toEqual('Item 0');
expect(breadcrumbs[1].textContent).toEqual('…');
expect(breadcrumbs[1].firstElementChild?.textContent).toContain('…');
expect(breadcrumbs[2].textContent).toEqual('Item 2');
});

it('should handle overflow when overflowButton is specified', () => {
const onClick = vi.fn();
useOverflowMock.mockReturnValue([vi.fn(), 2]);
const { container } = renderComponent({
overflowButton: (visibleCount) => (
<IconButton onClick={onClick(visibleCount)}>
<SvgMore />
</IconButton>
),
});

expect(container.querySelector('.iui-breadcrumbs')).toBeTruthy();
expect(container.querySelector('.iui-breadcrumbs-list')).toBeTruthy();

const breadcrumbs = container.querySelectorAll('.iui-breadcrumbs-item');
expect(breadcrumbs.length).toEqual(3);
fireEvent.click(breadcrumbs[1]);
expect(onClick).toHaveBeenCalledTimes(1);
expect(onClick).toHaveBeenCalledWith(2);
});

it('should show the last item when only one can be visible', () => {
useOverflowMock.mockReturnValue([vi.fn(), 1]);

const { container } = renderComponent();

expect(container.querySelector('.iui-breadcrumbs')).toBeTruthy();
expect(container.querySelector('.iui-breadcrumbs-list')).toBeTruthy();

const breadcrumbs = container.querySelectorAll('.iui-breadcrumbs-item');
expect(breadcrumbs.length).toEqual(2);
expect(breadcrumbs[0].textContent).toEqual('…');
expect(breadcrumbs[0].firstElementChild?.textContent).toContain('…');
expect(breadcrumbs[1].textContent).toEqual('Item 2');
});

it('should support legacy api', async () => {
const onClick = vi.fn();

Expand Down
4 changes: 2 additions & 2 deletions packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type BreadcrumbsProps = {
* <span>Current level</span>
* </Breadcrumbs>
*/
const BreadcrumbsComponent = React.forwardRef((props, ref) => {
const BreadcrumbsComponent = React.forwardRef((props, forwardedRef) => {
const {
children: childrenProp,
currentIndex = React.Children.count(childrenProp) - 1,
Expand All @@ -130,7 +130,7 @@ const BreadcrumbsComponent = React.forwardRef((props, ref) => {
<Box
as='nav'
className={cx('iui-breadcrumbs', className)}
ref={ref}
ref={forwardedRef}
aria-label='Breadcrumb'
{...rest}
>
Expand Down
141 changes: 1 addition & 140 deletions packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { SvgMore, SvgClose as SvgPlaceholder } from '../../utils/index.js';
import { fireEvent, render } from '@testing-library/react';
import * as UseOverflow from '../../utils/hooks/useOverflow.js';
import { render } from '@testing-library/react';

import { ButtonGroup } from './ButtonGroup.js';
import { Button } from '../Buttons/Button.js';
import { IconButton } from '../Buttons/IconButton.js';

it('should render with two buttons', () => {
const { container } = render(
Expand All @@ -34,101 +31,6 @@ it('should render with four buttons', () => {
expect(container.querySelectorAll('button').length).toBe(4);
});

it.each(['start', 'end'] as const)(
'should handle overflow when overflowButton is specified (%s)',
(overflowPlacement) => {
const scrollWidthSpy = vi
.spyOn(HTMLElement.prototype, 'scrollWidth', 'get')
.mockReturnValueOnce(250)
.mockReturnValue(200);
const offsetWidthSpy = vi
.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
.mockReturnValue(200);

const mockOnClick = vi.fn();

const OverflowGroup = () => {
const buttons = [...Array(3)].map((_, index) => (
<IconButton key={index}>
<SvgPlaceholder />
</IconButton>
));
return (
<ButtonGroup
overflowButton={(overflowStart) => (
<IconButton onClick={() => mockOnClick(overflowStart)}>
<SvgMore />
</IconButton>
)}
overflowPlacement={overflowPlacement}
>
{buttons}
</ButtonGroup>
);
};
const { container } = render(<OverflowGroup />);

expect(container.querySelector('.iui-button-group')).toBeTruthy();

const buttons = container.querySelectorAll('.iui-button');
expect(buttons).toHaveLength(2);
fireEvent.click(overflowPlacement === 'end' ? buttons[1] : buttons[0]);
expect(mockOnClick).toHaveBeenCalledWith(1);

scrollWidthSpy.mockRestore();
offsetWidthSpy.mockRestore();
},
);

it.each(['start', 'end'] as const)(
'should handle overflow when available space is smaller than one element (%s)',
(overflowPlacement) => {
const scrollWidthSpy = vi
.spyOn(HTMLElement.prototype, 'scrollWidth', 'get')
.mockReturnValue(200);
const offsetWidthSpy = vi
.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
.mockReturnValue(50);

const OverflowGroup = () => {
const buttons = [...Array(3)].map((_, index) => (
<IconButton key={index}>
<SvgPlaceholder />
</IconButton>
));
return (
<ButtonGroup
overflowButton={() => (
<IconButton>
<SvgMore />
</IconButton>
)}
overflowPlacement={overflowPlacement}
>
{buttons}
</ButtonGroup>
);
};
const { container } = render(<OverflowGroup />);
const {
container: { firstChild: moreIconButton },
} = render(
<IconButton>
<SvgMore />
</IconButton>,
);

expect(container.querySelector('.iui-button-group')).toBeTruthy();

const buttons = container.querySelectorAll('.iui-button');
expect(buttons).toHaveLength(1);
expect(buttons[0]).toEqual(moreIconButton);

scrollWidthSpy.mockRestore();
offsetWidthSpy.mockRestore();
},
);

it('should work in vertical orientation', () => {
const { container } = render(
<ButtonGroup orientation='vertical'>
Expand All @@ -142,44 +44,3 @@ it('should work in vertical orientation', () => {
expect(group).toBeTruthy();
expect(group.children).toHaveLength(2);
});

it.each`
visibleCount | overflowStart | length | overflowPlacement
${9} | ${1} | ${10} | ${'start'}
${8} | ${2} | ${10} | ${'start'}
${4} | ${6} | ${10} | ${'start'}
${3} | ${7} | ${10} | ${'start'}
${1} | ${9} | ${10} | ${'start'}
${9} | ${8} | ${10} | ${'end'}
${8} | ${7} | ${10} | ${'end'}
${4} | ${3} | ${10} | ${'end'}
${3} | ${2} | ${10} | ${'end'}
${1} | ${0} | ${10} | ${'end'}
`(
'should calculate correct values when overflowPlacement=$overflowPlacement and visibleCount=$visibleCount',
({ visibleCount, overflowStart, length, overflowPlacement }) => {
const useOverflowMock = vi
.spyOn(UseOverflow, 'useOverflow')
.mockReturnValue([vi.fn(), visibleCount]);

const buttons = [...Array(length)].map((_, index) => (
<button key={index}>{index}</button>
));

const { container } = render(
<ButtonGroup
overflowButton={(startIndex) => <span>{startIndex}</span>}
overflowPlacement={overflowPlacement}
>
{buttons}
</ButtonGroup>,
);

expect(container.querySelectorAll('button')).toHaveLength(visibleCount - 1);
expect(container.querySelector('span')).toHaveTextContent(
`${overflowStart}`,
);

useOverflowMock.mockRestore();
},
);
4 changes: 2 additions & 2 deletions packages/itwinui-react/src/core/Select/SelectTagContainer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type SelectTagContainerProps = {

/**
*/
export const SelectTagContainer = React.forwardRef((props, ref) => {
export const SelectTagContainer = React.forwardRef((props, forwardedRef) => {
const { tags: tagsProp, className, ...rest } = props;

const tags = React.useMemo(
Expand All @@ -29,7 +29,7 @@ export const SelectTagContainer = React.forwardRef((props, ref) => {
<OverflowContainer
itemsCount={tags.length}
className={cx('iui-select-tag-container', className)}
ref={ref}
ref={forwardedRef}
{...rest}
>
<SelectTagContainerContent {...props} tags={tags} />
Expand Down
36 changes: 0 additions & 36 deletions packages/itwinui-react/src/core/Table/Table.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
} from '../../utils/index.js';
import { DefaultCell, EditableCell } from './cells/index.js';
import { TablePaginator } from './TablePaginator.js';
import * as UseOverflow from '../../utils/hooks/useOverflow.js';
import { userEvent } from '@testing-library/user-event';
import {
ActionColumn,
Expand Down Expand Up @@ -2587,41 +2586,6 @@ it('should handle unwanted actions on editable cell', async () => {
expect(onSelect).not.toHaveBeenCalled();
});

it('should render data in pages', async () => {
vi.spyOn(UseOverflow, 'useOverflow').mockImplementation((itemsCount) => [
vi.fn(),
itemsCount,
]);
const { container } = renderComponent({
data: mockedData(100),
pageSize: 10,
paginatorRenderer: (props) => <TablePaginator {...props} />,
});

let rows = container.querySelectorAll('.iui-table-body .iui-table-row');
expect(rows).toHaveLength(10);
expect(rows[0].querySelector('.iui-table-cell')?.textContent).toEqual(
'Name1',
);
expect(rows[9].querySelector('.iui-table-cell')?.textContent).toEqual(
'Name10',
);

const pages = container.querySelectorAll<HTMLButtonElement>(
'.iui-table-paginator .iui-table-paginator-page-button',
);
expect(pages).toHaveLength(10);
await userEvent.click(pages[3]);
rows = container.querySelectorAll('.iui-table-body .iui-table-row');
expect(rows).toHaveLength(10);
expect(rows[0].querySelector('.iui-table-cell')?.textContent).toEqual(
'Name31',
);
expect(rows[9].querySelector('.iui-table-cell')?.textContent).toEqual(
'Name40',
);
});

it('should change page size', async () => {
const { container } = renderComponent({
data: mockedData(100),
Expand Down
Loading

0 comments on commit 9cdf466

Please sign in to comment.