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

[RNMobile] Prevent blocks being un-wantingly replaced with newly added blocks #49154

Merged
merged 3 commits into from
Mar 17, 2023

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented Mar 17, 2023

What?

The changes in logic surrounding dictation handling in #49056 led to a regression where currently selected blocks are replaced with newly added blocks. This PR addresses that issue by reverting the changes to dictation introduced in #49056.

Why?

Fixes #49145

How?

  • As part of [RNMobile] Fix dictation regression on iOS #49056, the isInsertingDictationResult bool was set to true for devices running iOS 16 or later. However, this meant that the textViewDidChange never ran, which then caused the focus/replacement regression:

func textViewDidChange(_ textView: UITextView) {
guard isInsertingDictationResult == false else {
return
}
propagateContentChanges()
updatePlaceholderVisibility()
//Necessary to send height information to JS after pasting text.
textView.setNeedsLayout()
}

  • The removal of the logic that removed the objectPlaceholder also caused an obj symbol to display amidst text when previewed.

  • With the above considered, the dictation fix has been reverted, so that the regression described in [RNMobile] Newly block replaces currently selected block on iOS #49145 is fixed and we can continue with the Gutenberg Mobile release.

Testing Instructions

It should be confirmed that these changes fix the regression it's addressing:

  1. Navigate through the steps to create a new post in the iOS app.
  2. Type some text into a paragraph block.
  3. Tap the inserter in the editor's toolbar.
  4. Select a new block to add to the editor.
  5. Confirm that the new block is added beneath the currently select block, and that it doesn't replace it.

In addition, we should confirm that the changes in this PR don't cause further issues with dictation:

  1. Navigate to My Site → Posts and create a new post.
  2. Activate the iOS dictation feature and dictate some text.
  3. Notice the text added to the editor.
  4. Tap another block on the editor or begin typing onto the keyboard to confirm there is no content loss.
  5. Experiment with adding various text and attempting to break dictation.

Screenshots or screencast

You'll see in the following screen recording that a newly added block is correctly added beneath a currently selected block (it doesn't replace the selected block):

replacement.mov

At the moment, the isInsertingDictationResult bool is set to true for devices running iOS 16 and later, which causes conflicts with logic later in the code. With this commit, the logic is changed, so that the bool is always the same for all devices, preventing any conflicts with the logic.
@SiobhyB SiobhyB self-assigned this Mar 17, 2023
@SiobhyB SiobhyB added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Regression Related to a regression in the latest release labels Mar 17, 2023
@SiobhyB SiobhyB marked this pull request as ready for review March 17, 2023 09:12
@SiobhyB SiobhyB requested a review from fluiddot March 17, 2023 09:13
@github-actions
Copy link

github-actions bot commented Mar 17, 2023

Flaky tests detected in 2f6408f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4446524553
📝 Reported issues:

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 ! Thanks @SiobhyB for fixing this 🏅 !

I tested it on a real device using iOS 15.4:

  • Paragraph block with text is not replaced when adding a new block
  • Dictation works

And on iOS 16 using a simulator:

  • Paragraph block with text is not replaced when adding a new block
  • Dictation works ⚠️ : I noticed that after the dictation ends, the Unicode character \u{FFFC} is added but it's not removed (see attached video capture). I think the only side-effect of leaving the character is that adds extra empty space, so probably not a critical issue.

NOTE: The word I'm saying when dictating is hey.

Kapture.2023-03-17.at.10.58.28.mp4

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

I confirm the revert matches the original code. Thanks!

@SiobhyB SiobhyB merged commit 584c1d2 into trunk Mar 17, 2023
@SiobhyB SiobhyB deleted the rnmobile/fix/focus-replacement-bug branch March 17, 2023 11:02
@github-actions github-actions bot added this to the Gutenberg 15.5 milestone Mar 17, 2023
SiobhyB pushed a commit that referenced this pull request Mar 17, 2023
…d blocks (#49154)

The changes in logic surrounding dictation handling in #49056 led to a regression where currently selected blocks are replaced with newly added blocks. This PR addresses that issue by reverting the changes to the dictation logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RNMobile] Newly block replaces currently selected block on iOS
2 participants