Repository navigation
perf(dropdown): Virtualize the All Projects settings submenu - #129659
Draft
sentry-junior[bot] wants to merge 2 commits into
Draft
sentry-junior[bot] wants to merge 2 commits into
sentry-junior[bot] wants to merge 2 commits into
Conversation
The All Projects submenu in the settings navigation rendered a full menu row for every project, and each hover re-rendered every row. Large organizations made opening the submenu and moving through it slow. DropdownMenu submenus now accept virtualizeThreshold. Above it, only the visible rows render, sections keep their group semantics, and keyboard focus scrolls rows that are not rendered into view. The project settings submenu turns this on above 50 projects. Co-Authored-By: Priscila Andre de Oliveira <29228205+priscilawebdev@users.noreply.github.com>
Contributor
Story previewsPreview the stories changed in this PR on the Vercel deployment: Preview deployment: https://sentry-8optxuqhk.sentry.dev |
Move the virtualized row rendering, the virtualizer setup, and its keyboard helpers out of list.tsx into virtualCollection.tsx. Both render paths now share one separator component, so they do not drift apart. Menus with nested sub-menus no longer virtualize. A sub-menu trigger must stay mounted to keep its sub-menu open, so these menus render every row and warn in development instead.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hovering View All Projects in the settings v3 organization menu was slow in organizations with many projects. The submenu rendered a full
DropdownMenuItemfor every project, and each row includes react-aria hooks, aLink, aProjectAvatar, and three tooltips. Each hover changed the submenu'sfocusedKey, which gave every row a newTreeState, so all rows re-rendered on each hover.This PR adds an opt-in
virtualizeThresholdtoDropdownMenusubmenu options. Once a submenu has more items than the threshold, only the visible rows (plus overscan) render, using the existinguseVirtualRows/@tanstack/react-virtualsetup thatCompactSelectalready uses. The project settings submenu turns it on above 50 projects. Smaller orgs, and every otherDropdownMenu, keep the current non-virtualized path.Jest benchmark (React dev build, rows mocked at 32px in a 240px menu), time to open the submenu and per-row hover time, before → after:
Approach / review focus (
static/app/components/core/dropdownMenu/list.tsx)role="group"semantics. A section whose heading is scrolled out of view stays labelled witharia-label.lithat carries the 2xs gap as padding. That lets the virtualizer measure the gap instead of it coming from flexgap, so the visual spacing doesn't change.data-keyagainst the first and last enabled keys instead of the first and last rendered elements. ArrowDown from search focuses the first item through the selection manager.maxWidthas its width.Known limits
DropdownMenure-rendering every row on hover in non-virtualized menus. It only stops mattering here because few rows are mounted. A broader fix (memoizing rows or narrowing what each row receives fromTreeState) could be a follow-up.Not yet verified in a real browser. Coverage so far is jest (2 new virtualized submenu tests, all existing
DropdownMenuandOrganizationDropdownspecs pass) andpnpm run typecheck. Before marking this ready, please sanity-check scrolling, keyboard navigation, and search inpnpm dev-uiagainst a large org.via @priscilawebdev.
--
View Session [Sentry]