-
-
Notifications
You must be signed in to change notification settings - Fork 514
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.
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
fix(android): disappearing search icon on opening transparent modal #2274
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have some explanation what was the error mechanism & why the previous code does not work.
@kkafar I added a brief explanation to the description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so the onPrepareOptionsMenu
method is called for every screen on the stack.
Hypothesis: it seems that Android looks for closest menu in view hierarchy to update, hence if a screen does not have a header we try to update our sibling's header.
Usually this works fine because we unmount the screen below on the stack. When transparent stack presentation is in use we try to update the sibling's toolbar with transparent screen's null
header config, thus effectively clearing it, leading to the bug.
The logic fix is ok, however we need to tune it up a bit. See below.
Good job overall!
android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're good now!
…oftware-mansion#2274) ## Description This PR fixes disappearing search icon of the previous screen when navigating to a transparent modal with hidden header. When navigating to a screen with `presentation: "transparentModal"` and `headerShown: false` the toolbar menu was unnecessarily updated with new screen's options. The update caused the search icon of the previous screen (still visible in the background) to disappear. Fixes software-mansion#2271 . ## Changes - added `Test2271.tsx` repro - updating the toolbar menu conditionally ## Screenshots / GIFs ### Before https://github.com/user-attachments/assets/390f054b-b003-4b7f-b6f5-9aa9fd936dca ### After https://github.com/user-attachments/assets/cb1fc795-c7a3-4514-9bb5-b7e1995d0b90 ## Test code and steps to reproduce - use `Test2271.tsx` for repro ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
Description
This PR fixes disappearing search icon of the previous screen when navigating to a transparent modal with hidden header.
When navigating to a screen with
presentation: "transparentModal"
andheaderShown: false
the toolbar menu wasunnecessarily updated with new screen's options. The update caused the search icon of the previous screen (still visible in the background) to disappear.
Fixes #2271 .
Changes
Test2271.tsx
reproScreenshots / GIFs
Before
Screen.Recording.2024-07-31.at.18.42.53.mov
After
Screen.Recording.2024-07-31.at.18.41.43.mov
Test code and steps to reproduce
Test2271.tsx
for reproChecklist