feat(Android, Stack v5): expose showAsAction prop for toolbar menu items#4101
Open
kligarski wants to merge 9 commits into
Open
feat(Android, Stack v5): expose showAsAction prop for toolbar menu items#4101kligarski wants to merge 9 commits into
kligarski wants to merge 9 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an Android-only showAsAction configuration for Stack v5 (gamma) header toolbar menu items, including support for runtime updates via the existing setToolbarMenuItemOptions command, plus a codegen workaround to keep iOS builds passing.
Changes:
- Introduces
showAsActionfortoolbarMenuItems(props + imperative updates) with a new enum mapping to AndroidMenuItem#setShowAsActionflags. - Adds a Fabric codegen workaround prop (
DO_NOT_USE) to force enum generation inProps.hand avoid iOS build failures. - Adds/updates Single Feature Test scenarios and aligns scenario-description conventions.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/fabric/gamma/stack/StackHeaderConfigAndroidNativeComponent.ts | Adds showAsAction to the codegen spec for toolbar menu items and introduces DO_NOT_USE workaround prop. |
| src/components/gamma/stack/index.ts | Re-exports newly exposed Android types (StackHeaderToolbarMenuItemClickedEvent, StackHeaderToolbarMenuItemShowAsActionAndroid). |
| src/components/gamma/stack/header/StackHeaderConfig.android.types.ts | Documents and exposes the showAsAction union type and prop in the public experimental API types. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-show-as-action-android/scenario.md | Adds manual test scenario coverage for all showAsAction variants and command updates. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-show-as-action-android/scenario-descriptions.ts | Adds scenario metadata for the new SFT. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-show-as-action-android/index.tsx | Implements the new SFT screen that drives prop + command updates for showAsAction. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-commands-android/scenario.md | Updates scenario formatting/details (OS version + prerequisites wording). |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-commands-android/scenario-descriptions.ts | Extracts scenario metadata into the shared convention file. |
| apps/src/tests/single-feature-tests/stack-v5/test-stack-toolbar-menu-commands-android/index.tsx | Uses extracted scenarioDescription instead of inline definition. |
| apps/src/tests/single-feature-tests/stack-v5/index.ts | Registers the new SFT scenario in the Stack v5 group. |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/toolbar/StackHeaderToolbarMenuItemShowAsAction.kt | Adds native enum and mapping to MenuItem show-as-action flags. |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/toolbar/StackHeaderToolbarMenuItemOptions.kt | Extends command options with showAsAction (and currently also showAsActionWithText). |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/toolbar/StackHeaderToolbarMenuItemDefaults.kt | Adds default SHOW_AS_ACTION = NEVER. |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/toolbar/StackHeaderToolbarMenuItemConfig.kt | Persists showAsAction as part of the menu item config. |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/toolbar/StackHeaderToolbarMenuCoordinator.kt | Applies showAsAction when creating/updating menu items; makes clear/updateItem internal. |
| android/src/main/java/com/swmansion/rnscreens/gamma/stack/header/config/StackHeaderConfigViewManager.kt | Parses showAsAction from props/commands and adds a no-op setter for DO_NOT_USE. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
showAsActionproperty to toolbar menu items in header on Android in Stack v5.Closes https://github.com/software-mansion/react-native-screens-labs/issues/1202.
API shape
The initial API idea was:
But this turned out to be problematic on the native side. You can only set the
showAsActionflags, you can't read them from theMenuItem. If the userchanged only one of these 2 props, we wouldn't know all the flags that need to
be applied. We could add state to the
ToolbarMenuCoordinatororHeaderConfigbut we would like to avoid it.
I decided to merge these 2 props into one enum. For
never, we don't need thewithTextmodifier so there are only 5 options possible which isn't bad.We could also use nested object but we've been trying to avoid complicated
nesting.
The final API therefore looks like this:
I'm open to suggestions though.
Codegen problem
When adding the
showAsAction?: CT.WithDefault<StackHeaderToolbarMenuItemShowAsActionAndroid, 'never'>;prop to
StackHeaderToolbarMenuItemAndroidinterface in native spec on Android,iOS build stopped working. In
Props.h, all types are generated (also forAndroid-only components). For some reason, if we use nested object as an array
(
toolbarMenuItems?: StackHeaderToolbarMenuItemAndroid[] | undefined;), theenum type isn't generated which causes the build to fail. After I added
DO_NOT_USE?: StackHeaderToolbarMenuItemAndroid | undefined;property, the enumis generated correctly. I assumed that it's better to add this workaround
instead of losing the typing completely.
Limited support for
ifRoomifRoomoption doesn't react to orientation changes. The maximum width ofaction items is determined on first layout and it remains that way even through
layout/orientation changes. This means that if you open the screen in portrait,
more items won't move from overflow menu to the toolbar when rotated to
landscape and if you open the screen in landscape, rotating to portrait won't
make the items move to the overflow menu, usually resulting in the title being
shrunk down.
portrait_to_landscape.mp4
landscape_to_portrait.mp4
This happens due to native implementation. Typically on Android, every
orientation change causes the activity to restart. The maximum width of action
menu items is evaluated only on initialization of
ActionMenuProesenter(
initForMenu). If activity restarts are disabled, as is the case inreact-nativeapps, the initially set width will still apply.Note
By the way, this value turns out to be half of the width of the device:
This value is then made smaller by subtracting the width of the collapse icon.
I tried looking for a way to force this value to reset. Unfortunately, I didn't
find any way that wouldn't involve heavy reflection usage or completely
resetting the menu, resulting in the loss of state (i.e. changes applied via
view commands would be completely lost). I think that keeping the state is more
important in this case but I'm open for discussion here.
While testing this I also noticed some problems with handling orientation
changes for the screen. I will investigate this separately
(https://github.com/software-mansion/react-native-screens-labs/issues/1499).
orientation_bug.mp4
Changes
showAsActionprop andStackHeaderToolbarMenuItemShowAsAction(Android)enumsDO_NOT_USE?: StackHeaderToolbarMenuItemAndroid | undefined;to native spec to ensure generation of enum inProps.hto fix build on iOSStackHeaderToolbarMenuItemClickedEventclear,updateIteminternal inToolbarMenuCoordinatortest-stack-toolbar-menu-commandsto new conventionVisual documentation
showAsAction.mp4
Test plan
Run
test-stack-toolbar-menu-show-as-action-androidSFT.Checklist