-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
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.
Flaky tests detected in 2f6408f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4446524553
|
There was a problem hiding this 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
There was a problem hiding this 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!
Gutenberg Mobile
: [RNMobile] Prevent blocks being un-wantingly replaced with newly added blocks wordpress-mobile/gutenberg-mobile#5569What?
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?
isInsertingDictationResult
bool was set totrue
for devices running iOS 16 or later. However, this meant that thetextViewDidChange
never ran, which then caused the focus/replacement regression:gutenberg/packages/react-native-aztec/ios/RNTAztecView/RCTAztecView.swift
Lines 787 to 796 in d76d917
The removal of the logic that removed the
objectPlaceholder
also caused anobj
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:
In addition, we should confirm that the changes in this PR don't cause further issues with 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