Exclude EditPostActivity from handling orientation change for alpha#8800
Exclude EditPostActivity from handling orientation change for alpha#8800
Conversation
marecar3
left a comment
There was a problem hiding this comment.
Tested and it seems that it's working good 👍
While I was working on wordpress-mobile/gutenberg-mobile#377
and trying different solutions I had exactly the same thoughts as you had, so it seems that this solution is fair enough for now.
|
Thanks for the review @marecar3 - as discussed elsewhere we're deferring it for |
|
From what has been discussed we can derive we're not leaning towards risking changing |
Fixes wordpress-mobile/gutenberg-mobile#377, #8739, and load time issues mentioned in #8739 (comment) and elsewhere
This PR adds a specific
AndroidManifest.xmlmerge forEditPostActivity, declaring the following attributes:keyboard|keyboardHidden|orientation|screenSize. What this effectively means is thatEditPostActivitynow does not handle orientation changes, and lets the changes in configuration be listened to and handled by the underlying RN implementation.Been looking into this and trying different approaches:
GutenbergEditorFragmentas a host, a first idea was to usesetRetainInstance(true)(see Avoid Gutenberg reload times on rotation #8790 and Keeps reference to mReactInstanceManager to avoid re-creating on rotation gutenberg-mobile#382) and keep the fragment's instance alive with the RN code alive as well. Unfortunately, even by doing this meant the re-wiring needed was not trivial, and even if so, the RN lifecycle relies heavily on listening to the host's Activity lifecycle # #, apparently re-initializing things internally as the Activity lifecycle wired events were run (so, keeping the Fragment instance wasn't really helping here).orientation|screenSizemodifiers in theorientationattribute, which indicates the Activity is in charge of handling orientation changes (and thus it not going to be destroyed for this reason). Also searching a bit for it found out most of the work is handled by RN itself on one activity, so it makes sense generally speaking to keep the Activity instance as well #.Given the situation in which the user is writing has the
EditPostActivityon the foreground most of the time, an orientation change losing the focus / scroll position or making the user wait to reload seems less than ideal. On the contrary, if the user is writing and switches the app to do something else, then it perfectly makes sense for it to occasionally need to re-load the content.This seems a good solution for the situation we're trying to handle, which aims at recognizing the orientaiton change (which RN does well and adjusts the contents to fit the new screen size after rotation) while not affecting the writing flow (which until this PR meant waiting for reload, losing scroll position and undo/redo history among other hurdles).
Note: this PR makes the
EditPostActivitynot be killed on screen orientation changes also for Aztec and Visual Editors as the implementation for each is done by means of Fragments, but I chose to keep the PR simple foralphaonly as opposed to making a newEditPostActivityclass only for Gutenberg. We'll have to create one though, once we get into Beta.To test:
Update release notes:
RELEASE-NOTES.txt.