diff --git a/.changeset/light-poems-trade.md b/.changeset/light-poems-trade.md new file mode 100644 index 00000000000..7c4ff59be45 --- /dev/null +++ b/.changeset/light-poems-trade.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-css': minor +--- + +`iui-breadcrumbs-list` now stretches to the width of `iui-breadcrumbs`. diff --git a/.changeset/mean-mangos-relax.md b/.changeset/mean-mangos-relax.md new file mode 100644 index 00000000000..d1408491976 --- /dev/null +++ b/.changeset/mean-mangos-relax.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-react': patch +--- + +Fixed a bug in `ComboBox` with `multiple` enabled where the number of tags used to keep changing in an infinite loop in certain specific container widths. As a result of this fix, the overflow behavior has been improved in other components too (e.g. `ButtonGroup`, `Breadcrumbs`, `MiddleTextTruncation`, etc.) diff --git a/packages/itwinui-css/src/breadcrumbs/breadcrumbs.scss b/packages/itwinui-css/src/breadcrumbs/breadcrumbs.scss index 64004a580bc..bce71c12a7c 100644 --- a/packages/itwinui-css/src/breadcrumbs/breadcrumbs.scss +++ b/packages/itwinui-css/src/breadcrumbs/breadcrumbs.scss @@ -20,6 +20,7 @@ list-style-type: none; user-select: none; block-size: 100%; + inline-size: 100%; } .iui-breadcrumbs-item { diff --git a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx index 15eea9a0711..c322feaa9f2 100644 --- a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx +++ b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx @@ -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'; @@ -56,13 +54,6 @@ const assertBaseElement = ( }); }; -const useOverflowMock = vi - .spyOn(UseOverflow, 'useOverflow') - .mockImplementation((items) => [vi.fn(), items.length]); -beforeEach(() => { - useOverflowMock.mockImplementation((items) => [vi.fn(), items.length]); -}); - it('should render all elements in default state', () => { const { container } = renderComponent(); assertBaseElement(container); @@ -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) => ( - - - - ), - }); - - 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(); diff --git a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx index 82b09fe528a..969233521b4 100644 --- a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx +++ b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx @@ -5,8 +5,6 @@ import * as React from 'react'; import cx from 'classnames'; import { - useMergedRefs, - useOverflow, SvgChevronRight, Box, useWarningLogger, @@ -14,6 +12,7 @@ import { import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; import { Button } from '../Buttons/Button.js'; import { Anchor } from '../Typography/Anchor.js'; +import { OverflowContainer } from '../../utils/components/OverflowContainer.js'; type BreadcrumbsProps = { /** @@ -122,62 +121,67 @@ const BreadcrumbsComponent = React.forwardRef((props, ref) => { ...rest } = props; - const [overflowRef, visibleCount] = useOverflow(items); - const refs = useMergedRefs(overflowRef, ref); - return ( - - {visibleCount > 1 && ( - <> - - - - )} - {items.length - visibleCount > 0 && ( + + {(visibleCount: number) => ( <> - - {overflowButton ? ( - overflowButton(visibleCount) - ) : ( - - … + {visibleCount > 1 && ( + <> + + + + )} + {items.length - visibleCount > 0 && ( + <> + + {overflowButton ? ( + overflowButton(visibleCount) + ) : ( + + … + + )} - )} - - + + + )} + {items + .slice( + visibleCount > 1 + ? items.length - visibleCount + 1 + : items.length - 1, + ) + .map((_, _index) => { + const index = + visibleCount > 1 + ? 1 + (items.length - visibleCount) + _index + : items.length - 1; + return ( + + + {index < items.length - 1 && ( + + )} + + ); + })} )} - {items - .slice( - visibleCount > 1 - ? items.length - visibleCount + 1 - : items.length - 1, - ) - .map((_, _index) => { - const index = - visibleCount > 1 - ? 1 + (items.length - visibleCount) + _index - : items.length - 1; - return ( - - - {index < items.length - 1 && ( - - )} - - ); - })} - + ); }) as PolymorphicForwardRefComponent<'nav', BreadcrumbsProps>; diff --git a/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.test.tsx b/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.test.tsx index 44022720fb8..5f56f929178 100644 --- a/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.test.tsx +++ b/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.test.tsx @@ -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( @@ -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) => ( - - - - )); - return ( - ( - mockOnClick(overflowStart)}> - - - )} - overflowPlacement={overflowPlacement} - > - {buttons} - - ); - }; - const { container } = render(); - - 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) => ( - - - - )); - return ( - ( - - - - )} - overflowPlacement={overflowPlacement} - > - {buttons} - - ); - }; - const { container } = render(); - const { - container: { firstChild: moreIconButton }, - } = render( - - - , - ); - - 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( @@ -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) => ( - - )); - - const { container } = render( - {startIndex}} - overflowPlacement={overflowPlacement} - > - {buttons} - , - ); - - expect(container.querySelectorAll('button')).toHaveLength(visibleCount - 1); - expect(container.querySelector('span')).toHaveTextContent( - `${overflowStart}`, - ); - - useOverflowMock.mockRestore(); - }, -); diff --git a/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.tsx b/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.tsx index 00739344889..7f253185417 100644 --- a/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.tsx +++ b/packages/itwinui-react/src/core/ButtonGroup/ButtonGroup.tsx @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; import cx from 'classnames'; -import { useOverflow, useMergedRefs, Box } from '../../utils/index.js'; +import { Box } from '../../utils/index.js'; import type { AnyString, PolymorphicForwardRefComponent, @@ -14,6 +14,7 @@ import { CompositeItem, FloatingDelayGroup, } from '@floating-ui/react'; +import { OverflowContainer } from '../../utils/components/OverflowContainer.js'; // ---------------------------------------------------------------------------- @@ -172,6 +173,7 @@ const BaseGroup = React.forwardRef((props, forwardedRef) => { // ---------------------------------------------------------------------------- +/** Note: If `overflowButton == null`, it behaves like a `BaseGroup`. */ const OverflowGroup = React.forwardRef((props, forwardedRef) => { const { children: childrenProp, @@ -186,14 +188,11 @@ const OverflowGroup = React.forwardRef((props, forwardedRef) => { [childrenProp], ); - const [overflowRef, visibleCount] = useOverflow( - items, - !overflowButton, - orientation, - ); - - return ( - { }, props.className, )} - ref={useMergedRefs(forwardedRef, overflowRef)} - > - {(() => { - if (!(visibleCount < items.length)) { - return items; - } - - const overflowStart = + ref={forwardedRef} + overflowTag={(visibleCount) => { + const firstOverflowingIndex = overflowPlacement === 'start' - ? items.length - visibleCount + ? items.length - visibleCount - 2 : visibleCount - 1; - return ( - <> - {overflowButton && - overflowPlacement === 'start' && - overflowButton(overflowStart)} - - {overflowPlacement === 'start' - ? items.slice(overflowStart + 1) - : items.slice(0, Math.max(0, overflowStart))} - - {overflowButton && - overflowPlacement === 'end' && - overflowButton(overflowStart)} - - ); - })()} + return overflowButton(firstOverflowingIndex); + }} + > + {items} + + ) : ( + + {childrenProp} ); }) as PolymorphicForwardRefComponent< diff --git a/packages/itwinui-react/src/core/ComboBox/ComboBox.test.tsx b/packages/itwinui-react/src/core/ComboBox/ComboBox.test.tsx index 478494784e2..ed33de74e19 100644 --- a/packages/itwinui-react/src/core/ComboBox/ComboBox.test.tsx +++ b/packages/itwinui-react/src/core/ComboBox/ComboBox.test.tsx @@ -642,91 +642,6 @@ it('should update options (does not have selected option in new options list)', expect(input).toHaveValue(''); }); -it('should select multiple options', async () => { - const mockOnChange = vi.fn(); - const options = [0, 1, 2, 3].map((value) => ({ - value, - label: `Item ${value}`, - })); - - const { container } = render( - , - ); - - const inputContainer = container.querySelector( - '.iui-input-grid', - ) as HTMLDivElement; - await userEvent.tab(); - await userEvent.click(screen.getByText('Item 1')); - await userEvent.click(screen.getByText('Item 2')); - await userEvent.click(screen.getByText('Item 3')); - - const tags = inputContainer.querySelectorAll('.iui-select-tag'); - expect(tags.length).toBe(3); - expect(tags[0].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 1', - ); - expect(tags[1].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 2', - ); - expect(tags[2].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 3', - ); -}); - -it('should override multiple selected options', async () => { - const mockOnChange = vi.fn(); - const options = [0, 1, 2, 3].map((value) => ({ - value, - label: `Item ${value}`, - })); - const values = [0, 1]; - - const { container, rerender } = render( - , - ); - - const inputContainer = container.querySelector( - '.iui-input-grid', - ) as HTMLDivElement; - - const tags = inputContainer.querySelectorAll('.iui-select-tag'); - expect(tags.length).toBe(2); - expect(tags[0].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 0', - ); - expect(tags[1].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 1', - ); - - const values2 = [1, 2, 3]; - - rerender( - , - ); - const tags2 = inputContainer.querySelectorAll('.iui-select-tag'); - expect(tags2.length).toBe(3); - expect(tags2[0].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 1', - ); - expect(tags2[1].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 2', - ); - expect(tags2[2].querySelector('.iui-select-tag-label')).toHaveTextContent( - 'Item 3', - ); -}); - it('should handle keyboard navigation when multiple is enabled', async () => { const id = 'test-component'; const mockOnChange = vi.fn(); diff --git a/packages/itwinui-react/src/core/Select/Select.test.tsx b/packages/itwinui-react/src/core/Select/Select.test.tsx index 6d63de0614a..fd67d3167a0 100644 --- a/packages/itwinui-react/src/core/Select/Select.test.tsx +++ b/packages/itwinui-react/src/core/Select/Select.test.tsx @@ -579,47 +579,6 @@ it('should update live region when selection changes', async () => { expect(liveRegion).toHaveTextContent('Item 1, Item 2'); }); -it.each([true, false] as const)( - 'should work in uncontrolled mode (multiple=%s)', - async (multiple) => { - const { container } = render( - + + + ); +} diff --git a/testing/e2e/app/routes/Select/spec.ts b/testing/e2e/app/routes/Select/spec.ts new file mode 100644 index 00000000000..8cede3cd80d --- /dev/null +++ b/testing/e2e/app/routes/Select/spec.ts @@ -0,0 +1,143 @@ +import { test, expect, Page } from '@playwright/test'; + +test.describe('Select', () => { + test.describe('General', () => { + [true, false].forEach((multiple) => { + test(`should work in uncontrolled mode (multiple=${multiple})`, async ({ + page, + }) => { + await page.goto(`/Select?multiple=${multiple}`); + + await page.waitForTimeout(30); + + await page.keyboard.press('Tab'); + await page.keyboard.press('Space'); + + await page.getByRole('option').first().click(); + expect(page.getByRole('combobox')).toHaveText('option 0'); + + if (!multiple) { + await page.getByRole('combobox').click(); + } + + await page.getByRole('option').nth(1).click(); + + if (multiple) { + await page.getByRole('option').first().click(); + } + + expect(page.getByRole('combobox')).toHaveText('option 1'); + }); + }); + }); + + test.describe('Overflow', () => { + test(`should overflow whenever there is not enough space`, async ({ + page, + }) => { + await page.goto(`/Select?multiple=true&value=all`); + + const setContainerSize = getSetContainerSize(page); + const expectOverflowState = getExpectOverflowState(page); + + await page.waitForTimeout(30); + + await expectOverflowState({ + expectedItemLength: 10, + expectedLastTagTextContent: 'Very long option', + }); + + await setContainerSize('600px'); + + await expectOverflowState({ + expectedItemLength: 6, + expectedLastTagTextContent: '+5 item(s)', + }); + }); + + test(`should at minimum always show one overflow tag`, async ({ page }) => { + await page.goto(`/Select?multiple=true&value=all`); + + const setContainerSize = getSetContainerSize(page); + const expectOverflowState = getExpectOverflowState(page); + + await page.waitForTimeout(30); + + await expectOverflowState({ + expectedItemLength: 10, + expectedLastTagTextContent: 'Very long option', + }); + + await setContainerSize('10px'); + + await expectOverflowState({ + expectedItemLength: 1, + expectedLastTagTextContent: '+10 item(s)', + }); + }); + + test(`should always show the selected tag and no overflow tag when only one item is selected`, async ({ + page, + }) => { + await page.goto(`/Select?multiple=true&value=[9]`); + + const setContainerSize = getSetContainerSize(page); + const expectOverflowState = getExpectOverflowState(page); + + await page.waitForTimeout(30); + + await expectOverflowState({ + expectedItemLength: 1, + expectedLastTagTextContent: 'Very long option', + }); + + await setContainerSize('160px'); + + await expectOverflowState({ + expectedItemLength: 1, + expectedLastTagTextContent: 'Very long option', + }); + }); + }); +}); + +// ---------------------------------------------------------------------------- + +const getSetContainerSize = (page: Page) => { + return async (dimension: string | undefined) => { + await page.locator('#container').evaluate( + (element, args) => { + if (args.dimension != null) { + element.style.setProperty('width', args.dimension); + } else { + element.style.removeProperty('width'); + } + }, + { dimension }, + ); + await page.waitForTimeout(30); + }; +}; + +const getExpectOverflowState = (page: Page) => { + return async ({ + expectedItemLength, + expectedLastTagTextContent, + }: { + expectedItemLength: number; + expectedLastTagTextContent: string | undefined; + }) => { + const tags = await page + .locator('div[class$="-select-tag-container"] > span') + .all(); + expect(tags).toHaveLength(expectedItemLength); + + const lastTag = tags[tags.length - 1]; + + if (expectedLastTagTextContent != null) { + expect(await lastTag.textContent()).toBe(expectedLastTagTextContent); + } else { + expect(tags).toHaveLength(0); + } + }; +}; diff --git a/testing/e2e/app/routes/Table/route.tsx b/testing/e2e/app/routes/Table/route.tsx index 6678c491104..eaa8439d453 100644 --- a/testing/e2e/app/routes/Table/route.tsx +++ b/testing/e2e/app/routes/Table/route.tsx @@ -56,10 +56,16 @@ function FiltersTest() { function EverythingElse() { const [searchParams] = useSearchParams(); + const exampleType = searchParams.get('exampleType') || 'default'; const disableResizing = searchParams.get('disableResizing') === 'true'; const columnResizeMode = searchParams.get('columnResizeMode') || 'fit'; const maxWidths = searchParams.getAll('maxWidth'); const minWidths = searchParams.getAll('minWidth'); + const density = (searchParams.get('density') || undefined) as + | 'default' + | 'condensed' + | 'extra-condensed' + | undefined; const isSelectable = searchParams.get('isSelectable') === 'true'; const rows = searchParams.get('rows'); const filter = searchParams.get('filter') === 'true'; @@ -100,12 +106,16 @@ function EverythingElse() { } })(); - const isRowDisabled = React.useCallback( - (rowData: Record) => { - return rowData.name === 'Name3.2'; - }, - [], - ); + const data = React.useMemo( + () => + Array(505) + .fill(null) + .map((_, index) => ({ + name: `Name ${index}`, + description: `Description ${index}`, + })), + [], + ); return ( <>