Skip to content

Radix dropdown -> Headless dropdown #2452

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.

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

Merged
merged 9 commits into from
Sep 18, 2024
Merged

Radix dropdown -> Headless dropdown #2452

merged 9 commits into from
Sep 18, 2024

Conversation

david-crespo
Copy link
Collaborator

@david-crespo david-crespo commented Sep 17, 2024

Radix has given us a bit of trouble here and there and I'd prefer to be all-in on Headless. While working on #2450, decided to poke at converting our dropdown to use Headless. The goal is to keep the API of our wrapper component roughly the same to minimize the diff. Plus we cut the bundle size a tiny bit (384 kB to 376 kB gzipped). Can't drop @radix-ui/react-focus-guards yet because it's still used by the dialog lib.

The four spots the dropdown menu is used are:

  • TopBarPicker (project/instance/vpc picker in header bar)
  • User menu in top right corner
  • Actions menu on table rows
  • More actions menu in top right on detail pages, e.g., instance actions

Copy link

vercel bot commented Sep 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Sep 18, 2024 5:29pm

Copy link
Contributor

@charliepark charliepark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks cleaner, and, in clicking through the vercel deployment, all seems good.

@david-crespo
Copy link
Collaborator Author

Still need to fix this. Radix locks scroll when the dropdown is open to prevent this from happening. Headless has a modal mode that in theory prevents scroll, but it might only affect page-level scrolling. That would be fine in this PR, but I think in #2450 it causes the layout shift mentioned in the description because it causes the scrollbar to disappear. I think for this PR I'll just fix the z-index so it looks normal when scrolling.

2024-09-17-dropdown-menu-z-bug.mp4

@david-crespo
Copy link
Collaborator Author

5c5cdd1 seems good:

2024-09-18-z-topBarPopover.mp4

@david-crespo david-crespo enabled auto-merge (squash) September 18, 2024 17:28
@david-crespo david-crespo merged commit e3d4245 into main Sep 18, 2024
8 checks passed
@david-crespo david-crespo deleted the headless-dropdown branch September 18, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants