feat: Handle onPress in iOS Menu#4148
Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for per-menu-item onPress callbacks in the experimental iOS Stack v5 header menu system by introducing stable menuElementId routing from native (UIAction) back to JS.
Changes:
- Introduces
menuElementId(required) on all iOS header menu elements and adds optional JS-sideonPresson menu items. - Wires native menu selection to Fabric event emission (
onPressMenuItem) and dispatches to the correct JS menu item handler via ID lookup. - Updates the single-feature test app + scenario instructions to exercise menu presses and show Toast feedback.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/fabric/gamma/stack/StackHeaderConfigIOSNativeComponent.ts | Adds onPressMenuItem direct event + payload type for Fabric codegen. |
| src/components/gamma/stack/header/utils.ts | Adds recursive lookup helpers to resolve a menu element by menuElementId. |
| src/components/gamma/stack/header/StackHeaderConfig.ios.types.ts | Introduces SupportsMenuIOS and reuses it across header item types. |
| src/components/gamma/stack/header/StackHeaderConfig.ios.tsx | Subscribes to native onPressMenuItem and invokes the matched JS menu item onPress. |
| src/components/gamma/stack/header/ios/StackHeaderMenu.ios.types.ts | Makes menuElementId required and adds optional onPress on menu items. |
| ios/gamma/stack/header/RNSStackHeaderMenuMapper.mm | Parses/validates menuElementId and constructs menu/menuItem data with IDs. |
| ios/gamma/stack/header/RNSStackHeaderMenuEventsDelegate.h | Defines delegate protocol for native menu element press events. |
| ios/gamma/stack/header/RNSStackHeaderMenuData.h | Adds menuElementId requirement to the menu element protocol and updates initializers. |
| ios/gamma/stack/header/RNSStackHeaderMenuData.mm | Stores/synthesizes menuElementId on menu/menuItem data objects. |
| ios/gamma/stack/header/RNSStackHeaderMenuCoordinator.h | Extends coordinator API to accept a menu events delegate. |
| ios/gamma/stack/header/RNSStackHeaderMenuCoordinator.mm | Builds UIAction handlers that notify delegate with pressed element ID. |
| ios/gamma/stack/header/RNSStackHeaderContentFactory.h | Threads menu events delegate into bar button item creation. |
| ios/gamma/stack/header/RNSStackHeaderContentFactory.mm | Applies menus using the coordinator + delegate for event forwarding. |
| ios/gamma/stack/header/RNSStackHeaderConfigEventEmitter.h | Adds an ObjC wrapper to emit Fabric onPressMenuItem events. |
| ios/gamma/stack/header/RNSStackHeaderConfigEventEmitter.mm | Implements onPressMenuItem emission to the Fabric event emitter. |
| ios/gamma/stack/header/RNSStackHeaderConfigComponentView.mm | Implements the menu events delegate and forwards presses to JS via the event emitter. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-header-menu-ios/scenario.md | Extends manual scenario to verify menu item presses (Toast expectations). |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-header-menu-ios/index.tsx | Updates test app to attach menuElementIds and show Toasts on menu item presses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,9 +1,12 @@ | |||
| export interface StackHeaderMenuItem { | |||
| menuElementId: string; | |||
There was a problem hiding this comment.
- do we need
menuElementprefix if we're already in menu element? - should we unify naming with regular items (
keyvsid)?
There was a problem hiding this comment.
same applies to the content of PressMenuItemEvent
| menuElementId: string; | ||
| type: 'menuItem'; | ||
| title?: string | undefined; | ||
| onPress?: () => void; |
There was a problem hiding this comment.
| onPress?: () => void; | |
| onPress?: () => void | undefined; |
| largeSubtitle?: string | undefined; | ||
| largeTitleEnabled?: CT.WithDefault<boolean, false>; | ||
|
|
||
| onPressMenuItem?: CT.DirectEventHandler<PressMenuItemEvent> | undefined; |
There was a problem hiding this comment.
| onPressMenuItem?: CT.DirectEventHandler<PressMenuItemEvent> | undefined; | |
| onMenuItemPress?: CT.DirectEventHandler<MenuItemPressEvent> | undefined; |
nit, up to you
|
|
||
| export interface StackHeaderInlineItemIOS extends StackHeaderBaseItemIOS { | ||
| type: 'item'; | ||
| export interface SupportsMenuIOS { |
There was a problem hiding this comment.
tbh, do we even need this base type & do we need it exported? I'm not convinced about the name here.
| ...(leadingItems ?? []).filter(it => it && it.type === 'item'), | ||
| ...(trailingItems ?? []).filter(it => it && it.type === 'item'), | ||
| ); | ||
| const menu = findMenuElementByIdInItems( |
There was a problem hiding this comment.
| const menu = findMenuElementByIdInItems( | |
| const menuElement = findMenuElementByIdInItems( |
nit
| continue; | ||
| } | ||
|
|
||
| const menu = findMenuElementById(item.menu, menuElementId); |
There was a problem hiding this comment.
| const menu = findMenuElementById(item.menu, menuElementId); | |
| const menuElement = findMenuElementById(item.menu, menuElementId); |
| - (BOOL)emitOnPressMenuItem:(NSString *)menuElementId | ||
| { | ||
| if (_reactEventEmitter != nullptr) { | ||
| _reactEventEmitter->onPressMenuItem({.menuElementId = menuElementId.cString}); |
There was a problem hiding this comment.
e.g. in tabs we use RCTStringFromNSString, maybe we should do so here as well
{.selectedScreenKey = RCTStringFromNSString(payload.selectedScreenKey),
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1521
Description
This PR adds support for
onPressin iOS Stack v5 header.Changes
menuElementIdfor all menuElements, currently used foronPressin menu itemsonPressfor menu item - a callback defined per menu itemBefore & after - visual documentation
N/A
Test plan
Use
test-stack-header-menu-ios, follow the scenario.mdChecklist