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

[iOS]Fix: Oddly formatted text hangs Gutenberg #1352

Merged
merged 2 commits into from
Sep 5, 2019

Conversation

pinarol
Copy link
Contributor

@pinarol pinarol commented Sep 4, 2019

Fixes #1354

WPiOS PR

Root Cause Analysis

I used Time Profiler to see what's taking so long and it turns out the enumeration on attributes was taking so long. So I put beginEditing/endEditing calls to make the updates as a batch.

This is still not perfect because it is a very long text but I observed that on a iPhone 6 simulator the opening of the page dropped from ~55secs to ~5secs.

About Text Getting Invisible
Also, you might observe that text appears as invisible. This is a known iOS behavior about rendering very long text inside a non-scrolling UITextView. If you split the block at some point you can observe that the text is visible again. The ideal world for iOS would be allowing the text scroll inside a smaller viewport, that way the text color doesn't get invisible. We had the same issue on HTML mode and I fixed it that way. Although as far as I know real devices are performing better compared to simulators about this, so we might not need to worry a lot currently.

To test:

WPiOS App Tests

Checkout the branch of WPiOS PR
Run rake dependencies
Follow the instructions in the issue
Verify that the screen is not hanging for a long time

Apart from this, some smoke test is done in paragraph/heading blocks to make sure we are not causing regression.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

Copy link
Member

@koke koke left a comment

Choose a reason for hiding this comment

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

It still takes a couple seconds on an iPhone XS, and moving the cursor around is pretty slow, but it's so much better than before and at least it shouldn't crash now 😅

@pinarol
Copy link
Contributor Author

pinarol commented Sep 5, 2019

Thanks for the review @koke 🎉 Yes I noticed that it is still not super good but couldn't detect more improvement points ATM 🤔

@pinarol pinarol merged commit eeace3d into develop Sep 5, 2019
@pinarol pinarol deleted the fix/infinite-loop-on-long-text branch September 5, 2019 18:21
@koke
Copy link
Member

koke commented Sep 6, 2019

Just for reference, I was looking at the performance tab in Chrome while testing this on iOS, and the cursor moving is slow because both of RichText's onSelectionChange and __experimentalGetAccessibilityLabel create a new record to do their calculations and each call to create takes ~120ms in the simulator for this sample text.

daniloercoli added a commit that referenced this pull request Sep 19, 2019
…rg-mobile into add/autosave-monitor

* 'develop' of https://github.com/wordpress-mobile/gutenberg-mobile:
  [iOS]Fix: Oddly formatted text hangs Gutenberg (#1352)
  Remove redundant bg color within button appender (#1348)
  Update bundles
  Update package.json version to 1.12.0

# Conflicts:
#	bundle/android/App.js
#	bundle/android/App.js.map
#	bundle/ios/App.js
#	bundle/ios/App.js.map
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor extremely slow or will not open oddly formatted text
2 participants