Skip to content

feat: add closeOnBackPress prop to Dialog #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fxamauri wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add closeOnBackPress prop to Dialog #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 22 additions & 21 deletions apps/docs/src/content/docs/dialog.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ A modal dialog that interrupts the user with important content and expects a res
<Tabs>
<TabItem label="NPM">

Install the component via your command line.
Install the component via your command line.

```bash
npx expo install @rn-primitives/dialog
Expand Down Expand Up @@ -86,21 +86,21 @@ A modal dialog that interrupts the user with important content and expects a res

<Aside type="caution">

Requires a `<PortalHost />` as the last child of your `<Root/>` (`app/_layout.tsx`) component
Requires a `<PortalHost />` as the last child of your `<Root/>` (`app/_layout.tsx`) component

```tsx
import { PortalHost } from '@rn-primitives/portal';
```tsx
import { PortalHost } from '@rn-primitives/portal';

function Root() {
return (
<>
<Stack />
{/* Default Portal Host (one per app) */}
<PortalHost />
</>
);
}
```
function Root() {
return (
<>
<Stack />
{/* Default Portal Host (one per app) */}
<PortalHost />
</>
);
}
```

</Aside>

Expand All @@ -118,7 +118,7 @@ function Example() {
<DialogPrimitive.Portal>
<DialogPrimitive.Overlay>
<DialogPrimitive.Content>
<DialogPrimitive.Title>Dialog Title</DialogPrimitive.Title>
<DialogPrimitive.Title>Dialog Title</DialogPrimitive.Title>
<DialogPrimitive.Description>
Dialog description.
</DialogPrimitive.Description>
Expand All @@ -137,12 +137,13 @@ function Example() {

Extends [`View`](https://reactnative.dev/docs/view#props) props

| Prop | Type | Note |
| :----------: | :----------------------: | :----------: |
| asChild | boolean | _(optional)_ |
| open | boolean | _(optional)_ |
| onOpenChange | (value: boolean) => void | _(optional)_ |
| defaultOpen | boolean | _(optional)_ |
| Prop | Type | Note |
| :--------------: | :----------------------: | :----------------------: |
| asChild | boolean | _(optional)_ |
| open | boolean | _(optional)_ |
| onOpenChange | (value: boolean) => void | _(optional)_ |
| defaultOpen | boolean | _(optional)_ |
| closeOnBackPress | boolean | Native Only _(optional)_ |

### Trigger

Expand Down
21 changes: 17 additions & 4 deletions packages/dialog/src/dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,17 @@ import type {
const DialogContext = React.createContext<(RootContext & { nativeID: string }) | null>(null);

const Root = React.forwardRef<RootRef, RootProps>(
({ asChild, open: openProp, defaultOpen, onOpenChange: onOpenChangeProp, ...viewProps }, ref) => {
(
{
asChild,
open: openProp,
defaultOpen,
onOpenChange: onOpenChangeProp,
closeOnBackPress = true,
...viewProps
},
ref
) => {
const nativeID = React.useId();
const [open = false, onOpenChange] = useControllableState({
prop: openProp,
Expand All @@ -40,6 +50,7 @@ const Root = React.forwardRef<RootRef, RootProps>(
open,
onOpenChange,
nativeID,
closeOnBackPress,
}}
>
<Component ref={ref} {...viewProps} />
Expand Down Expand Up @@ -130,18 +141,20 @@ Overlay.displayName = 'OverlayNativeDialog';

const Content = React.forwardRef<ContentRef, ContentProps>(
({ asChild, forceMount, ...props }, ref) => {
const { open, nativeID, onOpenChange } = useRootContext();
const { open, nativeID, onOpenChange, closeOnBackPress } = useRootContext();

React.useEffect(() => {
const backHandler = BackHandler.addEventListener('hardwareBackPress', () => {
onOpenChange(false);
if (closeOnBackPress) {
onOpenChange(false);
}
return true;
});

return () => {
backHandler.remove();
};
}, []);
}, [closeOnBackPress]);
Comment on lines 146 to +157
Copy link

@coderabbitai coderabbitai bot Oct 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add onOpenChange to the effect dependency array.

The useEffect hook calls onOpenChange inside the back handler but doesn't include it in the dependency array. This can lead to stale closure bugs where an outdated version of onOpenChange is invoked.

Apply this diff:

     React.useEffect(() => {
       const backHandler = BackHandler.addEventListener('hardwareBackPress', () => {
         if (closeOnBackPress) {
           onOpenChange(false);
         }
         return true;
       });
 
       return () => {
         backHandler.remove();
       };
-    }, [closeOnBackPress]);
+    }, [closeOnBackPress, onOpenChange]);
📝 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
React.useEffect(() => {
const backHandler = BackHandler.addEventListener('hardwareBackPress', () => {
onOpenChange(false);
if (closeOnBackPress) {
onOpenChange(false);
}
return true;
});
return () => {
backHandler.remove();
};
}, []);
}, [closeOnBackPress]);
React.useEffect(() => {
const backHandler = BackHandler.addEventListener('hardwareBackPress', () => {
if (closeOnBackPress) {
onOpenChange(false);
}
return true;
});
return () => {
backHandler.remove();
};
}, [closeOnBackPress, onOpenChange]);
🤖 Prompt for AI Agents 
In packages/dialog/src/dialog.tsx around lines 146 to 157, the useEffect
registers a hardwareBackPress handler that calls onOpenChange but onOpenChange
is missing from the dependency array; update the effect dependencies to include
onOpenChange (i.e., change the dependency array from [closeOnBackPress] to
[closeOnBackPress, onOpenChange]) so the latest callback is used and
stale-closure bugs are avoided.


if (!forceMount) {
if (!open) {
Expand Down
10 changes: 10 additions & 0 deletions packages/dialog/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,22 @@ import type {
type RootContext = {
open: boolean;
onOpenChange: (value: boolean) => void;

/**
* Platform: NATIVE ONLY
*/
closeOnBackPress?: boolean;
};

type RootProps = SlottableViewProps & {
open?: boolean;
defaultOpen?: boolean;
onOpenChange?: (value: boolean) => void;

/**
* Platform: NATIVE ONLY
*/
closeOnBackPress?: boolean;
};

interface PortalProps extends ForceMountable {
Expand Down