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] Fix dictation regression on iOS #49056

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented Mar 14, 2023

Proposed fix for wordpress-mobile/gutenberg-mobile#5165

What?

The changes in this PR fixes an issue with dictation not working as expected on devices running iOS 16 or later. To achieve this, an old dictation-related hack has been removed for later iOS version.

Why?

The dictation hack stopped being necessary following improvements made to dictation in iOS 16‌.

Now, with devices that are running iOS 16 or later, text added with iOS dictation is lost when tapping into the editor or typing while the dictation is still recording. This causes unexpected content loss, and could be viewed as an accessibility issue for those who rely on dictation to use the editor.

How?

The hack that was first introduced in wordpress-mobile/gutenberg-mobile#610 has been confined to only run for older versions of iOS 16.

It's necessary to keep this in place for older versions. ⤵️

If we remove the hack for older versions, an obj symbol is generated when dictating, as reported in wordpress-mobile/gutenberg-mobile#1969. Screenshot taken from an iPhone 6s (iOS 15.5) emulator with the hack removed:

You can refer to the comment section of wordpress-mobile/gutenberg-mobile#5165 for further context on how this solution came about.

Testing Instructions

Note
There are times where capitalisation is lost while dictating. This will be reported in a separate GitHub issue.

An installable build is available at wordpress-mobile/WordPress-iOS#20335 for testing on a physical device.

The following steps need to be tested with both iOS 16 or later and an earlier version of iOS:

  1. Navigate to My SitePosts 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.

It would also be helpful to go through the testing steps in the following PRs, where the hacks were originally introduced, to ensure there are no similar issues now they've been removed:

Screenshots or screencast

Screen.Recording.2023-03-15.at.20.29.08.mov

@SiobhyB SiobhyB force-pushed the rnmobile/fix/dictation-regression branch from b1acc0d to 46f7468 Compare March 14, 2023 10:37
@github-actions
Copy link

github-actions bot commented Mar 14, 2023

Flaky tests detected in 274ecde17bce0b1a894698f785c7639b622a96cb.
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/4429990923
📝 Reported issues:

@SiobhyB SiobhyB self-assigned this Mar 15, 2023
@SiobhyB SiobhyB added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Mar 15, 2023
@SiobhyB SiobhyB force-pushed the rnmobile/fix/dictation-regression branch from fc5eacf to 274ecde Compare March 15, 2023 19:04
@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) Mobile App - Automation Label used to initiate Mobile App PR Automation and removed Mobile App - Automation Label used to initiate Mobile App PR Automation labels Mar 15, 2023
@SiobhyB SiobhyB force-pushed the rnmobile/fix/dictation-regression branch from a57db24 to 4321062 Compare March 15, 2023 20:01
By confining the hack to iOS 16 or above, we fix an issue with dictation causing content loss in later versions of iOS.
isInsertingDictationResult = false
self.text = self.text?.replacingOccurrences(of: objectPlaceholder, with: dictationText)
if #available(iOS 16, *) {
insertText(dictationText)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note, I had originally attempted to call super.insertDictationResult(), so that we could rely on the default function, but this caused a crash.

} else {
let objectPlaceholder = "\u{FFFC}"
isInsertingDictationResult = false
self.text = self.text?.replacingOccurrences(of: objectPlaceholder, with: dictationText)
Copy link
Contributor Author

@SiobhyB SiobhyB Mar 15, 2023

Choose a reason for hiding this comment

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

It's necessary to keep this in place for older version, as without it an obj icon is added along with dictated text.

Screenshot taken from an iPhone 6s (iOS 15.5) emulator with the hack removed:

Simulator Screen Shot - iPhone 6s (15 5) - 2023-03-15 at 17 33 15

Copy link
Member

@derekblank derekblank left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this @SiobhyB! I was able to test dictation successfully following the testing steps on this PR, and the other PRs mentioned, and did not note any content loss.

I did note the issue where multiline dictations do not expand the text input for each line until the user taps out of the block, as mentioned and described in your comment here, and also this video example from the same thread. I agree that your ideas to use isInsertingDictationResult or textViewDidChange would be a good approach to leverage in a later PR. Also, I did not run into the capitalization issue mentioned in that comment. 👍

LGTM! 🚀

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Mar 16, 2023

Thank you so much @derekblank! 🙌

I did note the issue where multiline dictations do not expand the text input for each line until the user taps out of the block

I'll create a separate GitHub issue for this so we don't lose track and it can be tackled in later PRs, as you suggested 🙇‍♀️

@SiobhyB SiobhyB merged commit 43b8db2 into trunk Mar 16, 2023
@SiobhyB SiobhyB deleted the rnmobile/fix/dictation-regression branch March 16, 2023 08:56
@github-actions github-actions bot added this to the Gutenberg 15.5 milestone Mar 16, 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.
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
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants