Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rohanchkrabrty
requested review from
paanSinghCoder,
rohilsurana and
rsbh
and removed request for
rohilsurana
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughReplaces the legacy DropdownMenu system with a new Menu system across the codebase: drops DropdownMenu implementation, tests, types, and docs; adds Menu primitives (root, content, item, trigger, submenu, utils); and migrates consuming components, examples, and exports to use Menu. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Trigger as Menu.Trigger
participant Root as Menu.Root
participant Content as Menu.Content
participant Item as Menu.Item
participant Combobox as Autocomplete/Input
User->>Trigger: click
Trigger->>Root: setOpen(true)
Root->>Content: render (open)
alt autocomplete enabled
Content->>Combobox: focus input
User->>Combobox: type "search"
Combobox->>Root: onInputValueChange
Root->>Content: filter items (getMatch)
end
User->>Item: click item
Item->>Root: onClick -> setOpen(false)
Root->>Trigger: update state (closed)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/raystack/components/menu/menu.module.css (1)
11-19:⚠️ Potential issue | 🟡 MinorDead
min-widthfallback — use CSS variable fallback syntax insteadThe
min-width: 80pxon line 11 is fully overridden bymin-width: var(--anchor-width)on line 19. More importantly, if--anchor-widthis ever undefined, CSS resolves the property to its initial value (0) — not80px. The static declaration provides no protection.🛠️ Proposed fix
min-width: 80px; ... - min-width: var(--anchor-width); + min-width: var(--anchor-width, 80px);Then remove the now-redundant standalone
min-width: 80pxline 11, or keep it as a pre-CSS-variables fallback for browsers that don't support custom properties.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu.module.css` around lines 11 - 19, The CSS has a dead fallback where "min-width: 80px" is overridden by "min-width: var(--anchor-width)"; replace the two-line pattern by using the CSS custom property fallback syntax so the min-width uses var(--anchor-width, 80px) (or remove the standalone 80px line and keep the fallback if you prefer older-browsers pre-CSS-variable support), updating the rules that reference min-width and the --anchor-width variable accordingly.
🧹 Nitpick comments (17)
packages/raystack/components/menu/cell.module.css (1)
13-23: Redundant font declarations in highlighted state.Lines 17–20 repeat the exact same
font-weight,font-size,line-height, andletter-spacingvalues already set in the base.cellrule (lines 7–10). Onlyoutline,cursor,border-radius, andbackgroundare new. The duplicates can be removed to reduce maintenance surface.Proposed cleanup
.cell[data-highlighted], .cell[data-popup-open] { outline: none; cursor: pointer; - font-weight: var(--rs-font-weight-regular); - font-size: var(--rs-font-size-small); - line-height: var(--rs-line-height-small); - letter-spacing: var(--rs-letter-spacing-small); border-radius: var(--rs-radius-2); background: var(--rs-color-background-base-primary-hover); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/cell.module.css` around lines 13 - 23, The highlighted/popup state rule .cell[data-highlighted], .cell[data-popup-open] repeats font properties already declared in the base .cell selector; remove the redundant declarations (font-weight, font-size, line-height, letter-spacing) from the highlighted/popup rule and keep only the differing properties (outline, cursor, border-radius, background) so .cell base remains the single source of truth for typography.packages/raystack/components/menu/utils.ts (1)
30-44: SyntheticKeyboardEventuses deprecatedkeyCodeandwhichproperties.
keyCodeandwhichare deprecated in the KeyboardEvent spec. Modern libraries (including Base UI) typically checkevent.key. If the consumer of these events only inspectskey/code, the deprecated fields are unnecessary baggage. If they're needed for a specific Base UI internal, a comment explaining why would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/utils.ts` around lines 30 - 44, The synthetic KeyboardEvent created in dispatchKeyboardEvent (using KEYCODES) sets deprecated properties keyCode and which; remove keyCode and which from the event options and rely on key and code instead, updating the new KeyboardEvent call inside dispatchKeyboardEvent to only include key, code, bubbles (and any other modern fields needed), and if you must keep keyCode/which for a specific Base UI consumer add a short inline comment above dispatchKeyboardEvent explaining that those legacy fields are intentionally preserved and why.packages/raystack/components/menu/menu-root.tsx (2)
94-159: Significant code duplication betweenMenuRootandMenuSubMenu.Both components share nearly identical state management logic: controlled/uncontrolled
inputValue, controlled/uncontrolledopen,setValuecallback,handleOpenChangewith autocomplete reset, ref creation, and context provider setup. The only material differences are the primitive used (MenuPrimitive.RootvsMenuPrimitive.SubmenuRoot) and theparentcontext field.Consider extracting the shared state logic into a custom hook (e.g.,
useMenuState) to reduce duplication.Also applies to: 184-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-root.tsx` around lines 94 - 159, MenuRoot and MenuSubMenu duplicate the same controlled/uncontrolled state logic (internalInputValue, internalOpen, inputValue, open), refs (inputRef, contentRef), callbacks (setValue, handleOpenChange) and context provider wiring; extract that shared logic into a custom hook (e.g., useMenuState) that returns {autocomplete, autocompleteMode, inputRef, contentRef, inputValue, open, isInitialRender, setValue, handleOpenChange} and then have MenuRoot call useMenuState and pass its result into the MenuContext.Provider before rendering MenuPrimitive.Root, and have MenuSubMenu do the same (adding its parent field when populating context); keep existing function names setValue and handleOpenChange and preserve their behavior (including the autocomplete reset and onInputValueChange/onOpenChange callbacks).
116-122: In controlled mode,setInternalInputValueis called needlessly.When
providedInputValueis supplied (controlled), the internal state is still updated on everysetValuecall (line 118), even though it's masked byprovidedInputValue ??on line 114. This causes unnecessary state updates and re-renders. Consider gating the internal setter:Proposed fix
const setValue = useCallback( (value: string) => { - setInternalInputValue(value); + if (providedInputValue === undefined) { + setInternalInputValue(value); + } onInputValueChange?.(value); }, - [onInputValueChange] + [onInputValueChange, providedInputValue] );The same pattern applies to
MenuSubMenu(lines 205–211).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-root.tsx` around lines 116 - 122, The setValue callback currently always calls setInternalInputValue(value) even when the component is in controlled mode (providedInputValue is defined); change setValue (and the analogous callback in MenuSubMenu) to only call setInternalInputValue(value) when providedInputValue is undefined (i.e., uncontrolled), and always still call onInputValueChange?.(value); locate the setValue function and the MenuSubMenu handlers and wrap the internal setter behind a conditional check against providedInputValue to avoid needless state updates and re-renders.packages/raystack/components/data-table/components/filters.tsx (1)
42-63: Memoization oftriggeris ineffective due to unstableavailableFiltersreference.
availableFiltersis a new array created on every render (line 38–40 via.filter()), so including it in theuseMemodependency array defeats the memoization — the callback re-runs every render regardless.If you intend to memoize, either memoize
availableFiltersitself or remove it from the dependency array (it's only used whenchildrenis a function).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/components/filters.tsx` around lines 42 - 63, The useMemo for the trigger constant is ineffective because availableFilters is recreated each render; update the code so availableFilters is stable (or omit it from the dependency list): either memoize availableFilters (e.g., compute availableFilters with useMemo using its true upstream dependency like filters) and keep [children, appliedFiltersSet, availableFilters] for useMemo(trigger), or remove availableFilters from trigger's dependency array and only reference it inside the children-is-function branch (ensuring children and appliedFiltersSet remain in the dependency array); adjust the const availableFilters and the useMemo that defines trigger accordingly, referencing the symbols availableFilters, trigger, useMemo, children, and appliedFiltersSet.packages/raystack/components/menu/cell.tsx (1)
9-16: Remove the unusedtypeprop fromCellPropsand component destructuring.The
typeprop is defined inCellProps(line 11) and destructured (line 16) but is never referenced in the component render. Since consumers (menu-trigger.tsx,menu-item.tsx) don't pass this prop and it has no effect on the component's output, it should be removed from both the type definition and the parameter destructuring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/cell.tsx` around lines 9 - 16, Remove the unused "type" prop from the CellProps type and the Cell component parameter list: delete the "type?: 'select' | 'item';" entry from CellProps and remove "type = 'item'" from the destructured params in the forwardRef for Cell (the function that starts with "({ className, children, leadingIcon, trailingIcon, type = 'item', ...props }"). Ensure no other references to "type" remain in the Cell component or exports.packages/raystack/components/menu/menu-trigger.tsx (1)
3-4: Standardize to deep imports for consistency and improved tree-shaking.Lines 3–4 use inconsistent import styles: line 3 imports from the barrel path
@base-ui/reactwhile line 4 imports from the deep path@base-ui/react/menu. Switch line 3 to the deep path@base-ui/react/autocompleteto align with line 4 and avoid potential tree-shaking issues in bundlers that don't optimize barrel imports.Proposed fix
-import { Autocomplete as AutocompletePrimitive } from '@base-ui/react'; +import { Autocomplete as AutocompletePrimitive } from '@base-ui/react/autocomplete';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-trigger.tsx` around lines 3 - 4, Change the inconsistent barrel import for Autocomplete to a deep import to match Menu's style: replace the current import that brings in Autocomplete as AutocompletePrimitive from '@base-ui/react' with an import from '@base-ui/react/autocomplete' (the Menu import remains from '@base-ui/react/menu'); update the import statement referencing AutocompletePrimitive to the deep path so tree-shaking and consistency are preserved.packages/raystack/components/menu/menu-content.tsx (2)
167-169: Unused parametereinonPointerEnterThe event parameter is captured but never used.
🛠️ Proposed fix
- onPointerEnter={e => { - focusInput(); - }} + onPointerEnter={focusInput}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-content.tsx` around lines 167 - 169, The onPointerEnter handler captures an unused event parameter; remove the unused parameter and directly pass the focusInput handler instead (e.g., use onPointerEnter={focusInput} or an arrow with no args) so the event variable is not declared but focusInput still runs; update the onPointerEnter usage in the menu-content.tsx component accordingly.
64-108:nth-childDOM lookup doesn't account for non-option children — usequerySelectorAllinsteadAll four DOM lookups (
highlightFirstItem,checkAndOpenSubMenu,checkAndCloseSubMenu,blurStaleMenuItem) use[role="option"]:nth-child(n). In CSS,:nth-child(n)counts all children, not only those matching[role="option"]. The index provided byonItemHighlightedis an option-only index. If theAutocompletePrimitive.Listever contains non-option children (e.g., separators, headers), the wrong element is selected and submenu interactions silently fail.♻️ Proposed refactor — replace all four lookups with a stable helper
+ const getOptionAt = useCallback((index: number): Element | null => { + const options = containerRef.current?.querySelectorAll('[role="option"]'); + return options?.[index] ?? null; + }, []); const highlightFirstItem = useCallback(() => { if (!isInitialRender?.current) return; isInitialRender.current = false; - const item = containerRef.current?.querySelector( - '[role="option"]:nth-child(1)' - ); + const item = getOptionAt(0); if (!item) return; item.dispatchEvent(new PointerEvent('mousemove', { bubbles: true })); - }, [isInitialRender]); + }, [isInitialRender, getOptionAt]); const checkAndOpenSubMenu = useCallback(() => { if (highlightedItem.current[0] === -1) return; - const item = containerRef.current?.querySelector( - `[role="option"]:nth-child(${highlightedItem.current[0] + 1})` - ); + const item = getOptionAt(highlightedItem.current[0]); ... - }, []); + }, [getOptionAt]); const checkAndCloseSubMenu = useCallback((e: KeyboardEvent) => { if (highlightedItem.current[0] === -1) return; - const item = containerRef.current?.querySelector( - `[role="option"]:nth-child(${highlightedItem.current[0] + 1})` - ); + const item = getOptionAt(highlightedItem.current[0]); ... - }, []); + }, [getOptionAt]); const blurStaleMenuItem = useCallback((index: number) => { - const item = containerRef.current?.querySelector( - `[role="option"]:nth-child(${index + 1})` - ); + const item = getOptionAt(index); ... - }, []); + }, [getOptionAt]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-content.tsx` around lines 64 - 108, The DOM lookups using `[role="option"]:nth-child(n)` in highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu, and blurStaleMenuItem are wrong because nth-child counts all children; replace them with a helper that does container.querySelectorAll('[role="option"]') and returns the element at the option-only index (e.g., getOptionElementByIndex(containerRef.current, index)); then update highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu, and blurStaleMenuItem to call this helper (use highlightedItem.current[0] or the passed index), keep the existing guards (isElementSubMenuTrigger/isElementSubMenuOpen) and existing dispatchKeyboardEvent/PointerEvent calls.apps/www/src/components/demo/demo.tsx (1)
16-16: Filename mismatch with exported component:linear-dropdown-demo.tsxexportsLinearMenuDemoThe file
apps/www/src/components/linear-dropdown-demo.tsxexports a component namedLinearMenuDemoand uses theMenucomponent internally, but its filename references "dropdown". Consider renaming the file tolinear-menu-demo.tsxto match the exported symbol and align with the component's actual purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/components/demo/demo.tsx` at line 16, The exported component name LinearMenuDemo does not match its file name linear-dropdown-demo.tsx; rename the file to linear-menu-demo.tsx and update the import in demo.tsx (and any other imports) to import LinearMenuDemo from './linear-menu-demo' so the filename reflects the exported symbol and component purpose; ensure the component's internal references (if any) still resolve after the rename.apps/www/src/components/linear-dropdown-demo.tsx (1)
272-272: Redundant explicit type annotation ononInputValueChangecallback.♻️ Proposed fix
- onInputValueChange={(value: string) => setSearchQuery(value)} + onInputValueChange={setSearchQuery}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/components/linear-dropdown-demo.tsx` at line 272, The onInputValueChange prop currently uses a redundant explicit type annotation "(value: string) => setSearchQuery(value)"; remove the ": string" type and pass the callback as onInputValueChange={value => setSearchQuery(value)} (or simply onInputValueChange={setSearchQuery} if compatible) so TypeScript infers the parameter type; update the occurrence near the onInputValueChange prop in the LinearDropdownDemo component referencing setSearchQuery.apps/www/src/app/examples/page.tsx (1)
1543-1567: Three identicalMenublocks — extract to a shared component to reduce duplication.Lines 1543-1567 (Dialog), 1677-1701 (Sheet), and 1815-1839 (Nested Dialog) are character-for-character identical menus. A single
<TeamActionsMenu />component would eliminate the triplication.Also applies to: 1677-1701, 1815-1839
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/app/examples/page.tsx` around lines 1543 - 1567, Extract the repeated Menu JSX into a new reusable component (e.g., TeamActionsMenu) that returns the shared structure (Menu with Menu.Trigger using Button, Menu.Content containing Group, Label, Tooltip-wrapped Menu.Item "Add Member", other Menu.Item entries, Separators, and the danger Menu.Item "Delete Team"); then replace the three identical blocks currently inline in Dialog, Sheet, and Nested Dialog with <TeamActionsMenu /> imports/uses. Ensure the new component exports default (or named) and preserves all original child element types (Menu.Trigger, Menu.Item, Menu.Content, Tooltip) so existing parent components keep the same behavior and styling.packages/raystack/components/breadcrumb/breadcrumb-item.tsx (1)
63-71: Unnecessary optional chain ondropdownItem?.onClick.
dropdownItemis always defined — it's the iteration value of.map()overdropdownItems. The?.is redundant.♻️ Proposed fix
- onClick={dropdownItem?.onClick} + onClick={dropdownItem.onClick}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 63 - 71, In breadcrumb-item.tsx inside the dropdown rendering (the dropdownItems.map loop), remove the unnecessary optional chaining on the onClick handler: change the Menu.Item prop from onClick={dropdownItem?.onClick} to onClick={dropdownItem.onClick} (references: dropdownItems, dropdownItem, Menu.Item, styles['breadcrumb-dropdown-item']) to avoid the redundant ?. usage.packages/raystack/components/menu/__tests__/menu.test.tsx (1)
50-136: Consider adding coverage for submenu, autocomplete, keyboard navigation, andEmptyState.The current suite covers basic open/close, item clicks, disabled state, and controlled
onOpenChange. Given the significant new surface area (autocomplete mode,shouldFilterbehavior,EmptyState,Submenu/SubmenuTrigger/SubmenuContent, keyboard interactions), these are left untested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/__tests__/menu.test.tsx` around lines 50 - 136, Tests lack coverage for submenu, autocomplete/shouldFilter behavior, keyboard navigation, and EmptyState; add focused unit tests that render the component variants (use Menu.Submenu with Menu.SubmenuTrigger and Menu.SubmenuContent to verify nested opening/closing and item activation), render Menu in autocomplete mode (Menu.Autocomplete or the prop that enables autocomplete) to assert filtering behavior and that shouldFilter toggles filtering results, add a test rendering the EmptyState to assert it appears when no items match, and add keyboard interaction tests (use fireEvent.keyDown on the trigger and menu to simulate ArrowDown/ArrowUp/Enter/Escape and assert focus/menu open state and onClick/onOpenChange are invoked). Reuse existing helpers like renderAndOpenDropdown and BasicDropdown to mount components, use screen queries to assert presence/absence and aria attributes, and mock callbacks with vi.fn() to assert handlers are called.apps/www/src/content/docs/components/menu/demo.ts (1)
275-280:renderMenureads outersearchQuerystate for the empty-state guard instead of thequeryparameter.if (searchQuery && filteredItems.length === 0) { // outer stateSince
renderMenuis always called asrenderMenu(menuData, searchQuery), they're always equal at call time and there's no current bug. However, if the call-site ever changes (e.g. nested recursion with a sub-query), the stale closure would produce incorrect behaviour. Usingquerythroughout keeps the function self-contained.♻️ Proposed fix
- if (searchQuery && filteredItems.length === 0) { + if (query && filteredItems.length === 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/content/docs/components/menu/demo.ts` around lines 275 - 280, The empty-state guard inside renderMenu uses the outer searchQuery state instead of the function parameter query, which can cause stale-closure bugs; update the condition that checks for no results to use the local query parameter (i.e., replace uses of searchQuery with query) so renderMenu is self-contained—look for the renderMenu function and the line that reads "if (searchQuery && filteredItems.length === 0)" and change it to use query, keeping filterMenuItems and filteredItems unchanged.packages/raystack/components/menu/menu-misc.tsx (2)
17-21:cx(className)inMenuGroupis a no-op — consider either removing it or adding a base style.Every sibling component (
MenuLabel,MenuSeparator,MenuEmptyState) merges astyles.*base class viacx.MenuGrouppasses onlyclassNametocx, which is equivalent to justclassName. If there is an intentional base style for the group inmenu.module.css, it should be merged here; otherwise the call tocxcan be dropped.♻️ Option A — no base style intended
- <MenuPrimitive.Group ref={ref} className={cx(className)} {...props}> + <MenuPrimitive.Group ref={ref} className={className} {...props}>♻️ Option B — base style exists in the CSS module
- <MenuPrimitive.Group ref={ref} className={cx(className)} {...props}> + <MenuPrimitive.Group ref={ref} className={cx(styles.group, className)} {...props}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-misc.tsx` around lines 17 - 21, The MenuGroup component currently calls cx(className) which is a no-op; either remove cx and pass className directly on MenuPrimitive.Group, or merge a base style by changing the className to cx(styles.group, className) (ensure styles.group exists in menu.module.css). Update the MenuGroup export in menu-misc.tsx to use one of these options and adjust the CSS module to include a .group rule if you choose the base-style approach.
9-23:refis silently unset whenshouldFilteristrue.When
shouldFilteris true the component returns<Fragment>{children}</Fragment>, which renders no DOM node. Any consumer that attaches arefto<Menu.Group>and reads it (e.g. for layout measurements) will receivenullin filter mode without any indication. This is likely intentional given the design intent, but worth a brief inline comment for future maintainers.♻️ Proposed clarifying comment
if (shouldFilter) { + // ref is intentionally not forwarded here: no DOM element is rendered during filter mode return <Fragment>{children}</Fragment>; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-misc.tsx` around lines 9 - 23, The early return in the MenuGroup forwardRef (the "if (shouldFilter) return <Fragment>{children}</Fragment>;") silently drops the ref because Fragment renders no DOM node; add a brief inline comment above that return explaining that when shouldFilter is true the component intentionally does not render a DOM element so any consumer ref passed to MenuGroup will be null (mention MenuGroup, shouldFilter, forwardRef and MenuPrimitive.Group), and note that if callers need a stable DOM ref in filter mode they should wrap Menu.Group or use an alternative wrapper (do not change behavior here, just document it).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/app/examples/menu/page.tsx`:
- Around line 25-32: The runtime crash is caused by using non-existent
properties Menu.SubMenu, Menu.SubTrigger, and Menu.SubContent; update the three
menu blocks to reference the exported component names Menu.Submenu,
Menu.SubmenuTrigger, and Menu.SubmenuContent instead (replace every occurrence
in the blocks around the existing Menu usage at the three locations), ensuring
all usages match the exported symbols in menu.tsx so React receives valid
component references.
- Around line 41-51: Duplicate Menu.Item elements both use value='remove' which
creates duplicated entries in the searchable Menu; remove the unintended second
occurrence of <Menu.Item value='remove'>Delete...</Menu.Item> (the one after
Menu.SubMenu) so only one Menu.Item with value='remove' remains, ensuring
Menu.Item and value props are unique for correct autocomplete/filter behavior.
In `@apps/www/src/app/examples/page.tsx`:
- Line 1565: The Menu.Item usage is passing color='danger' which currently leaks
an invalid HTML attribute and has no styling; update the Menu item
implementation to either remove support for color or properly implement it by
adding a color prop to MenuItemProps and propagating it safely (e.g., map color
to a data-color attribute or a className in the Cell wrapper inside the Menu
item implementation so it is not passed as a raw DOM prop), add CSS rules for
the danger variant (matching Button/Spinner red styling), and ensure the prop is
not forwarded to the underlying <div> (adjust Cell or the component that spreads
...props to strip color or use a mapping function). Reference: Menu.Item,
MenuItemProps, MenuPrimitive.Item.Props, CellBaseProps, and Cell.
In `@apps/www/src/components/ai/page-actions.tsx`:
- Around line 257-276: Replace the current pattern that wraps <Menu.Item> inside
an <a> (in the items.map loop) with Base UI’s idiomatic use of Menu.Item’s
render prop: stop rendering an outer anchor and instead return an anchor element
from the Menu.Item render callback so the incoming props (onClick, onKeyDown,
role, aria-*, etc.) are merged onto the same DOM node; ensure the anchor uses
href={item.href.toString()} and rel/target as before, and recreate
leadingIcon/trailingIcon visually inside the render output (or move their markup
into the custom anchor) so icons and accessibility attributes are applied to the
same element and keyboard/ARIA behavior is correct.
In `@apps/www/src/content/docs/components/menu/demo.ts`:
- Line 253: The filter currently lowercases item but compares it to the raw
simpleSearchQuery, causing case-sensitive misses; update the filtering logic
where simpleSearchQuery is used (the .filter(...) in the Manual Autocomplete
demo) to normalize the query (e.g., const q =
simpleSearchQuery.toLowerCase().trim() or similar) and compare
item.toLowerCase().includes(q) so both sides are lowercased (and optionally
trimmed) before matching.
- Around line 395-496: The snippet labeled "data.ts" actually contains JSX
(icons like <Download /> and <Calendar />) so update the demo tab label from
"data.ts" to "data.tsx" so consumers copy a correct TSX file; locate the object
with label: 'data.ts' in this demo (the code block that defines MenuItem and
menuData) and change that string to 'data.tsx' so the example and filename match
the JSX content.
- Around line 28-38: The generated playground markup returned by getCode
mistakenly places <Menu.Item>All (.zip)</Menu.Item> as a direct child of
Menu.Submenu (alongside Menu.SubmenuTrigger and Menu.SubmenuContent); move that
Menu.Item into the Menu.SubmenuContent so all submenu items are children of
Menu.SubmenuContent (match the structure used in the autocompleteDemo "Default
Autocomplete" example) and update getCode to emit Menu.SubmenuTrigger, then
Menu.SubmenuContent containing the Menu.Item entries.
In `@apps/www/src/content/docs/components/menu/index.mdx`:
- Line 83: The docs reference non-existent properties on the Menu object
(Menu.SubTrigger, Menu.SubContent, Menu.SubMenu); replace those references with
the actual exported API names from the Menu module (use the named exports the
module provides rather than accessing them as properties on Menu), update the
example code blocks and prose to import and use the correct symbols (e.g.,
replace usages of Menu.SubTrigger/Menu.SubContent/Menu.SubMenu with the module’s
real exports), and verify the example page runs without runtime errors.
- Line 53: Fix two copy nits in the Menu docs: add a missing space after the
inline code snippet so "`role='option'`when" becomes "`role='option'` when", and
simplify the duplicated phrase "renders an inline search input inside the popup
with a search input" to something concise like "renders an inline search input
inside the popup" (or "renders an inline search input inside the popup for
filtering"); update the text in index.mdx where those exact phrases appear.
In `@apps/www/src/content/docs/components/menu/props.ts`:
- Around line 1-209: This file references React types (React.ReactElement,
React.ReactNode, React.CSSProperties) but lacks a React import, causing TS
errors; add a top-of-file import for React (e.g., import React from 'react' or
import type { ReactElement, ReactNode, CSSProperties } from 'react') so the
types used in interfaces like MenuRootProps, MenuContentProps, MenuItemProps,
MenuSubContentProps, etc., resolve correctly.
In `@packages/raystack/components/data-table/components/filters.tsx`:
- Line 67: The Menu.Trigger line in the Filters component unsafely casts trigger
(typed ReactNode) to React.ReactElement and uses odd `{}` children; update
Filters to avoid the downcast by rendering the trigger only if it's a valid
element (use React.isValidElement(trigger) ? trigger : null) or tighten the
Trigger prop type to ReactElement, and change the JSX to a self-closing element
(<Menu.Trigger render={...} />) so Menu.Trigger receives a proper element or
null instead of an unsafe cast and empty children; reference the trigger prop in
Filters and the Menu.Trigger usage to apply the fix.
In `@packages/raystack/components/menu/__tests__/menu.test.tsx`:
- Line 2: Remove the unused userEvent import and fix misleading async/await
usage: delete the import of userEvent since it’s never used, then remove
unnecessary await keywords on synchronous calls (e.g., the await before
fireEvent.click(...) inside renderAndOpenDropdown and the await before
render(...) in the onOpenChange test). Alternatively, if you intended to use
async user interactions, replace fireEvent.click(...) with userEvent.click(...)
and make the helper/tests async appropriately; target the renderAndOpenDropdown
helper and the onOpenChange test when making these changes.
In `@packages/raystack/components/menu/menu-item.tsx`:
- Around line 34-38: Remove the leftover debug comments in the MenuItem render
invocation: delete the three commented lines "// render={cell}", "//
aria-selected={false}", and "// data-selected={undefined}" in the JSX where
MenuPrimitive.Item is used (the line with render={<MenuPrimitive.Item
render={cell} />} and the surrounding {...props}); keep only the active props
and render prop to ensure no WIP artifacts remain in the MenuItem component.
- Around line 50-54: The onFocus handler in Menu.Item (the inline handler
calling e.stopPropagation(), e.preventDefault(), and e.preventBaseUIHandler())
is suppressing Base UI's focus processing and risks breaking keyboard
navigation; change this to only suppress focus when the specific scroll-on-focus
condition is met (e.g., detect event.target/source or a prop like
suppressScrollOnFocus) instead of blanket suppression, keep Base UI focus
behavior otherwise, and add keyboard navigation tests for Menu (cover
ArrowUp/ArrowDown, Home, End, Escape) to assert focus movement and ARIA updates
so regressions are caught; refer to the onFocus handler and Menu.Item in
menu-item.tsx and to preventBaseUIHandler when implementing the conditional
suppression and tests.
In `@packages/raystack/components/menu/menu-root.tsx`:
- Around line 150-156: The component hardcodes loopFocus={false} on
MenuPrimitive.Root, which contradicts the documented default of true; update the
MenuPrimitive.Root invocation (the block using open,
onOpenChange/handleOpenChange and {...props}) to align with docs by either
removing the explicit loopFocus prop so the primitive's default (true) is used,
or set loopFocus to props.loopFocus ?? true so consumers can override but the
effective default is true.
In `@packages/raystack/components/menu/utils.ts`:
- Around line 3-15: getMatch can return undefined when search is non-empty but
both value is undefined and getChildrenValue(children) returns null; update the
return in getMatch to always produce a boolean by coercing the expression, e.g.,
wrap the current OR expression with a boolean coercion (!!(...)) or use the
nullish fallback ((...) ?? false). Locate the getMatch function and the use of
getChildrenValue(children) in packages/raystack/components/menu/utils.ts and
change the final return to explicitly return a boolean.
- Around line 17-23: getChildrenValue currently calls .toString() on non-string
children which yields "[object Object]" for React elements; update
getChildrenValue to only return a string when children is actually a string or
when children is a React element that exposes a string-valued props.value;
specifically, if children is a string return it, if children is a ReactElement
check for (children.props && typeof children.props.value === 'string') and
return that, otherwise return null (and optionally log or throw a clear message
that a value prop is required for non-string children) so search filtering
doesn't match "[object Object]".
---
Outside diff comments:
In `@packages/raystack/components/menu/menu.module.css`:
- Around line 11-19: The CSS has a dead fallback where "min-width: 80px" is
overridden by "min-width: var(--anchor-width)"; replace the two-line pattern by
using the CSS custom property fallback syntax so the min-width uses
var(--anchor-width, 80px) (or remove the standalone 80px line and keep the
fallback if you prefer older-browsers pre-CSS-variable support), updating the
rules that reference min-width and the --anchor-width variable accordingly.
---
Nitpick comments:
In `@apps/www/src/app/examples/page.tsx`:
- Around line 1543-1567: Extract the repeated Menu JSX into a new reusable
component (e.g., TeamActionsMenu) that returns the shared structure (Menu with
Menu.Trigger using Button, Menu.Content containing Group, Label, Tooltip-wrapped
Menu.Item "Add Member", other Menu.Item entries, Separators, and the danger
Menu.Item "Delete Team"); then replace the three identical blocks currently
inline in Dialog, Sheet, and Nested Dialog with <TeamActionsMenu />
imports/uses. Ensure the new component exports default (or named) and preserves
all original child element types (Menu.Trigger, Menu.Item, Menu.Content,
Tooltip) so existing parent components keep the same behavior and styling.
In `@apps/www/src/components/demo/demo.tsx`:
- Line 16: The exported component name LinearMenuDemo does not match its file
name linear-dropdown-demo.tsx; rename the file to linear-menu-demo.tsx and
update the import in demo.tsx (and any other imports) to import LinearMenuDemo
from './linear-menu-demo' so the filename reflects the exported symbol and
component purpose; ensure the component's internal references (if any) still
resolve after the rename.
In `@apps/www/src/components/linear-dropdown-demo.tsx`:
- Line 272: The onInputValueChange prop currently uses a redundant explicit type
annotation "(value: string) => setSearchQuery(value)"; remove the ": string"
type and pass the callback as onInputValueChange={value =>
setSearchQuery(value)} (or simply onInputValueChange={setSearchQuery} if
compatible) so TypeScript infers the parameter type; update the occurrence near
the onInputValueChange prop in the LinearDropdownDemo component referencing
setSearchQuery.
In `@apps/www/src/content/docs/components/menu/demo.ts`:
- Around line 275-280: The empty-state guard inside renderMenu uses the outer
searchQuery state instead of the function parameter query, which can cause
stale-closure bugs; update the condition that checks for no results to use the
local query parameter (i.e., replace uses of searchQuery with query) so
renderMenu is self-contained—look for the renderMenu function and the line that
reads "if (searchQuery && filteredItems.length === 0)" and change it to use
query, keeping filterMenuItems and filteredItems unchanged.
In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx`:
- Around line 63-71: In breadcrumb-item.tsx inside the dropdown rendering (the
dropdownItems.map loop), remove the unnecessary optional chaining on the onClick
handler: change the Menu.Item prop from onClick={dropdownItem?.onClick} to
onClick={dropdownItem.onClick} (references: dropdownItems, dropdownItem,
Menu.Item, styles['breadcrumb-dropdown-item']) to avoid the redundant ?. usage.
In `@packages/raystack/components/data-table/components/filters.tsx`:
- Around line 42-63: The useMemo for the trigger constant is ineffective because
availableFilters is recreated each render; update the code so availableFilters
is stable (or omit it from the dependency list): either memoize availableFilters
(e.g., compute availableFilters with useMemo using its true upstream dependency
like filters) and keep [children, appliedFiltersSet, availableFilters] for
useMemo(trigger), or remove availableFilters from trigger's dependency array and
only reference it inside the children-is-function branch (ensuring children and
appliedFiltersSet remain in the dependency array); adjust the const
availableFilters and the useMemo that defines trigger accordingly, referencing
the symbols availableFilters, trigger, useMemo, children, and appliedFiltersSet.
In `@packages/raystack/components/menu/__tests__/menu.test.tsx`:
- Around line 50-136: Tests lack coverage for submenu, autocomplete/shouldFilter
behavior, keyboard navigation, and EmptyState; add focused unit tests that
render the component variants (use Menu.Submenu with Menu.SubmenuTrigger and
Menu.SubmenuContent to verify nested opening/closing and item activation),
render Menu in autocomplete mode (Menu.Autocomplete or the prop that enables
autocomplete) to assert filtering behavior and that shouldFilter toggles
filtering results, add a test rendering the EmptyState to assert it appears when
no items match, and add keyboard interaction tests (use fireEvent.keyDown on the
trigger and menu to simulate ArrowDown/ArrowUp/Enter/Escape and assert
focus/menu open state and onClick/onOpenChange are invoked). Reuse existing
helpers like renderAndOpenDropdown and BasicDropdown to mount components, use
screen queries to assert presence/absence and aria attributes, and mock
callbacks with vi.fn() to assert handlers are called.
In `@packages/raystack/components/menu/cell.module.css`:
- Around line 13-23: The highlighted/popup state rule .cell[data-highlighted],
.cell[data-popup-open] repeats font properties already declared in the base
.cell selector; remove the redundant declarations (font-weight, font-size,
line-height, letter-spacing) from the highlighted/popup rule and keep only the
differing properties (outline, cursor, border-radius, background) so .cell base
remains the single source of truth for typography.
In `@packages/raystack/components/menu/cell.tsx`:
- Around line 9-16: Remove the unused "type" prop from the CellProps type and
the Cell component parameter list: delete the "type?: 'select' | 'item';" entry
from CellProps and remove "type = 'item'" from the destructured params in the
forwardRef for Cell (the function that starts with "({ className, children,
leadingIcon, trailingIcon, type = 'item', ...props }"). Ensure no other
references to "type" remain in the Cell component or exports.
In `@packages/raystack/components/menu/menu-content.tsx`:
- Around line 167-169: The onPointerEnter handler captures an unused event
parameter; remove the unused parameter and directly pass the focusInput handler
instead (e.g., use onPointerEnter={focusInput} or an arrow with no args) so the
event variable is not declared but focusInput still runs; update the
onPointerEnter usage in the menu-content.tsx component accordingly.
- Around line 64-108: The DOM lookups using `[role="option"]:nth-child(n)` in
highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu, and
blurStaleMenuItem are wrong because nth-child counts all children; replace them
with a helper that does container.querySelectorAll('[role="option"]') and
returns the element at the option-only index (e.g.,
getOptionElementByIndex(containerRef.current, index)); then update
highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu, and
blurStaleMenuItem to call this helper (use highlightedItem.current[0] or the
passed index), keep the existing guards
(isElementSubMenuTrigger/isElementSubMenuOpen) and existing
dispatchKeyboardEvent/PointerEvent calls.
In `@packages/raystack/components/menu/menu-misc.tsx`:
- Around line 17-21: The MenuGroup component currently calls cx(className) which
is a no-op; either remove cx and pass className directly on MenuPrimitive.Group,
or merge a base style by changing the className to cx(styles.group, className)
(ensure styles.group exists in menu.module.css). Update the MenuGroup export in
menu-misc.tsx to use one of these options and adjust the CSS module to include a
.group rule if you choose the base-style approach.
- Around line 9-23: The early return in the MenuGroup forwardRef (the "if
(shouldFilter) return <Fragment>{children}</Fragment>;") silently drops the ref
because Fragment renders no DOM node; add a brief inline comment above that
return explaining that when shouldFilter is true the component intentionally
does not render a DOM element so any consumer ref passed to MenuGroup will be
null (mention MenuGroup, shouldFilter, forwardRef and MenuPrimitive.Group), and
note that if callers need a stable DOM ref in filter mode they should wrap
Menu.Group or use an alternative wrapper (do not change behavior here, just
document it).
In `@packages/raystack/components/menu/menu-root.tsx`:
- Around line 94-159: MenuRoot and MenuSubMenu duplicate the same
controlled/uncontrolled state logic (internalInputValue, internalOpen,
inputValue, open), refs (inputRef, contentRef), callbacks (setValue,
handleOpenChange) and context provider wiring; extract that shared logic into a
custom hook (e.g., useMenuState) that returns {autocomplete, autocompleteMode,
inputRef, contentRef, inputValue, open, isInitialRender, setValue,
handleOpenChange} and then have MenuRoot call useMenuState and pass its result
into the MenuContext.Provider before rendering MenuPrimitive.Root, and have
MenuSubMenu do the same (adding its parent field when populating context); keep
existing function names setValue and handleOpenChange and preserve their
behavior (including the autocomplete reset and onInputValueChange/onOpenChange
callbacks).
- Around line 116-122: The setValue callback currently always calls
setInternalInputValue(value) even when the component is in controlled mode
(providedInputValue is defined); change setValue (and the analogous callback in
MenuSubMenu) to only call setInternalInputValue(value) when providedInputValue
is undefined (i.e., uncontrolled), and always still call
onInputValueChange?.(value); locate the setValue function and the MenuSubMenu
handlers and wrap the internal setter behind a conditional check against
providedInputValue to avoid needless state updates and re-renders.
In `@packages/raystack/components/menu/menu-trigger.tsx`:
- Around line 3-4: Change the inconsistent barrel import for Autocomplete to a
deep import to match Menu's style: replace the current import that brings in
Autocomplete as AutocompletePrimitive from '@base-ui/react' with an import from
'@base-ui/react/autocomplete' (the Menu import remains from
'@base-ui/react/menu'); update the import statement referencing
AutocompletePrimitive to the deep path so tree-shaking and consistency are
preserved.
In `@packages/raystack/components/menu/utils.ts`:
- Around line 30-44: The synthetic KeyboardEvent created in
dispatchKeyboardEvent (using KEYCODES) sets deprecated properties keyCode and
which; remove keyCode and which from the event options and rely on key and code
instead, updating the new KeyboardEvent call inside dispatchKeyboardEvent to
only include key, code, bubbles (and any other modern fields needed), and if you
must keep keyCode/which for a specific Base UI consumer add a short inline
comment above dispatchKeyboardEvent explaining that those legacy fields are
intentionally preserved and why.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/www/src/content/docs/components/menu/demo.ts (1)
263-341: Linear demo code preview omits necessary imports — users will need to infer them.The
index.tsxpreview referencesuseState,Fragment,ChevronRight,Button,Menu,MenuItem,filterMenuItems, andmenuDatawithout showing imports. While the companion tabs coverutils.tsanddata.tsx, there's no import block in the main snippet. Consider adding an import preamble for copy-paste friendliness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/src/content/docs/components/menu/demo.ts` around lines 263 - 341, Add a short import preamble to the index.tsx snippet so consumers can copy/paste: import React hooks/types and components referenced (useState, Fragment), UI/icons (ChevronRight, Button), the Menu component and MenuItem type, and helper/data (filterMenuItems, menuData). Update the codePreview `index.tsx` entry in linearDemo to include that import block at the top so symbols like useState, Fragment, ChevronRight, Button, Menu, MenuItem, filterMenuItems, and menuData are defined.packages/raystack/components/menu/menu-trigger.tsx (1)
60-89: User-suppliedrenderprop silently overrides the autocomplete integration.
renderis not destructured fromprops, so{...props}on line 84 is applied after the explicitrenderon line 63. In React, later JSX attributes win — meaning a user passingrenderwill silently bypass theAutocompletePrimitive.Itemwrapping, breaking autocomplete behavior.Destructure
renderto prevent accidental override (or explicitly compose it):Proposed fix
( { children, value, trailingIcon = <TriangleRightIcon />, leadingIcon, + render: _render, ...props }, ref ) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-trigger.tsx` around lines 60 - 89, The MenuPrimitive.SubmenuTrigger's explicit render prop (used to wrap the cell in AutocompletePrimitive.Item when parent?.autocomplete is true) is being overridden by the {...props} spread because render isn't destructured from props; change the component to pull render out of props (e.g. const { render, ...rest } = props) and spread rest instead of props into MenuPrimitive.SubmenuTrigger so user-supplied render cannot silently override the autocomplete wrapper (or alternatively explicitly compose a user render with the internal render when parent?.autocomplete is true); reference MenuPrimitive.SubmenuTrigger, props, render, parent?.autocomplete, and AutocompletePrimitive.Item when applying the change.packages/raystack/components/menu/menu-content.tsx (1)
61-105: Prefer Base UI's documented event APIs over synthetic DOM event dispatch.The
highlightFirstItem,checkAndOpenSubMenu,checkAndCloseSubMenu, andblurStaleMenuItemfunctions dispatch syntheticPointerEventandKeyboardEventinstances to drive Base UI's state machine. While this works, it bypasses Base UI's designed event API. Base UI provides proper mechanisms through:
onOpenChangecallbacks witheventDetailsobject containing the nativeKeyboardEventandeventDetails.reasoneventDetails.cancel()to prevent state changes andeventDetails.allowPropagation()to manage event bubblingUsing Base UI's documented APIs would be more maintainable and decoupled from Base UI's internal event processing implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/menu/menu-content.tsx` around lines 61 - 105, The handlers highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu and blurStaleMenuItem currently synthesize PointerEvent/KeyboardEvent and dispatch them to drive Base UI; instead, replace those synthetic dispatches with Base UI's documented event APIs: invoke the component's onOpenChange (or the specific open/close callback) with an eventDetails object carrying the original/native event (or a constructed details object), use eventDetails.reason to indicate the trigger, and call eventDetails.cancel() or eventDetails.allowPropagation() as needed to control state changes and bubbling; in practice remove usages of new PointerEvent/KeyboardEvent and dispatchKeyboardEvent, and call the menu's onOpenChange/open/close handlers directly from those functions (highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu, blurStaleMenuItem), passing appropriate eventDetails and the native KeyboardEvent when available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/content/docs/components/menu/index.mdx`:
- Line 144: Update the heading string "### Linear inspired Menu" to hyphenate
the compound adjective so it reads "### Linear-inspired Menu"; locate the
heading text in the MDX file (search for the exact string "### Linear inspired
Menu") and replace it with "### Linear-inspired Menu".
---
Duplicate comments:
In `@apps/www/src/content/docs/components/menu/index.mdx`:
- Line 56: The inline code span currently includes a trailing space
(`role='option' `) which renders oddly; update the MDX sentence so the code span
is `role='option'` (no trailing space) and ensure a regular space follows the
closing backtick before "when" so the text reads "...`role='option'` when used
in an autocomplete menu." Reference the inline code token `role='option'` to
locate and fix it.
- Line 48: The sentence in the docs currently reads "The container that holds
the menu items. When autocomplete is enabled, renders an inline search input
inside the popup with a search input." — remove the redundant tail "with a
search input" so it reads "The container that holds the menu items. When
autocomplete is enabled, renders an inline search input inside the popup."
Update the text in apps/www/src/content/docs/components/menu/index.mdx where
that sentence appears (look for the exact phrase "renders an inline search input
inside the popup with a search input") to eliminate the duplication.
In `@packages/raystack/components/menu/menu-item.tsx`:
- Around line 42-55: The onFocus handler on MenuPrimitive.Item currently
unconditionally suppresses focus (calling stopPropagation(), preventDefault(),
preventBaseUIHandler()) and also overrides any user onFocus because {...props}
is spread before the handler; fix by composing handlers: destructure props to
extract onFocus (e.g. const { onFocus, ...rest } = props), implement a
handleFocus(e) that only calls
e.stopPropagation()/e.preventDefault()/e.preventBaseUIHandler() for the specific
autocomplete case (or when a clear flag is present) and then calls onFocus?.(e),
and pass {...rest} and onFocus={handleFocus} to MenuPrimitive.Item (keeping ref
and render={cell} intact) so Base UI focus is preserved and user handlers are
invoked.
---
Nitpick comments:
In `@apps/www/src/content/docs/components/menu/demo.ts`:
- Around line 263-341: Add a short import preamble to the index.tsx snippet so
consumers can copy/paste: import React hooks/types and components referenced
(useState, Fragment), UI/icons (ChevronRight, Button), the Menu component and
MenuItem type, and helper/data (filterMenuItems, menuData). Update the
codePreview `index.tsx` entry in linearDemo to include that import block at the
top so symbols like useState, Fragment, ChevronRight, Button, Menu, MenuItem,
filterMenuItems, and menuData are defined.
In `@packages/raystack/components/menu/menu-content.tsx`:
- Around line 61-105: The handlers highlightFirstItem, checkAndOpenSubMenu,
checkAndCloseSubMenu and blurStaleMenuItem currently synthesize
PointerEvent/KeyboardEvent and dispatch them to drive Base UI; instead, replace
those synthetic dispatches with Base UI's documented event APIs: invoke the
component's onOpenChange (or the specific open/close callback) with an
eventDetails object carrying the original/native event (or a constructed details
object), use eventDetails.reason to indicate the trigger, and call
eventDetails.cancel() or eventDetails.allowPropagation() as needed to control
state changes and bubbling; in practice remove usages of new
PointerEvent/KeyboardEvent and dispatchKeyboardEvent, and call the menu's
onOpenChange/open/close handlers directly from those functions
(highlightFirstItem, checkAndOpenSubMenu, checkAndCloseSubMenu,
blurStaleMenuItem), passing appropriate eventDetails and the native
KeyboardEvent when available.
In `@packages/raystack/components/menu/menu-trigger.tsx`:
- Around line 60-89: The MenuPrimitive.SubmenuTrigger's explicit render prop
(used to wrap the cell in AutocompletePrimitive.Item when parent?.autocomplete
is true) is being overridden by the {...props} spread because render isn't
destructured from props; change the component to pull render out of props (e.g.
const { render, ...rest } = props) and spread rest instead of props into
MenuPrimitive.SubmenuTrigger so user-supplied render cannot silently override
the autocomplete wrapper (or alternatively explicitly compose a user render with
the internal render when parent?.autocomplete is true); reference
MenuPrimitive.SubmenuTrigger, props, render, parent?.autocomplete, and
AutocompletePrimitive.Item when applying the change.
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
1 participant
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.
Description
Breaking Changes
DropdownMenucomponent renamed toMenuMenu.SubMenurenamed toMenu.SubmenuMenu.SubTriggerrenamed toMenu.SubmenuTriggerMenu.SubContentrenamed toMenu.SubmenuContentonSearchprop renamed toonInputValueChangesearchValueprop renamed toinputValuedefaultSearchValueprop renamed todefaultInputValueChanges
Menu.Submenu,Menu.SubmenuTrigger, andMenu.SubmenuContentMenu.Labelusage to always be insideMenu.Groupacross all examplesDropdownMenuItemtype toMenuItemandLinearDropdownDemotoLinearMenuDemoin examplesdropdown-menu-examples.tsxtomenu-examples.tsxandDropdownMenuExamplestoMenuExamplesSummary by CodeRabbit
New Features
Documentation
Refactor
Tests