Skip to content
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

Replace FragmentTransaction.show/hide with attach/detach - fix lifecycle #11927

Merged
merged 1 commit into from
May 15, 2020

Conversation

malinajirka
Copy link
Contributor

The recent changes to the BottomNavigation component introduced an anticipated issue with the fragment lifecycle. When show/hide methods are used, even fragments which are not visible on the screen are still in the RESUMED state. This behavior has several downsides - it requires way more memory and also breaks tons of behavior which relies on onPause being invoked when the fragment goes to background. A couple of examples would be tracking and registration to an event bus.

To test:

  • Make sure bottom navigation works
  • Make sure only a single instance of each fragment lives in the memory
  • Make sure onPause is called when you select a different tab

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APK here.

Copy link
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked the behaviour, debugged and profiled the app and it seems to work well 👍 good job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants