Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a complete ContextMenu system: root, trigger, content, item, group/label/separator/empty-state, submenu, autocomplete support, tests, demos, props, MDX docs, and package exports across raystack components and the website. Changes
Sequence DiagramsequenceDiagram
actor User
participant Trigger
participant ContextMenuRoot
participant ContextMenuContent
participant AutocompleteInput
participant MenuItem
User->>Trigger: Right-click / open
Trigger->>ContextMenuRoot: dispatch open
ContextMenuRoot->>ContextMenuContent: render (open)
ContextMenuContent->>ContextMenuContent: mount portal + items
alt Autocomplete enabled
ContextMenuContent->>AutocompleteInput: render input
User->>AutocompleteInput: type text
AutocompleteInput->>ContextMenuContent: update inputValue
ContextMenuContent->>MenuItem: apply getMatch(inputValue)
ContextMenuContent->>MenuItem: highlight first match
else Normal menu
ContextMenuContent->>MenuItem: render all visible items
end
User->>MenuItem: click / activate
MenuItem->>ContextMenuRoot: request close (onOpenChange)
ContextMenuRoot->>ContextMenuContent: close
ContextMenuContent->>User: menu hidden
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
📝 Coding Plan
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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
packages/raystack/components/context-menu/context-menu-misc.tsx (1)
13-18: Keep the public DOM contract stable while filtering.When
shouldFilterturns on,Groupbecomes aFragmentandLabel/Separatorbecomenull, so forwarded refs plus caller-providedid,data-*, andaria-*props vanish only while the user is typing. That makes these public subcomponents hard to target predictably from tests and accessibility hooks. Prefer a stable wrapper/hidden state, or explicitly document that these props are ignored in filtered mode.Also applies to: 32-37, 52-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/context-menu/context-menu-misc.tsx` around lines 13 - 18, The Group/Label/Separator components currently return Fragment or null when shouldFilter is true which drops forwarded refs and any caller-provided id/data-*/aria-* attributes; change these components (e.g., Group, Label, Separator in context-menu-misc.tsx) to always render a stable DOM wrapper element that forwards ref and spreads ...props, and when shouldFilter is true hide its content via an accessibility-safe mechanism (e.g., aria-hidden="true" and hidden or style display:none) or visually hide children while keeping the wrapper in the DOM so refs and attributes remain stable during filtering.packages/raystack/index.tsx (1)
22-22: Re-export the public prop types with the component.The package root now exposes
ContextMenu, but not the prop types that the docs name (ContextMenuRootProps,ContextMenuTriggerProps,ContextMenuContentProps,ContextMenuItemProps, etc.). That forces TS consumers into deep imports from internal files when they build wrappers. Consider surfacing the public types from the root entrypoint too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/index.tsx` at line 22, The root export exposes ContextMenu but not its public prop types; update the package root to re-export the named types used in docs (e.g., ContextMenuRootProps, ContextMenuTriggerProps, ContextMenuContentProps, ContextMenuItemProps, etc.) alongside the component so consumers don’t need deep imports; locate the ContextMenu export and add corresponding type re-exports for the public prop interfaces from the module that declares them (the component or its types file) so the types are available from the root entrypoint.
🤖 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/context-menu/props.ts`:
- Around line 33-34: The props table is hand-maintained and out-of-date; replace
the manual declaration (e.g., the onOpenChange?: (open: boolean) => void entry)
by importing and sourcing the real exported prop types from the component
package (for example the ContextMenuRoot/ContextMenuTrigger exported types in
packages/raystack/components/context-menu) so the docs reflect the actual
signatures (onOpenChange receives (open, eventDetails) and TriggerProps include
style, etc.); update the props generation to reference those exported types for
the sections that currently span lines like 33–34, 47–53 and 162–163 so the docs
are derived directly from ContextMenuTriggerProps/ContextMenuRootProps (or the
package export names) instead of hand-copying.
In `@packages/raystack/components/context-menu/__tests__/context-menu.test.tsx`:
- Around line 104-113: Update the tests for BasicContextMenu to assert exact
interaction payloads: when calling renderAndOpenContextMenu with a mocked
onClick and onOpenChange, click the specific menu item text from
MENU_ITEMS[0].label and assert onClick was called with the expected id/value
from MENU_ITEMS[0] (not just called), assert that the disabled item's handler
(the onClick for MENU_ITEMS that has disabled: true) was not called after
clicking it, and assert onOpenChange was called with the correct open/closed
boolean state after the click; use the helpers renderAndOpenContextMenu,
MENU_ITEMS, and BasicContextMenu to locate the relevant items and mocks.
- Around line 6-10: The test file currently mutates
Element.prototype.scrollIntoView at module scope; move that mock into the test
lifecycle and restore the original to avoid leaking into other suites: in
context-menu.test.tsx, save the original (e.g., const _origScrollIntoView =
Element.prototype.scrollIntoView) in a beforeAll or beforeEach block then
replace it with vi.fn() inside that setup, and restore
Element.prototype.scrollIntoView = _origScrollIntoView in an afterAll or
afterEach block so the original implementation is returned after the suite.
In `@packages/raystack/components/context-menu/context-menu-content.tsx`:
- Around line 18-25: The ContextMenuContentProps interface incorrectly includes
ContextMenuPrimitive.Popup.Props while the component only spreads remaining
props (positionerProps) onto Positioner, causing Popup props to be misrouted and
onFocus to be lost; fix by either removing ContextMenuPrimitive.Popup.Props from
ContextMenuContentProps or by explicitly splitting/forwarding props: keep
ContextMenuContentProps limited to Positioner props plus the specific extra
props used (e.g., searchPlaceholder) and update the destructuring in the
ContextMenuContent component to gather popupProps separately (so Popup receives
its own props) or ensure positionerProps and popupProps are both derived and
spread to Positioner and Popup respectively, and make sure onFocus from props is
forwarded to the correct element rather than defaulting to undefined.
In `@packages/raystack/components/context-menu/context-menu-item.tsx`:
- Around line 45-53: The onFocus handler for ContextMenuPrimitive.Item currently
swallows any consumer onFocus from props; update the handler in
context-menu-item.tsx (the ContextMenuPrimitive.Item with ref and render={cell})
to invoke props.onFocus?.(e) before/after handling internal logic so the
caller's focus logic is preserved, then continue with e.stopPropagation(),
e.preventDefault(), and e.preventBaseUIHandler() as in the existing handler to
maintain internal behavior and API consistency with
menu-content.tsx/context-menu-content.tsx.
In `@packages/raystack/components/context-menu/context-menu-root.tsx`:
- Around line 59-73: The component currently resets autocomplete (setValue('')
and isInitialRender.current = true) only inside handleOpenChange, so when a
parent directly sets the controlled open prop to false the autocomplete state is
not cleared; add a useEffect that watches the effective open state used by the
component (the resolved/controlled `open` variable or `openProp`/`internalOpen`
combination) and when that value becomes false and `autocomplete` is true call
setValue('') and set isInitialRender.current = true to mirror the existing
behavior in handleOpenChange; ensure this effect does not duplicate behavior
when handleOpenChange already runs (i.e., it should run on changes to the
effective open state and depend on `open`/`internalOpen`, `autocomplete`, and
`setValue`).
---
Nitpick comments:
In `@packages/raystack/components/context-menu/context-menu-misc.tsx`:
- Around line 13-18: The Group/Label/Separator components currently return
Fragment or null when shouldFilter is true which drops forwarded refs and any
caller-provided id/data-*/aria-* attributes; change these components (e.g.,
Group, Label, Separator in context-menu-misc.tsx) to always render a stable DOM
wrapper element that forwards ref and spreads ...props, and when shouldFilter is
true hide its content via an accessibility-safe mechanism (e.g.,
aria-hidden="true" and hidden or style display:none) or visually hide children
while keeping the wrapper in the DOM so refs and attributes remain stable during
filtering.
In `@packages/raystack/index.tsx`:
- Line 22: The root export exposes ContextMenu but not its public prop types;
update the package root to re-export the named types used in docs (e.g.,
ContextMenuRootProps, ContextMenuTriggerProps, ContextMenuContentProps,
ContextMenuItemProps, etc.) alongside the component so consumers don’t need deep
imports; locate the ContextMenu export and add corresponding type re-exports for
the public prop interfaces from the module that declares them (the component or
its types file) so the types are available from the root entrypoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f80cb359-7fb8-4e68-8a55-f91cc9889ae2
📒 Files selected for processing (14)
apps/www/src/components/playground/context-menu-examples.tsxapps/www/src/components/playground/index.tsapps/www/src/content/docs/components/context-menu/demo.tsapps/www/src/content/docs/components/context-menu/index.mdxapps/www/src/content/docs/components/context-menu/props.tspackages/raystack/components/context-menu/__tests__/context-menu.test.tsxpackages/raystack/components/context-menu/context-menu-content.tsxpackages/raystack/components/context-menu/context-menu-item.tsxpackages/raystack/components/context-menu/context-menu-misc.tsxpackages/raystack/components/context-menu/context-menu-root.tsxpackages/raystack/components/context-menu/context-menu-trigger.tsxpackages/raystack/components/context-menu/context-menu.tsxpackages/raystack/components/context-menu/index.tspackages/raystack/index.tsx
| /** Callback fired when the menu is opened or closed */ | ||
| onOpenChange?: (open: boolean) => void; |
There was a problem hiding this comment.
Source the docs props from the real component types.
This hand-maintained copy is already drifting: onOpenChange here only documents open, but packages/raystack/components/context-menu/context-menu-root.tsx forwards (open, eventDetails) for both root and submenu, and apps/www/src/content/docs/components/context-menu/demo.ts uses style on ContextMenu.Trigger even though ContextMenuTriggerProps does not list it. Deriving these tables from the exported prop types will keep the docs accurate.
Also applies to: 47-53, 162-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/src/content/docs/components/context-menu/props.ts` around lines 33 -
34, The props table is hand-maintained and out-of-date; replace the manual
declaration (e.g., the onOpenChange?: (open: boolean) => void entry) by
importing and sourcing the real exported prop types from the component package
(for example the ContextMenuRoot/ContextMenuTrigger exported types in
packages/raystack/components/context-menu) so the docs reflect the actual
signatures (onOpenChange receives (open, eventDetails) and TriggerProps include
style, etc.); update the props generation to reference those exported types for
the sections that currently span lines like 33–34, 47–53 and 162–163 so the docs
are derived directly from ContextMenuTriggerProps/ContextMenuRootProps (or the
package export names) instead of hand-copying.
| // Mock scrollIntoView for test environment | ||
| Object.defineProperty(Element.prototype, 'scrollIntoView', { | ||
| value: vi.fn(), | ||
| writable: true | ||
| }); |
There was a problem hiding this comment.
Restore the prototype mock after this suite.
This mutates Element.prototype at module scope and never restores it. In a shared jsdom worker, later tests can inherit the stub and miss regressions that depend on the original implementation. Move the patch into setup/teardown and put the original back in afterAll.
Suggested cleanup pattern
-import { describe, expect, it, vi } from 'vitest';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
import { ContextMenu } from '../context-menu';
-// Mock scrollIntoView for test environment
-Object.defineProperty(Element.prototype, 'scrollIntoView', {
- value: vi.fn(),
- writable: true
-});
+const originalScrollIntoView = Element.prototype.scrollIntoView;
+
+beforeAll(() => {
+ Object.defineProperty(Element.prototype, 'scrollIntoView', {
+ value: vi.fn(),
+ writable: true
+ });
+});
+
+afterAll(() => {
+ Object.defineProperty(Element.prototype, 'scrollIntoView', {
+ value: originalScrollIntoView,
+ writable: true
+ });
+});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/context-menu/__tests__/context-menu.test.tsx`
around lines 6 - 10, The test file currently mutates
Element.prototype.scrollIntoView at module scope; move that mock into the test
lifecycle and restore the original to avoid leaking into other suites: in
context-menu.test.tsx, save the original (e.g., const _origScrollIntoView =
Element.prototype.scrollIntoView) in a beforeAll or beforeEach block then
replace it with vi.fn() inside that setup, and restore
Element.prototype.scrollIntoView = _origScrollIntoView in an afterAll or
afterEach block so the original implementation is returned after the suite.
| it('handles item clicks with onClick', async () => { | ||
| const onClick = vi.fn(); | ||
|
|
||
| await renderAndOpenContextMenu(<BasicContextMenu onClick={onClick} />); | ||
|
|
||
| const item = screen.getByText(MENU_ITEMS[0].label); | ||
| fireEvent.click(item); | ||
|
|
||
| expect(onClick).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Strengthen the interaction assertions.
These tests only prove that callbacks fired at least once. They still pass if the wrong item id is emitted, the disabled item remains clickable, or onOpenChange reports the wrong value. Assert the exact payloads and verify the disabled item's handler stays untouched after a click.
Tighter assertions
const item = screen.getByText(MENU_ITEMS[0].label);
fireEvent.click(item);
- expect(onClick).toHaveBeenCalled();
+ expect(onClick).toHaveBeenCalledWith(MENU_ITEMS[0].id);
@@
const disabledItem = screen.getByTestId('disabled-item');
expect(disabledItem).toHaveAttribute('aria-disabled', 'true');
+ fireEvent.click(disabledItem);
+ expect(onClick).not.toHaveBeenCalled();
@@
const trigger = screen.getByText(TRIGGER_TEXT);
fireEvent.contextMenu(trigger);
- expect(onOpenChange).toHaveBeenCalled();
+ expect(onOpenChange).toHaveBeenCalledWith(true);Also applies to: 115-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/context-menu/__tests__/context-menu.test.tsx`
around lines 104 - 113, Update the tests for BasicContextMenu to assert exact
interaction payloads: when calling renderAndOpenContextMenu with a mocked
onClick and onOpenChange, click the specific menu item text from
MENU_ITEMS[0].label and assert onClick was called with the expected id/value
from MENU_ITEMS[0] (not just called), assert that the disabled item's handler
(the onClick for MENU_ITEMS that has disabled: true) was not called after
clicking it, and assert onOpenChange was called with the correct open/closed
boolean state after the click; use the helpers renderAndOpenContextMenu,
MENU_ITEMS, and BasicContextMenu to locate the relevant items and mocks.
| export interface ContextMenuContentProps | ||
| extends Omit< | ||
| ContextMenuPrimitive.Positioner.Props, | ||
| 'render' | 'className' | 'style' | ||
| >, | ||
| ContextMenuPrimitive.Popup.Props { | ||
| searchPlaceholder?: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/raystack/components/context-menu/context-menu-content.tsx | head -150Repository: raystack/apsara
Length of output: 5646
🏁 Script executed:
cat -n packages/raystack/components/context-menu/context-menu-content.tsx | tail -100Repository: raystack/apsara
Length of output: 4144
🏁 Script executed:
# Let me check what props are in ContextMenuPrimitive.Popup.Props vs Positioner.Props
# First, search for any documentation or type definitions in the repo
find . -type f -name "*.d.ts" -o -name "*.ts" -o -name "*.tsx" | head -20Repository: raystack/apsara
Length of output: 911
🏁 Script executed:
# Search for other usages of ContextMenuPrimitive.Popup and Positioner to understand the props
rg "ContextMenuPrimitive\.(Popup|Positioner)" --type tsx --type ts -A 3 -B 1Repository: raystack/apsara
Length of output: 86
🏁 Script executed:
rg "ContextMenuPrimitive\.(Popup|Positioner)" -A 3 -B 1Repository: raystack/apsara
Length of output: 2306
🏁 Script executed:
# Check package.json to see what version of `@base-ui/react` is used
cat packages/raystack/package.json | grep -A 2 -B 2 "@base-ui/react"Repository: raystack/apsara
Length of output: 228
🏁 Script executed:
# Let me check if there are any node_modules or type definitions we can inspect
# First, look for base-ui type files or documentation
find . -type f -path "*/node_modules/@base-ui/react/*" -name "*.d.ts" 2>/dev/null | head -20Repository: raystack/apsara
Length of output: 41
🌐 Web query:
@base-ui/react ContextMenu Popup Props API documentation
💡 Result:
<ContextMenu.Popup> in @base-ui/react is documented on the Base UI Context Menu page under API reference → Popup. It renders a <div> and supports these props: finalFocus, children, className (string or (state) => string), style (object or (state) => CSSProperties), and render (ReactElement or (props, state) => ReactElement). It also sets state/position data-attributes like data-open, data-closed, data-align, data-side, data-instant, data-starting-style, data-ending-style. [1]
Type-wise, ContextMenuPopupProps is an alias of MenuPopupProps (and ContextMenuPopupState aliases MenuPopupState). [2]
Sources
[1] (base-ui.com)
[2] (app.unpkg.com)
Citations:
- 1: https://base-ui.com/react/components/context-menu?utm_source=openai
- 2: https://app.unpkg.com/%40base-ui/react%401.2.0/files/context-menu/index.d.ts
🌐 Web query:
@base-ui/react ContextMenu Positioner Props API documentation
💡 Result:
<ContextMenu.Positioner /> (from @base-ui/react/context-menu) uses the same prop surface as the Menu Positioner—ContextMenuPositionerProps is re-exported from MenuPositionerProps. [1]
Props (API reference)
disableAnchorTracking?: boolean(defaultfalse) — disables tracking layout shifts of the anchor. [2]align?: 'start' | 'center' | 'end'(default'center') [2]alignOffset?: number | ((data) => number)(default0) [2]side?: 'top' | 'bottom' | 'left' | 'right' | 'inline-start' | 'inline-end'(default'bottom') [3]sideOffset?: number | ((data) => number)(default0) [3]arrowPadding?: number(default5) [3]anchor?: Element | VirtualElement | React.RefObject<Element | null> | (() => Element | VirtualElement | null) | null— overrides the default anchor (the trigger). [3]collisionAvoidance?: { side?: 'flip' | 'shift' | 'none'; align?: 'flip' | 'shift' | 'none'; fallbackAxisSide?: 'start' | 'end' | 'none' }[3]collisionBoundary?: 'clipping-ancestors' | Element | Element[] | Rect(default'clipping-ancestors') [4]collisionPadding?: number | { top?: number; right?: number; bottom?: number; left?: number }(default5) [4]sticky?: boolean(defaultfalse) [4]positionMethod?: 'absolute' | 'fixed'(default'absolute') [4]className?: string | ((state) => string | undefined)[4]style?: React.CSSProperties | ((state) => React.CSSProperties | undefined)[4]render?: ReactElement | ((props, state) => ReactElement)[4]
Positioner data attributes + CSS variables
- Attributes:
data-open,data-closed,data-anchor-hidden,data-align,data-side. [5] - CSS vars:
--anchor-height,--anchor-width,--available-height,--available-width,--transform-origin. [5]
Sources:
[1] UNPKG type re-exports (ContextMenuPositionerProps ↔ MenuPositionerProps) (app.unpkg.com)
[2] Context Menu docs — Positioner: disableAnchorTracking, align, alignOffset (base-ui.com)
[3] Context Menu docs — Positioner: side, sideOffset, arrowPadding, anchor, collisionAvoidance (base-ui.com)
[4] Context Menu docs — Positioner: collision/viewport + styling/render props (base-ui.com)
[5] Context Menu docs — Positioner attributes + CSS variables (base-ui.com)
Citations:
- 1: https://app.unpkg.com/%40base-ui/react%401.2.0/files/context-menu/index.d.ts
- 2: https://base-ui.com/react/components/context-menu
- 3: https://base-ui.com/react/components/context-menu
- 4: https://base-ui.com/react/components/context-menu
- 5: https://base-ui.com/react/components/context-menu
Narrow ContextMenuContentProps or split prop forwarding between Positioner and Popup.
The interface extends ContextMenuPrimitive.Popup.Props (line 23), but the destructuring at lines 31–43 catches remaining props into positionerProps, which spreads only onto Positioner (line 116). Any Popup prop not explicitly extracted will silently pass to Positioner instead, creating an inconsistent API contract. Additionally, onFocus handlers passed in non-autocomplete mode are discarded (lines 129–138, where it defaults to undefined).
Either explicitly forward all Popup props to the Popup component, or remove Popup.Props from the interface and document which props are actually supported.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/context-menu/context-menu-content.tsx` around
lines 18 - 25, The ContextMenuContentProps interface incorrectly includes
ContextMenuPrimitive.Popup.Props while the component only spreads remaining
props (positionerProps) onto Positioner, causing Popup props to be misrouted and
onFocus to be lost; fix by either removing ContextMenuPrimitive.Popup.Props from
ContextMenuContentProps or by explicitly splitting/forwarding props: keep
ContextMenuContentProps limited to Positioner props plus the specific extra
props used (e.g., searchPlaceholder) and update the destructuring in the
ContextMenuContent component to gather popupProps separately (so Popup receives
its own props) or ensure positionerProps and popupProps are both derived and
spread to Positioner and Popup respectively, and make sure onFocus from props is
forwarded to the correct element rather than defaulting to undefined.
| <ContextMenuPrimitive.Item | ||
| ref={ref} | ||
| render={cell} | ||
| {...props} | ||
| onFocus={e => { | ||
| e.stopPropagation(); | ||
| e.preventDefault(); | ||
| e.preventBaseUIHandler(); | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/raystack/components/context-menu/context-menu-item.tsxRepository: raystack/apsara
Length of output: 2060
🏁 Script executed:
fd -e ts -e tsx -e d.ts | xargs rg -l "ContextMenuPrimitive" | head -5Repository: raystack/apsara
Length of output: 384
🏁 Script executed:
rg -l "ContextMenuItem" --type tsx --type ts | xargs grep -l "onFocus" | head -5Repository: raystack/apsara
Length of output: 86
🏁 Script executed:
rg "ContextMenuItem" | grep -i "onfocus" -C 3Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
find . -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" | xargs grep -l "ContextMenuItem" 2>/dev/null | head -3Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
rg "ContextMenuPrimitive\.Item\.Props" -A 10Repository: raystack/apsara
Length of output: 1178
🏁 Script executed:
find . -name "package.json" -path "*/node_modules" -prune -o -type f -name "package.json" -print | head -1 | xargs cat | grep -A 5 "@base-ui"Repository: raystack/apsara
Length of output: 41
🏁 Script executed:
cat packages/raystack/package.json | grep -A 5 -B 5 "@base-ui"Repository: raystack/apsara
Length of output: 404
🏁 Script executed:
rg "AutocompletePrimitive\.Item" -A 5 -B 2Repository: raystack/apsara
Length of output: 3973
🏁 Script executed:
rg "onFocus" packages/raystack/components/ --type-listRepository: raystack/apsara
Length of output: 448
🏁 Script executed:
rg "onFocus" packages/raystack/components/Repository: raystack/apsara
Length of output: 2004
🏁 Script executed:
cat packages/raystack/components/menu/menu-item.tsx | head -60Repository: raystack/apsara
Length of output: 1539
🏁 Script executed:
cat packages/raystack/components/menu/menu-content.tsx | grep -A 10 "onFocus="Repository: raystack/apsara
Length of output: 381
🏁 Script executed:
cat packages/raystack/components/context-menu/context-menu-content.tsx | grep -A 10 "onFocus="Repository: raystack/apsara
Length of output: 381
Compose the caller's onFocus handler.
The regular rendering branch (line 49) overrides any onFocus passed through props, causing consumer focus logic to work in autocomplete mode but be silently dropped here. Call props.onFocus?.(e) inside this handler to maintain API consistency, matching the pattern used in menu-content.tsx and context-menu-content.tsx.
💡 Suggested change
<ContextMenuPrimitive.Item
ref={ref}
render={cell}
{...props}
onFocus={e => {
e.stopPropagation();
e.preventDefault();
e.preventBaseUIHandler();
+ props.onFocus?.(e);
}}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ContextMenuPrimitive.Item | |
| ref={ref} | |
| render={cell} | |
| {...props} | |
| onFocus={e => { | |
| e.stopPropagation(); | |
| e.preventDefault(); | |
| e.preventBaseUIHandler(); | |
| }} | |
| <ContextMenuPrimitive.Item | |
| ref={ref} | |
| render={cell} | |
| {...props} | |
| onFocus={e => { | |
| e.stopPropagation(); | |
| e.preventDefault(); | |
| e.preventBaseUIHandler(); | |
| props.onFocus?.(e); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/context-menu/context-menu-item.tsx` around lines
45 - 53, The onFocus handler for ContextMenuPrimitive.Item currently swallows
any consumer onFocus from props; update the handler in context-menu-item.tsx
(the ContextMenuPrimitive.Item with ref and render={cell}) to invoke
props.onFocus?.(e) before/after handling internal logic so the caller's focus
logic is preserved, then continue with e.stopPropagation(), e.preventDefault(),
and e.preventBaseUIHandler() as in the existing handler to maintain internal
behavior and API consistency with menu-content.tsx/context-menu-content.tsx.
| const handleOpenChange: ContextMenuPrimitive.Root.Props['onOpenChange'] = | ||
| useCallback( | ||
| ( | ||
| value: boolean, | ||
| eventDetails: ContextMenuPrimitive.Root.ChangeEventDetails | ||
| ) => { | ||
| if (!value && autocomplete) { | ||
| setValue(''); | ||
| isInitialRender.current = true; | ||
| } | ||
| setInternalOpen(value); | ||
| onOpenChange?.(value, eventDetails); | ||
| }, | ||
| [onOpenChange, setValue, autocomplete] | ||
| ); |
There was a problem hiding this comment.
Reset autocomplete state on externally controlled closes.
Lines 59-73 and 150-164 clear inputValue only from handleOpenChange. If a parent drives open to false directly, the old query and isInitialRender flag stay behind, so reopening an autocomplete menu/submenu reuses the stale filter. Mirror that reset in an effect keyed on the effective open state.
Also applies to: 150-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/context-menu/context-menu-root.tsx` around lines
59 - 73, The component currently resets autocomplete (setValue('') and
isInitialRender.current = true) only inside handleOpenChange, so when a parent
directly sets the controlled open prop to false the autocomplete state is not
cleared; add a useEffect that watches the effective open state used by the
component (the resolved/controlled `open` variable or `openProp`/`internalOpen`
combination) and when that value becomes false and `autocomplete` is true call
setValue('') and set isInitialRender.current = true to mirror the existing
behavior in handleOpenChange; ensure this effect does not duplicate behavior
when handleOpenChange already runs (i.e., it should run on changes to the
effective open state and depend on `open`/`internalOpen`, `autocomplete`, and
`setValue`).
1 participant
Description
Adds a new
ContextMenucomponent built on the Base UI ContextMenu primitive. Opens on right-click or long press instead of a button click.Summary by CodeRabbit
New Features
Documentation
Tests