Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions packages/context-menu/src/context-menu.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { augmentRef } from '@rn-primitives/utils';
import {
useAugmentedRef,
useControllableState,
useRelativePosition,
type LayoutPosition,
Expand Down Expand Up @@ -121,21 +121,6 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
ref
) => {
const { open, onOpenChange, relativeTo, setPressPosition } = useRootContext();
const augmentedRef = useAugmentedRef({
ref,
methods: {
open: () => {
onOpenChange(true);
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setPressPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setPressPosition(null);
onOpenChange(false);
},
},
});

function onLongPress(ev: GestureResponderEvent) {
if (disabled) return;
Expand All @@ -148,7 +133,8 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
});
}
if (relativeTo === 'trigger') {
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setPressPosition({ width, pageX, pageY: pageY, height });
});
}
Expand All @@ -171,10 +157,25 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
onAccessibilityActionProp?.(event);
}

const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setPressPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setPressPosition(null);
onOpenChange(false);
},
};

const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
role='button'
onLongPress={onLongPress}
Expand Down
40 changes: 20 additions & 20 deletions packages/dropdown-menu/src/dropdown-menu.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { augmentRef } from '@rn-primitives/utils';
import {
useAugmentedRef,
useControllableState,
useRelativePosition,
type LayoutPosition,
Expand Down Expand Up @@ -109,36 +109,36 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
({ asChild, onPress: onPressProp, disabled = false, ...props }, ref) => {
const { open, onOpenChange, setTriggerPosition } = useRootContext();

const augmentedRef = useAugmentedRef({
ref,
methods: {
open: () => {
onOpenChange(true);
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
},
});

function onPress(ev: GestureResponderEvent) {
if (disabled) return;
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
const newValue = !open;
onOpenChange(newValue);
onPressProp?.(ev);
}


const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};
Comment on lines +123 to +136
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistent function ref handling in methods.open.

The same issue exists here as in tooltip.tsx: methods.open (line 126) calls measure without checking if ref is a function, while onPress (line 114) includes this guard.

Although augmentRef won't attach methods to function refs, the inconsistency makes the code harder to maintain.

     const methods = {
       open: () => {
+        if (typeof ref === 'function') return;
         onOpenChange(true);
         (ref as React.RefObject<TriggerRef>)?.current?.measure(
           (_x, _y, width, height, pageX, pageY) => {
             setTriggerPosition({ width, pageX, pageY: pageY, height });
           }
         );
       },
       close: () => {
         setTriggerPosition(null);
         onOpenChange(false);
       },
     };
📝 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.

Suggested change
const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};
const methods = {
open: () => {
if (typeof ref === 'function') return;
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};
🤖 Prompt for AI Agents
In packages/dropdown-menu/src/dropdown-menu.tsx around lines 123 to 136,
methods.open calls measure on ref without guarding against function refs
(inconsistent with onPress at ~line 114); update methods.open to check if ref is
a function before attempting to access ref.current (e.g. if (typeof ref ===
'function') return; or guard and only call (ref as
React.RefObject<TriggerRef>)?.current?.measure(...) when ref is not a function)
so function refs are skipped and behavior matches onPress.


const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
role='button'
onPress={onPress}
Expand Down
1 change: 1 addition & 0 deletions packages/hover-card/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"@rn-primitives/popover": "workspace:*",
"@rn-primitives/slot": "workspace:*",
"@rn-primitives/hooks": "workspace:*",
"@rn-primitives/utils": "workspace:*",
"@rn-primitives/types": "workspace:*"
},
"devDependencies": {
Expand Down
39 changes: 20 additions & 19 deletions packages/hover-card/src/hover-card.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useAugmentedRef, useRelativePosition, type LayoutPosition } from '@rn-primitives/hooks';
import { augmentRef } from '@rn-primitives/utils';
import { useRelativePosition, type LayoutPosition } from '@rn-primitives/hooks';
import { Portal as RNPPortal } from '@rn-primitives/portal';
import * as Slot from '@rn-primitives/slot';
import * as React from 'react';
Expand Down Expand Up @@ -91,36 +92,36 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
({ asChild, onPress: onPressProp, disabled = false, ...props }, ref) => {
const { open, onOpenChange, setTriggerPosition } = useRootContext();

const augmentedRef = useAugmentedRef({
ref,
methods: {
open: () => {
onOpenChange(true);
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
},
});

function onPress(ev: GestureResponderEvent) {
if (disabled) return;
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});

onOpenChange(!open);
onPressProp?.(ev);
}

const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};

const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
role='button'
onPress={onPress}
Expand Down
8 changes: 4 additions & 4 deletions packages/menubar/src/menubar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { augmentRef } from '@rn-primitives/utils';
import {
useAugmentedRef,
useControllableState,
useRelativePosition,
type LayoutPosition,
Expand Down Expand Up @@ -124,13 +124,13 @@ function useMenuContext() {

const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
({ asChild, onPress: onPressProp, disabled = false, ...props }, ref) => {
const triggerRef = useAugmentedRef({ ref });
const { value, onValueChange, setTriggerPosition } = useRootContext();
const { value: menuValue } = useMenuContext();

function onPress(ev: GestureResponderEvent) {
if (disabled) return;
triggerRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY, height });
});

Expand All @@ -141,7 +141,7 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={triggerRef}
ref={(self) => augmentRef(ref, self)} // copy hook behaviour if not needed then pass 'ref' instead
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing methods parameter for ref augmentation.

Unlike the other components in this PR (tooltip, dropdown-menu), this component calls augmentRef(ref, self) without providing the methods parameter containing open/close functions. This means:

  1. The augmented ref won't expose imperative open/close methods to consumers
  2. The behavior is inconsistent with the pattern established in tooltip.tsx (lines 104-115) and dropdown-menu.tsx (lines 123-136)

If imperative control via ref is not needed for Menubar, the comment should be more explicit, or you should simply pass ref directly. Otherwise, add the methods object:

+    const methods = {
+      open: () => {
+        onValueChange(menuValue);
+        if (typeof ref === 'function') return;
+        ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
+          setTriggerPosition({ width, pageX, pageY, height });
+        });
+      },
+      close: () => {
+        setTriggerPosition(null);
+        onValueChange(undefined);
+      },
+    };
+
     const Component = asChild ? Slot.Pressable : Pressable;
     return (
       <Component
-        ref={(self) => augmentRef(ref, self)} // copy hook behaviour if not needed then pass 'ref' instead
+        ref={(self) => augmentRef(ref, self, methods)}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/menubar/src/menubar.tsx around line 144, the call ref={(self) =>
augmentRef(ref, self)} omits the methods parameter (open/close) so the augmented
ref does not expose imperative control and is inconsistent with tooltip.tsx and
dropdown-menu.tsx; either pass the methods object (e.g., augmentRef(ref, self, {
open: () => setOpen(true), close: () => setOpen(false) }) matching the
component's open/close functions) so consumers get imperative open/close, or if
imperative control is intentionally not supported, replace the augmentRef call
with ref directly and update the inline comment to explicitly state that no
imperative methods are exposed.

aria-disabled={disabled ?? undefined}
role='button'
onPress={onPress}
Expand Down
1 change: 1 addition & 0 deletions packages/popover/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"dependencies": {
"@radix-ui/react-popover": "^1.1.14",
"@rn-primitives/hooks": "workspace:*",
"@rn-primitives/utils": "workspace:*",
"@rn-primitives/slot": "workspace:*",
"@rn-primitives/types": "workspace:*"
},
Expand Down
39 changes: 20 additions & 19 deletions packages/popover/src/popover.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useAugmentedRef, useRelativePosition, type LayoutPosition } from '@rn-primitives/hooks';
import { augmentRef } from '@rn-primitives/utils';
import { useRelativePosition, type LayoutPosition } from '@rn-primitives/hooks';
import { Portal as RNPPortal } from '@rn-primitives/portal';
import * as Slot from '@rn-primitives/slot';
import * as React from 'react';
Expand Down Expand Up @@ -81,35 +82,35 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
({ asChild, onPress: onPressProp, disabled = false, ...props }, ref) => {
const { onOpenChange, open, setTriggerPosition } = useRootContext();

const augmentedRef = useAugmentedRef({
ref,
methods: {
open: () => {
onOpenChange(true);
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
},
});

function onPress(ev: GestureResponderEvent) {
if (disabled) return;
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
onOpenChange(!open);
onPressProp?.(ev);
}

const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};

const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
Comment on lines 85 to 114
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Restore trigger measurement regardless of forwarded ref shape

Early-returning when ref is a function (or absent) skips both measurement and onOpenChange, so any consumer using a callback ref—or no ref at all—can no longer open the popover. Because we no longer keep an internal ref (the old useAugmentedRef handled this), ref?.current is undefined in those cases, so the trigger position never updates and the portal content stays null. This is a functional regression that completely breaks the component outside of the narrow “object ref” scenario. Please reinstate a local ref that is always wired to the host node and drive both measurement and the exposed methods through it instead of bailing on callback refs.

@@
-    function onPress(ev: GestureResponderEvent) {
-      if (disabled) return;
-      if (typeof ref === 'function') return;
-      ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
-        setTriggerPosition({ width, pageX, pageY: pageY, height });
-      });
-      onOpenChange(!open);
-      onPressProp?.(ev);
-    }
-
-    const methods = {
-      open: () => {
-        onOpenChange(true);
-        (ref as React.RefObject<TriggerRef>)?.current?.measure(
-          (_x, _y, width, height, pageX, pageY) => {
-            setTriggerPosition({ width, pageX, pageY: pageY, height });
-          }
-        );
-      },
-      close: () => {
-        setTriggerPosition(null);
-        onOpenChange(false);
-      },
-    };
+    const triggerRef = React.useRef<TriggerRef>(null);
+
+    const measureTrigger = React.useCallback(() => {
+      triggerRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
+        setTriggerPosition({ width, pageX, pageY, height });
+      });
+    }, [setTriggerPosition]);
+
+    function onPress(ev: GestureResponderEvent) {
+      if (disabled) return;
+      measureTrigger();
+      onOpenChange(!open);
+      onPressProp?.(ev);
+    }
+
+    const methods = React.useMemo(
+      () => ({
+        open: () => {
+          onOpenChange(true);
+          measureTrigger();
+        },
+        close: () => {
+          setTriggerPosition(null);
+          onOpenChange(false);
+        },
+      }),
+      [measureTrigger, onOpenChange, setTriggerPosition]
+    );
@@
-      <Component
-        ref={(self) => augmentRef(ref, self, methods)}
+      <Component
+        ref={(self) => {
+          triggerRef.current = self;
+          augmentRef(ref, self, methods);
+        }}
📝 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.

Suggested change
function onPress(ev: GestureResponderEvent) {
if (disabled) return;
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
onOpenChange(!open);
onPressProp?.(ev);
}
const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure(
(_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
}
);
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};
const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
const triggerRef = React.useRef<TriggerRef>(null);
const measureTrigger = React.useCallback(() => {
triggerRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY, height });
});
}, [setTriggerPosition]);
function onPress(ev: GestureResponderEvent) {
if (disabled) return;
measureTrigger();
onOpenChange(!open);
onPressProp?.(ev);
}
const methods = React.useMemo(
() => ({
open: () => {
onOpenChange(true);
measureTrigger();
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
}),
[measureTrigger, onOpenChange, setTriggerPosition]
);
const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={(self) => {
triggerRef.current = self;
augmentRef(ref, self, methods);
}}
aria-disabled={disabled ?? undefined}

role='button'
onPress={onPress}
Expand Down
1 change: 1 addition & 0 deletions packages/select/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"dependencies": {
"@radix-ui/react-select": "^2.2.5",
"@rn-primitives/hooks": "workspace:*",
"@rn-primitives/utils": "workspace:*",
"@rn-primitives/slot": "workspace:*",
"@rn-primitives/types": "workspace:*"
},
Expand Down
36 changes: 17 additions & 19 deletions packages/select/src/select.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { augmentRef } from '@rn-primitives/utils';
import {
useAugmentedRef,
useControllableState,
useRelativePosition,
type LayoutPosition,
Expand Down Expand Up @@ -122,35 +122,33 @@ const Trigger = React.forwardRef<TriggerRef, TriggerProps>(
({ asChild, onPress: onPressProp, disabled = false, ...props }, ref) => {
const { open, onOpenChange, disabled: disabledRoot, setTriggerPosition } = useRootContext();

const augmentedRef = useAugmentedRef({
ref,
methods: {
open: () => {
onOpenChange(true);
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
},
});

function onPress(ev: GestureResponderEvent) {
if (disabled) return;
augmentedRef.current?.measure((_x, _y, width, height, pageX, pageY) => {
if (typeof ref === 'function') return;
ref?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
onOpenChange(!open);
onPressProp?.(ev);
}

const methods = {
open: () => {
onOpenChange(true);
(ref as React.RefObject<TriggerRef>)?.current?.measure((_x, _y, width, height, pageX, pageY) => {
setTriggerPosition({ width, pageX, pageY: pageY, height });
});
},
close: () => {
setTriggerPosition(null);
onOpenChange(false);
},
};

const Component = asChild ? Slot.Pressable : Pressable;
return (
<Component
ref={augmentedRef}
ref={(self) => augmentRef(ref, self, methods)}
aria-disabled={disabled ?? undefined}
role='combobox'
onPress={onPress}
Expand Down
1 change: 1 addition & 0 deletions packages/tooltip/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"dependencies": {
"@radix-ui/react-tooltip": "^1.2.7",
"@rn-primitives/hooks": "workspace:*",
"@rn-primitives/utils": "workspace:*",
"@rn-primitives/slot": "workspace:*",
"@rn-primitives/types": "workspace:*"
},
Expand Down
Loading