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

Text input gets very slow on some Android devices #2589

Closed
borisyankov opened this issue May 30, 2018 · 1 comment
Closed

Text input gets very slow on some Android devices #2589

borisyankov opened this issue May 30, 2018 · 1 comment
Labels
a-Android bug upstream: RN Issues related to an issue in React Native
Milestone

Comments

@borisyankov
Copy link
Contributor

borisyankov commented May 30, 2018

On some Android devices, in some specific cases, the 'compose' input responds very slowly to a point of being unusable.
Reported here:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/speed

The issue was reproduced on Pixel 2, so it is not a purely performance problem.

Very likely, it is related to:
facebook/react-native#19126

The fix then seems to be to make our compose box not-controlled.

This is not a bad idea as it will be a performance improvement regardless of the specific bug encountered, and something I wanted to do before this issue was reported.

@borisyankov borisyankov self-assigned this May 30, 2018
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue May 30, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue May 30, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
@gnprice gnprice changed the title Text input is slow on some Android devices Text input gets very slow on some Android devices May 30, 2018
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 4, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 5, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 5, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
gnprice pushed a commit that referenced this issue Jun 6, 2018
This is the beginning of a general performance improvement and also
a fix for #2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 7, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 7, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
@gnprice
Copy link
Member

gnprice commented Jun 9, 2018

An update: a couple of days ago (2018-06-06) we released a fix for this to all users through the Play Store, as version 13.5.89.

That version is in this repo (as a tag, and on a stable branch) but not merged to master; it's a preliminary version of #2595 which solves this Android-only problem (working around an Android-only bug in React Native) nicely, but in turn triggers a different, iOS-only bug in React Native.

I'm leaving this issue open here until we have a fix merged into master, though it's already fixed for users.

borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 11, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 12, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 13, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
@gnprice gnprice added the upstream: RN Issues related to an issue in React Native label Jun 14, 2018
@gnprice gnprice added this to the v14.x milestone Jun 20, 2018
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 21, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 21, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 21, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 21, 2018
This is the beginning of a general performance improvement and also
a fix for zulip#2589
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

* make sure we do update input's initial values using `updateTextInput`
in `componentDidMount`
* remove the `value={...}` props that make the inputs controlled
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

* make sure we do update input's initial values using `updateTextInput`
in `componentDidMount`
* remove the `value={...}` props that make the inputs controlled
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

* make sure we do update input's initial values using `updateTextInput`
in `componentDidMount`
* remove the `value={...}` props that make the inputs controlled
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

* make sure we do update input's initial values using `updateTextInput`
in `componentDidMount`
* remove the `value={...}` props that make the inputs controlled
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

Now both message and topic inputs are updated regardless of wether
they are controlled or not we remove the `value={...}`.

* make sure we do update input's initial values using `updateTextInput`
in `componentDidMount`
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 27, 2018
Fixes zulip#2589

Now both message and topic inputs are updated regardless of wether
they are controlled or not we remove the `value={...}`.
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 28, 2018
Fixes zulip#2589

Remove the `value={topic}` from topic text input making it
an uncontrolled component.

To replicate the previous behavior:
 * initialze at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on a
the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 28, 2018
Fixes zulip#2589

Remove the `value={topic}` from topic text input making it
an uncontrolled component.

To replicate the previous behavior:
 * initialze at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on a
the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion
borisyankov added a commit to borisyankov/zulip-mobile that referenced this issue Jun 29, 2018
Fixes zulip#2589

Remove the `value={topic}` from topic text input making it
an uncontrolled component.

To replicate the previous behavior:
 * initialze at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on a
the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion
gnprice pushed a commit to borisyankov/zulip-mobile that referenced this issue Jun 30, 2018
We want to make the compose box uncontrolled.  Instead of using the
`value` property to change the message input's and topic input's
text, we will call `setNativeProps` on a reference to the underlying
control.

Currently on iOS there is a React Native bug that affects uncontrolled
inputs: facebook/react-native#18272
On the other hand, in order to fix zulip#2589 we're eager to switch to
an uncontrolled input at least on Android, in order to avoid the
(different) underlying React Native bug that causes that.

So for now, as a suboptimal solution, we will support two versions of
the component.  For iOS it will remain unchanged, and we'll modify the
Android version of the file in subsequent commits.
gnprice pushed a commit to borisyankov/zulip-mobile that referenced this issue Jun 30, 2018
Remove the `value={topic}` from topic text input, making it
an uncontrolled component.

To replicate the previous behavior:
 * initialize at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on
   the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion

Fixes zulip#2589.
gnprice pushed a commit that referenced this issue Jun 30, 2018
We want to make the compose box uncontrolled.  Instead of using the
`value` property to change the message input's and topic input's
text, we will call `setNativeProps` on a reference to the underlying
control.

Currently on iOS there is a React Native bug that affects uncontrolled
inputs: facebook/react-native#18272
On the other hand, in order to fix #2589 we're eager to switch to
an uncontrolled input at least on Android, in order to avoid the
(different) underlying React Native bug that causes that.

So for now, as a suboptimal solution, we will support two versions of
the component.  For iOS it will remain unchanged, and we'll modify the
Android version of the file in subsequent commits.
gnprice pushed a commit that referenced this issue Jun 30, 2018
Remove the `value={topic}` from topic text input, making it
an uncontrolled component.

To replicate the previous behavior:
 * initialize at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on
   the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion

Fixes #2589.
jackrzhang pushed a commit to jackrzhang/zulip-mobile that referenced this issue Jul 17, 2018
We want to make the compose box uncontrolled.  Instead of using the
`value` property to change the message input's and topic input's
text, we will call `setNativeProps` on a reference to the underlying
control.

Currently on iOS there is a React Native bug that affects uncontrolled
inputs: facebook/react-native#18272
On the other hand, in order to fix zulip#2589 we're eager to switch to
an uncontrolled input at least on Android, in order to avoid the
(different) underlying React Native bug that causes that.

So for now, as a suboptimal solution, we will support two versions of
the component.  For iOS it will remain unchanged, and we'll modify the
Android version of the file in subsequent commits.
jackrzhang pushed a commit to jackrzhang/zulip-mobile that referenced this issue Jul 17, 2018
Remove the `value={topic}` from topic text input, making it
an uncontrolled component.

To replicate the previous behavior:
 * initialize at the start in `componentDidMount`
 * make sure the topic input text is updated when we focus on
   the message input (it is invisible until then)
 * change the text on picking a topic autocomplete suggestion

Fixes zulip#2589.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-Android bug upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

No branches or pull requests

3 participants