Skip to content

feat(menu): bump react-virtual #1745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(menu): bump react-virtual #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions packages/menu/__tests__/Menu.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { MenuProps } from '../src';

import { Popover } from '@launchpad-ui/popover';
import { describe, expect, it, vi } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';

import { render, screen, userEvent, waitFor } from '../../../test/utils';
import { Menu, MenuDivider, MenuItem, MenuSearch } from '../src';
Expand Down Expand Up @@ -31,6 +31,16 @@
);

describe('Menu', () => {
// https://github.com/TanStack/virtual/issues/641#issuecomment-2851908893
beforeEach(() => {
Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
value: 800,
});
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', {
value: 800,
});
});

it('renders', () => {
render(createMenu({ size: 'sm' }));
expect(screen.getByRole('menu')).toBeInTheDocument();
Expand All @@ -39,7 +49,7 @@
it('renders with virtualization', () => {
render(createMenu({ enableVirtualization: true }));
const items = screen.getAllByRole('presentation');
expect(items).toHaveLength(5);
expect(items).toHaveLength(7);
});

it('renders the search field', () => {
Expand Down Expand Up @@ -155,9 +165,9 @@

const user = userEvent.setup();
await user.click(screen.getByText('Target'));
const items = screen.getAllByRole('menuitem');
const items = await screen.findAllByRole('menuitem');

expect(items[0]).toHaveFocus();

Check failure on line 170 in packages/menu/__tests__/Menu.spec.tsx

View workflow job for this annotation

GitHub Actions / Test

__tests__/Menu.spec.tsx > Menu > can cycle through items with keyboard and virtualization enabled 

Error: expect(element).toHaveFocus()

Expected element with focus:
  <button
  class="_Menu-item_df6fdc"
  data-test-id="menu-item"
  id="menu-ctrl-«r5»-item-0"
  role="menuitem"
>
  one
</button>
Received element with focus:
  <div
  class="_Popover-content_2663ad _Popover-content--restrictWidth_2663ad"
  style="opacity: 0;"
  tabindex="-1"
>
  <div
    class="_Popover-scroller_2663ad"
  >
    <div
      class="_Menu_df6fdc _Menu--isVirtual_df6fdc"
      data-test-id="menu"
      role="menu"
    >
      <div
        class="_Menu-item-list_df6fdc"
        data-test-id="menu-item-list"
        role="presentation"
      >
        <div
          class="_VirtualMenu-item-list_df6fdc"
          role="presentation"
          style="height: 94.5px;"
        >
          <div
            class="_VirtualMenu-item_df6fdc"
            role="presentation"
            style="transform: translateY(0px);"
          >
            <button
              class="_Menu-item_df6fdc"
              data-test-id="menu-item"
              id="menu-ctrl-«r5»-item-0"
              role="menuitem"
            >
              one
            </button>
          </div>
          <div
            class="_VirtualMenu-item_df6fdc"
            role="presentation"
            style="transform: translateY(31.5px);"
          >
            <button
              class="_Menu-item_df6fdc"
              data-test-id="menu-item"
              id="menu-ctrl-«r5»-item-1"
              role="menuitem"
            >
              two
            </button>
          </div>
          <div
            class="_VirtualMenu-item_df6fdc"
            role="presentation"
            style="transform: translateY(63px);"
          >
            <button
              class="_Menu-item_df6fdc"
              data-test-id="menu-item"
              id="menu-ctrl-«r5»-item-2"
              role="menuitem"
            >
              three
            </button>
          </div>
        </div>
      </div>
    </div>
  </div>
</div>
 ❯ __tests__/Menu.spec.tsx:170:20
await user.keyboard('{arrowdown}');
expect(items[1]).toHaveFocus();
await user.keyboard('{arrowup}');
Expand Down
2 changes: 1 addition & 1 deletion packages/menu/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"@react-aria/focus": "3.21.0",
"@react-aria/separator": "3.4.11",
"classix": "2.2.0",
"react-virtual": "2.10.4"
"@tanstack/react-virtual": "3.13.12"
},
"peerDependencies": {
"react": "19.1.1",
Expand Down
80 changes: 30 additions & 50 deletions packages/menu/src/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,9 @@ import type { KeyboardEvent, ReactElement, ReactNode } from 'react';
import type { MenuItemProps } from './MenuItem';

import { useFocusManager } from '@react-aria/focus';
import { useVirtualizer } from '@tanstack/react-virtual';
import { cx } from 'classix';
import {
Children,
cloneElement,
useCallback,
useEffect,
useId,
useMemo,
useRef,
useState,
} from 'react';
import { useVirtual } from 'react-virtual';
import { Children, cloneElement, useCallback, useEffect, useId, useMemo, useRef } from 'react';

import { MenuBase } from './MenuBase';
import { MenuDivider } from './MenuDivider';
Expand Down Expand Up @@ -213,15 +204,13 @@ const ItemVirtualizer = <T extends number | string>(props: ItemVirtualizerProps<
const parentRef = useRef<HTMLDivElement | null>(null);
const searchRef = useRef<HTMLInputElement | null>(null);

const [nextFocusValue, setNextFocusValue] = useState<number | null>(null);

const hasSearch = !!searchElement;

const lastVirtualItemIndex = items ? items.length - 1 : 0;

const rowVirtualizer = useVirtual({
size: items !== null ? items.length : 0,
parentRef,
const rowVirtualizer = useVirtualizer({
count: items !== null ? items.length : 0,
getScrollElement: () => parentRef.current,
estimateSize: useCallback(() => itemHeight, [itemHeight]),
overscan,
});
Expand All @@ -238,7 +227,8 @@ const ItemVirtualizer = <T extends number | string>(props: ItemVirtualizerProps<
const focusMenuItem = useCallback(
(index: number) => {
rowVirtualizer.scrollToIndex(index);
setNextFocusValue(index);
const element = getNodeForIndex(index, menuId.current);
element?.focus();
},
[rowVirtualizer],
);
Expand Down Expand Up @@ -314,16 +304,10 @@ const ItemVirtualizer = <T extends number | string>(props: ItemVirtualizerProps<
},
[handleKeyboardFocusInteraction, menuItemClassName, onSelect],
);

useEffect(() => {
if (nextFocusValue !== null) {
requestAnimationFrame(() => {
const element = getNodeForIndex(nextFocusValue, menuId.current);
element?.focus();
});
setNextFocusValue(null);
}
}, [nextFocusValue]);
const element = getNodeForIndex(0, menuId.current);
element?.focus();
}, []);

/**
* Calls handleFocusForward when the user is attempting to focus forward using
Expand Down Expand Up @@ -365,29 +349,25 @@ const ItemVirtualizer = <T extends number | string>(props: ItemVirtualizerProps<
[searchElement, lastVirtualItemIndex, focusMenuItem],
);

const renderItems = useMemo(
() =>
rowVirtualizer.virtualItems.map((virtualRow) => {
if (!items) {
return null;
}
const elem = items[virtualRow.index];
return (
<div
key={virtualRow.index}
ref={virtualRow.measureRef}
role="presentation"
className={styles['VirtualMenu-item']}
style={{
transform: `translateY(${virtualRow.start}px)`,
}}
>
{cloneElement(elem, getItemProps(elem, virtualRow.index))}
</div>
);
}),
[rowVirtualizer.virtualItems, items, getItemProps],
);
const renderItems = rowVirtualizer.getVirtualItems().map((virtualRow) => {
if (!items) {
return null;
}
const elem = items[virtualRow.index];
return (
<div
key={virtualRow.index}
ref={rowVirtualizer.measureElement}
role="presentation"
className={styles['VirtualMenu-item']}
style={{
transform: `translateY(${virtualRow.start}px)`,
}}
>
{cloneElement(elem, getItemProps(elem, virtualRow.index))}
</div>
);
});

return (
<>
Expand All @@ -397,7 +377,7 @@ const ItemVirtualizer = <T extends number | string>(props: ItemVirtualizerProps<
role="presentation"
className={styles['VirtualMenu-item-list']}
style={{
height: `${rowVirtualizer.totalSize}px`,
height: `${rowVirtualizer.getTotalSize()}px`,
}}
>
{renderItems}
Expand Down
38 changes: 20 additions & 18 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading