Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions WordPress/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@
android:windowSoftInputMode="stateVisible" />
<activity
android:name=".ui.posts.EditPostActivity"
android:configChanges="fontScale"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm quite concerned when we touch the EditPostActivity at a very high level since it means we're effectively touching Aztec-standalone too and the scope if very big there.

In this case, I tried out changing the OS font size on a non-blocks post, in Aztec, and it seems that something goes wrong and all the images/videos/paragraphs are one on top of the other:

aztec-problem-if-configchange-fontsize

If our best solution at the moment is to adopt the fontScale config change and handle it ourselves, I might feel inclined to propose a solution like the option I mentioned in #8874 (comment), where the idea is to introduce a subclass of the EditPostActivity only to be able to specialize its manifest definition and similar.

In the meantime, can you check the issue shown in the gif @daniloercoli to see if it's fixable? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the issue I think is fixable without restarting Aztec but requires small changes to it. Calling RefreshText alone doesn't fix it, instead we need to reload the pictures/videos and re-layout them on configChanges. It alone is a rather small change, and we can try it and see the result.
Should we invest time on this?

Otherwise, for the reasons you've mentioned above, I think that's best to exclude Aztec from the config changes code altogether, and introduce another Activity...it could be a big PR though. Wondering if we should following this path for an event (font size change in settings) that is going to happen rarely.

Copy link
Contributor

Choose a reason for hiding this comment

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

instead we need to reload the pictures/videos and re-layout them on configChanges. It alone is a rather small change, and we can try it and see the result.
Should we invest time on this?

I think this might be a rabbit hole. Plus, I don't think we know which other code or flow breaks in Aztec if we lift the assumption of the activity re-creation.

OTOH, going the new-Activity way for this particular issue (the font-size change) that's not critical is a bit much. Let's leave this PR unmerged and we give it some more thought?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

going the new-Activity way for this particular issue (the font-size change) that's not critical is a bit much.

Yes, 100% agree with you on the points mentioned above. The only way to be sure to not break Aztec is to duplicate the Activity, but it feels way to much for a very rare writing flow (user needs to change the font size when the editor is open on the screen. If they change the font size and then open the editor the new font size is respected).

android:theme="@style/Calypso.ActionMode.Dark"
android:windowSoftInputMode="stateHidden|adjustResize">
<meta-data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ enum RestartEditorOptions {
// for keeping the media uri while asking for permissions
private ArrayList<Uri> mDroppedMediaUris;

private float mConfigurationFontSize;

private boolean isModernEditor() {
return mShowNewEditor || mShowAztecEditor || mShowGutenbergEditor;
}
Expand Down Expand Up @@ -587,6 +589,8 @@ public void run() {
}
});

mConfigurationFontSize = getResources().getConfiguration().fontScale;

ActivityId.trackLastActivity(ActivityId.POST_EDITOR);
}

Expand Down Expand Up @@ -801,6 +805,7 @@ protected void onSaveInstanceState(Bundle outState) {
outState.putBoolean(STATE_KEY_HTML_MODE_ON, mHtmlModeMenuStateOn);
outState.putSerializable(WordPress.SITE, mSite);
outState.putParcelable(STATE_KEY_REVISION, mRevision);
outState.putFloat("mConfigurationFontSize", mConfigurationFontSize);

outState.putSerializable(STATE_KEY_EDITOR_SESSION_DATA, mPostEditorAnalyticsSession);
mIsConfigChange = true; // don't call sessionData.end() in onDestroy() if this is an Android config change
Expand All @@ -822,12 +827,33 @@ protected void onRestoreInstanceState(Bundle savedInstanceState) {
if (savedInstanceState.getBoolean(STATE_KEY_IS_PHOTO_PICKER_VISIBLE, false)) {
showPhotoPicker();
}

mConfigurationFontSize = savedInstanceState.getFloat("mConfigurationFontSize", 0f);
}

@Override
public void onConfigurationChanged(Configuration newConfig) {
super.onConfigurationChanged(newConfig);


if (newConfig.fontScale != mConfigurationFontSize && (mEditorFragment instanceof GutenbergEditorFragment)) {
// Configuration has changed: Save the new fontScale and update the GB editor fragment(s).
// We can't rely on the Android and RN font scaling system, since we're using GB mobile
// in a retained fragment, and the configuration changed code is not called automatically.
// `setRetainInstance(true)` was set here
// https://github.com/wordpress-mobile/WordPress-Android/pull/9030
// in order to fix a couple of issues: history lost, and device rotation issues.
GutenbergEditorFragment gbFragment = (GutenbergEditorFragment) mEditorFragment;
try {
updatePostObject(true);
} catch (EditorFragmentNotAddedException e) {
AppLog.e(T.EDITOR, "Impossible to save the post, we weren't able to update it.");
return;
}
mConfigurationFontSize = newConfig.fontScale;
gbFragment.updateEditorContentAndReload(mPost.getTitle(), mPost.getContent());
}

// resize the photo picker if the user rotated the device
int orientation = newConfig.orientation;
if (orientation != mPhotoPickerOrientation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class GutenbergContainerFragment extends Fragment {
private static final String ARG_LOCALE = "param_locale";
private static final String ARG_TRANSLATIONS = "param_translations";

private static final String KEY_HTML_MODE_ENABLED = "KEY_HTML_MODE_ENABLED";

private boolean mHtmlModeEnabled;
private boolean mHasReceivedAnyContent;

Expand Down Expand Up @@ -53,6 +55,10 @@ public void onCreate(Bundle savedInstanceState) {
String localeString = getArguments().getString(ARG_LOCALE);
Bundle translations = getArguments().getBundle(ARG_TRANSLATIONS);

if (savedInstanceState != null) {
mHtmlModeEnabled = savedInstanceState.getBoolean(KEY_HTML_MODE_ENABLED);
}

mWPAndroidGlueCode = new WPAndroidGlueCode();
mWPAndroidGlueCode.onCreate(getContext());
mWPAndroidGlueCode.onCreateView(
Expand Down Expand Up @@ -90,6 +96,11 @@ public void onDestroy() {
mWPAndroidGlueCode.onDestroy(getActivity());
}

@Override
public void onSaveInstanceState(Bundle outState) {
outState.putBoolean(KEY_HTML_MODE_ENABLED, mHtmlModeEnabled);
}

public void setTitle(String title) {
mWPAndroidGlueCode.setTitle(title);
mHasReceivedAnyContent = mWPAndroidGlueCode.hasReceivedInitialTitleAndContent();
Expand Down Expand Up @@ -145,4 +156,13 @@ public void mediaFileUploadSucceeded(final int mediaId, final String mediaUrl, f
public void clearMediaFileURL(final int mediaId) {
mWPAndroidGlueCode.clearMediaFileURL(mediaId);
}

/**
* Recreate the react application and context.
* See: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/
* react/ReactInstanceManager.java#L350
*/
public void recreateReactContextInBackground() {
mWPAndroidGlueCode.recreateReactContextInBackground();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class GutenbergEditorFragment extends EditorFragmentAbstract implements
IHistoryListener {
private static final String KEY_HTML_MODE_ENABLED = "KEY_HTML_MODE_ENABLED";
private static final String KEY_EDITOR_DID_MOUNT = "KEY_EDITOR_DID_MOUNT";
private static final String KEY_TITLE_BEFORE_CONFIG_CHANGES = "KEY_TITLE_BEFORE_CONFIG_CHANGES";
private static final String KEY_CONTENT_BEFORE_CONFIG_CHANGES = "KEY_CONTENT_BEFORE_CONFIG_CHANGES";
private static final String ARG_IS_NEW_POST = "param_is_new_post";
private static final String ARG_LOCALE_SLUG = "param_locale_slug";

Expand All @@ -85,6 +87,9 @@ public class GutenbergEditorFragment extends EditorFragmentAbstract implements

private ProgressDialog mSavingContentProgressDialog;

private String mTitleBeforeConfigChanged = null;
private String mContentBeforeConfigChanged = null;

public static GutenbergEditorFragment newInstance(String title,
String content,
boolean isNewPost,
Expand Down Expand Up @@ -202,6 +207,8 @@ public void onCreate(Bundle savedInstanceState) {
if (savedInstanceState != null) {
mHtmlModeEnabled = savedInstanceState.getBoolean(KEY_HTML_MODE_ENABLED);
mEditorDidMount = savedInstanceState.getBoolean(KEY_EDITOR_DID_MOUNT);
mTitleBeforeConfigChanged = savedInstanceState.getString(KEY_TITLE_BEFORE_CONFIG_CHANGES, null);
mContentBeforeConfigChanged = savedInstanceState.getString(KEY_CONTENT_BEFORE_CONFIG_CHANGES, null);
}
}

Expand Down Expand Up @@ -260,6 +267,14 @@ public void onEditorDidMount(boolean hasUnsupportedBlocks) {
@Override
public void run() {
setEditorProgressBarVisibility(!mEditorDidMount);
if (mTitleBeforeConfigChanged != null) {
setTitle(mTitleBeforeConfigChanged);
mTitleBeforeConfigChanged = null;
}
if (mContentBeforeConfigChanged != null) {
setContent(mContentBeforeConfigChanged);
mContentBeforeConfigChanged = null;
}
}
});
}
Expand Down Expand Up @@ -452,6 +467,8 @@ public void onAttach(Activity activity) {
public void onSaveInstanceState(Bundle outState) {
outState.putBoolean(KEY_HTML_MODE_ENABLED, mHtmlModeEnabled);
outState.putBoolean(KEY_EDITOR_DID_MOUNT, mEditorDidMount);
outState.putString(KEY_TITLE_BEFORE_CONFIG_CHANGES, mTitleBeforeConfigChanged);
outState.putString(KEY_CONTENT_BEFORE_CONFIG_CHANGES, mContentBeforeConfigChanged);
}

@Override
Expand Down Expand Up @@ -774,4 +791,15 @@ public void run() {

return true;
}

// Save a temporary copy of the content in GB mobile, and restore it when the editor is refreshed
// Note: History, block focused, scroll and caret position are lost after this call!
public void updateEditorContentAndReload(String title, String content) {
mContentBeforeConfigChanged = content;
mTitleBeforeConfigChanged = title;
// set this to false otherwise the spinning dialog is not shown
mEditorDidMount = false;
setEditorProgressBarVisibility(mEditorDidMount);
mRetainedGutenbergContainerFragment.recreateReactContextInBackground();
}
}