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

RNAztec iOS: Fix removing content after dictation #700

Merged
merged 3 commits into from
Mar 7, 2019

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Mar 5, 2019

Fixes #673

This issue was originally fixed here #610, but it resurfaced.

The original fix was blocking the empty strings to be sent from textViewDidChange to the RN side, when UIKit was inserting the dictation text. That empty string was finally overwriting the content when it was sent back from RN to Native, since with an empty string, the JS side nullifies the eventCount.

I noticed that in this case, the same was happening on textViewDidChangeSelection, so using the same strategy there solved the problem.

As discussed in the original PR, this is not the best fix, but a better solution will probably involve all platforms. If this continue being an issue in the future, I'd suggest to give some time to find that better solution.

dictation

To test:

  • Build and run the project.
  • Create an empty paragraph block.
  • Test the iOS dictation feature.
  • Check that it behaves properly.

…emoving the inserted content from dictation.
@etoledom etoledom added [Type] Bug Something isn't working [OS] iOS labels Mar 5, 2019
@etoledom etoledom added this to the v1.1 milestone Mar 5, 2019
@etoledom etoledom self-assigned this Mar 5, 2019
@etoledom etoledom requested a review from pinarol March 5, 2019 10:10
@pinarol
Copy link
Contributor

pinarol commented Mar 6, 2019

hey @etoledom 👋 I tried testing this on the WPiOS app and there seems to be some problems:

  1. it can’t write the whole word
  2. caret position is messed up
  3. dictation listening ends very early I don’t know why

(iPhone 6, iOS 12.0.1)

ezgif com-video-to-gif 8

@etoledom
Copy link
Contributor Author

etoledom commented Mar 6, 2019

Thank you for the review @pinarol !

I updated the PR from develop and it seems to fix that issue you mentioned. Testing on WPiOS it seemed necessary to update the JS Bundle, since the app was crashing without metro running, so I did that too.

I also opened a WPiOS PR for easier testing with a description of the original issue:
wordpress-mobile/WordPress-iOS#11198

Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

Works and looks pretty good ! 🎉

@etoledom etoledom merged commit a7af71c into develop Mar 7, 2019
@etoledom
Copy link
Contributor Author

etoledom commented Mar 7, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[OS] iOS [Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Dictation doesn't work
2 participants