Repository navigation
feat(navigation): Add searchable project settings submenu - #128150
Conversation
Create a context-free Menu presentation API from master. Compose it from DropdownMenu and CompactSelect while retaining each control’s interaction behavior.
Make Menu own the shared item implementation and retain the existing menuListItem import as a compatibility re-export.
Use Menu.Item selection state for Preferences options and remove CompactSelect's local item padding override so both controls follow the shared menu presentation.
Read React Aria node props directly and keep the menu item generic type without a memo assertion.
Preserve primary styling for selected single-select options and finish the menu item props rename.
Keep menu content responsible for query state and pass the list its value and change handler separately.
Pass the controlled search input as one object while menu content retains ownership of the query.
Use one public search options type for root menus and submenus. Name the private list prop searchState to distinguish its controlled input data.
Expose maxHeight and maxWidth in submenu options, mirroring content sizing props. Keep the project submenu within 240px height and 258px width while allowing smaller content to fit naturally.
Use maxHeight and maxWidth for root menus and submenus, and migrate existing root menu height callers.
Keep shared maxHeight and maxWidth options for root menus and submenus, and cap the organization menu width without forcing it.
Export the existing selector from its original module and remove the additional package entry.
|
bugbot run |
natemoo-re
left a comment
There was a problem hiding this comment.
Shaping up super well! Feel free to tackle my feedback/questions in a follow-up, I don't think any of these are a blocker but they would help with robustness.
| /** | ||
| * An interactive control rendered beside, rather than inside, the focusable menu item. | ||
| */ | ||
| trailingAction?: ExtraContent; |
There was a problem hiding this comment.
Is trailingAction intended for select options too? Adding it to ListItemProps exposes it there, but CompactSelect drops it but the older Select renders it. Curious where we want that contract to live.
There was a problem hiding this comment.
Good catch, it's meant for DropdownMenu only. I moved trailingAction out of the shared ListItemProps and into MenuListItem's own props, and it's now exposed through DropdownMenu's MenuItemProps. Select and CompactSelect no longer pick it up. If a select ever needs an interactive trailing control, we can design it there on purpose, since it has different keyboard and ARIA rules.
| // gets 4px of space on each side. A `md` inset keeps leading items where | ||
| // they sat before the slot existed, at every size. | ||
| paddingLeft={leadingItems ? 'md' : undefined} | ||
| paddingRight={trailingAction ? '3xl' : undefined} |
There was a problem hiding this comment.
Similar to @TkDodo's comment here, do we want this slot to support anything beyond an icon button? It currently accepts arbitrary content, but the fixed right padding is designed around the bookmark star. Do we have an opinion on rendering the label when the action is wider?
There was a problem hiding this comment.
Good point, and thanks for pushing on this. No, the slot isn't meant for anything beyond one icon button. It sits in the same right-hand column as the submenu chevron, so something wider would break that alignment or cover the label.
I have updated the code f7fd72e and now we only accept an icon, a label and onAction function. Hope it is better!
| label: project.slug, | ||
| leadingItems: <ProjectAvatar project={project} size={16} />, | ||
| trailingAction: <BookmarkStar project={project} organization={organization} />, | ||
| onAction: () => navigate(`/settings/${organization.slug}/projects/${project.slug}/`), |
There was a problem hiding this comment.
Could these be links through to, like Settings above? I'd expect cmd+click and "open in new tab" to work for project settings too.
| e.key === 'ArrowUp' && | ||
| searchRef.current && | ||
| menuRef.current && | ||
| document.activeElement === menuRef.current.querySelector('[role^="menuitem"]') |
There was a problem hiding this comment.
Could this boundary account for disabled items? With a disabled row first, Up Arrow from the first enabled row stays in the list instead of returning to search. CompactSelect already checks the first enabled collection key, so I wonder if there's a shared navigation rule here.
There was a problem hiding this comment.
Fixed. Keyboard focus now uses the first and last enabled items, not the first and last DOM items, so Up Arrow from the first enabled row returns to search even when a disabled row comes first. The search field's ArrowDown had the same issue, and both use the same helper now. I added a test for a disabled first row. It follows the same aria-disabled rule CompactSelect uses.
| const SIMPLE_CSS_IDENTIFIER = /^-?(?!\d)\w[-\w]*$/; | ||
|
|
||
| /** Caps a requested menu height by the space available to its overlay. */ | ||
| export function minHeight( |
There was a problem hiding this comment.
I think this would fit best in the shared menu module! minHeight also feels a bit too generic for a global export so I'd prefer something like calculateMenuMinHeight.
Rename minHeight to calculateMenuMinHeight and export it from @sentry/scraps/menu instead of compactSelect utils.
Use `to` instead of an onAction navigate so project settings items support cmd+click and open in new tab, matching the Settings item.
Move trailingAction out of the shared ListItemProps so it is no longer exposed on Select and CompactSelect options. MenuListItem still renders it, and DropdownMenu's MenuItemProps is the only public API for it.
Replace the free-form trailingAction slot with {icon, label, onAction}
so the menu always renders one icon-sized button in the column shared
with the submenu chevron. The organization dropdown owns the bookmark
mutation instead of rendering BookmarkStar inside the menu.
ArrowUp back to search and the search field's ArrowDown used the first menu item in the DOM, which may be disabled. Use the first and last enabled items instead, matching CompactSelect.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5862cd9. Configure here.
|
@TkDodo @natemoo-re Thanks for the great feedback! I've addressed all the comments and the PR looks good to go. I'll go ahead and merge this now. Happy to follow up with any further iterations if needed |
Now it is possible to search for a project in the submenu. See the recording:
Screen.Recording.2026-10-21.at.11.40.28.mov
closes DE-1758