nandorojo / zeego

Menus for React (Native) done right.
https://zeego.dev
MIT License
1.45k stars 42 forks source link

Expose onOpenChange #14

Closed robertherber closed 1 year ago

robertherber commented 1 year ago

I guess this might be hard to support cross-platform, but it would be great if it's possible. Radix has a onOpenChange callback that makes it possible to know whether a DropdownMenu is open or not. I'm thinking it might be worth to add it even if it's not possible cross-platform (should of course be made clear through JSDoc etc in that case)?

My specific use-case: I needed this to be able to disable key listeners on web, so escape can work as expected

diff --git a/node_modules/zeego/lib/module/dropdown-menu/dropdown-menu.web.js b/node_modules/zeego/lib/module/dropdown-menu/dropdown-menu.web.js
index 9745390..b0a57e7 100644
--- a/node_modules/zeego/lib/module/dropdown-menu/dropdown-menu.web.js
+++ b/node_modules/zeego/lib/module/dropdown-menu/dropdown-menu.web.js
@@ -7,9 +7,12 @@ import * as DropdownMenu from '@radix-ui/react-dropdown-menu';

 const Root = _ref => {
   let {
-    children
+    children,
+    onOpenChange
   } = _ref;
-  return /*#__PURE__*/React.createElement(DropdownMenu.Root, null, children);
+  return /*#__PURE__*/React.createElement(DropdownMenu.Root, {
+    onOpenChange: (isOpen) => { onOpenChange && onOpenChange(isOpen) },
+  }, children);
 };

 Root.displayName = MenuDisplayName.Root;
diff --git a/node_modules/zeego/lib/typescript/menu/types.d.ts b/node_modules/zeego/lib/typescript/menu/types.d.ts
index 55b973b..c7f60d7 100644
--- a/node_modules/zeego/lib/typescript/menu/types.d.ts
+++ b/node_modules/zeego/lib/typescript/menu/types.d.ts
@@ -6,6 +6,7 @@ declare type TextStyle = React.ComponentProps<typeof Text>['style'];
 export declare type MenuRootProps = {
     children: React.ReactNode;
     style?: ViewStyle;
+    onOpenChange?: (open: boolean) => void;
 };
 export declare type MenuTriggerProps = {
     children: React.ReactElement;

This issue body was partially generated by patch-package.

nandorojo commented 1 year ago

Interesting! I'll look into this.

nandorojo commented 1 year ago

Looks like this would be possible for iOS too. The only place it wouldn't work is Android, due to https://github.com/react-native-menu/menu/issues/305.

I'm probably fine with adding the prop if 2/3 platforms support it. And we can mention it in the docs.

robertherber commented 1 year ago

Interesting! I'll look into this.

Awesome! Thanks for this great library that just keeps getting better 👍

nandorojo commented 1 year ago

Published in 0.5.0 #10