-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Reader IA] Fix top navigation bar disappearing when system has low-memory #20114
[Reader IA] Fix top navigation bar disappearing when system has low-memory #20114
Conversation
The added code tries to wait until the topBarUiState is not null by polling for its data for a few times. If it's still null it simply doesn't run the code, to avoid calling topbar updates that should only happen if there is already a top bar visible. If it's not null after that check (normal scenario) then it calls the block and executes the logic.
Save information about the selected feed id and top bar filter ui state in the SavedInstanceState Bundle and recover it when the activity is destroyed due to system-initiated destruction (low-memory or "Don't keep activities" dev setting) to keep the experience consistent.
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
Generated by 🚫 dangerJS |
Avoid breaking existing unit tests.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## feature/reader-ia #20114 +/- ##
=====================================================
- Coverage 40.48% 40.47% -0.01%
=====================================================
Files 1447 1447
Lines 66788 66812 +24
Branches 11030 11030
=====================================================
+ Hits 27036 27045 +9
- Misses 37268 37279 +11
- Partials 2484 2488 +4 ☔ View full report in Codecov by Sentry. |
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.
Code LGTM @thomashorta , also I was able to replicate consistently on a pixel 4 emulator, the VM gets deallocated and with the PR patch when it is reinitialized restores the state correctly from the saved state 👍
I'm approving this one 🙇
Fixes #20105
A couple of things were done to fix this issue:
To Test:
Also, try doing that in other feeds, including selected blog/tag filters, to confirm the top bar does not disappear and saves the correct state.
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.UI Changes Testing Checklist: