Ability to attach ReactRootView to different Activity #483
Conversation
This is useful for retaining the RN view/context. RN is still able to adapt to the new Activity's layout, which is super useful for portrait/landscape.
Otherwise, multiple Aztec instances autofocus and the exact caret position is lost on rotation.
hypest
force-pushed
the
feature/android-attach-to-new-activity
branch
from
f2dd578
to
2550973
Compare
| } | ||
|
|
||
| @Override | ||
| public void mediaUploadSync(MediaUploadCallback mediaUploadCallback) { | ||
| mPendingMediaUploadCallback = mediaUploadCallback; | ||
| onReattachQueryListener.onQueryCurrentProgressForUploadingMedia(); | ||
| mOnReattachQueryListener.onQueryCurrentProgressForUploadingMedia(); |
There was a problem hiding this comment.
Wondering if there could be a case where this listener can be called before the view is attached to a container. In that case we need to check that are not null(s)?
There was a problem hiding this comment.
In principal it could indeed be called before attaching the view, but that would be a bug (lifecycle error). Letting that throw would be a better handling, at least for the first implementation.
There is another case though, where the listener won't be null but, it will be old: after rotation and before the View gets re-attached, the listener will point to a previous instance. That can be buggy or crashing. Even in this case though, handling it is not a solution. We can alleviate the crash but, that doesn't recover the situation.
I think the callback-based solution, along with the async nature of title/content getters setters has these shortcomings. We will need a better messaging system.
| private void refocus() { | ||
| if (mLastFocusedView != null) { | ||
| // schedule a request for focus | ||
| new Handler(Looper.getMainLooper()).post(new Runnable() { |
There was a problem hiding this comment.
As mentioned in chat convo this could introduce some lag, but in my opinion it's unlikely to happen on "normal" workflow. Even it that happens, the focus/caret is restored properly with just a bit of delay.
|
I wasn't fully aware of what were the benefits of
Not quite sure if this can affect us in some way, since we could set the Title and the Content, and then start the RN application. Later we can attach it to a new view, and a different context. |
|
I think we will need to keep an eye on this solution anyway. It's hacky from its start. I mean, trying to retain a view while its Activity is rotating is quite against the Android best practices. It's only a necessity because we want to not touch the Aztec Activity (EditPostActivity over at the wpandroid repo) and continue to allow it to recreate on rotation. We'll want to revise that preference (to not touch Aztec-standalone at all) down the road as Gutenberg-mobile gets more stable. |
This is useful for retaining the RN view/context while rotating the device (and letting the Activity get recreated) or even when interested in attaching the view to a completely different Activity (say, preloading RN before the editor Activity appears).
RN is still able to adapt to the new Activity's layout, which is super useful for portrait/landscape/etc.
This PR is not opened for merging yet. Rather, it's purpose is to be able to try it easily and have a discussion.
Implementation Details
The solution here is based on
MutableContextWrapperwith its ability to change the Context after the ReactRootView has already been created. RN will still receive lifecycle events before and affter the change since those are propagated.After the Context gets changed, we need to remove the View from its previous parent (if any, happens on rotation) and add it to a new parent.
To test
Use this PR on wpandroid wordpress-mobile/WordPress-Android#9030.