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.

Sign up for GitHub

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

Jump to bottom

Make ComposeBox message and topic inputs uncontrolled #2738

Merged
merged 4 commits into from
Jun 30, 2018
Merged

Make ComposeBox message and topic inputs uncontrolled #2738

merged 4 commits into from
Jun 30, 2018

Commits on Jun 30, 2018

  1. compose: Split into Android and iOS specific versions. 

    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.
    @borisyankov @gnprice
    borisyankov authored and gnprice committed Jun 30, 2018
    Configuration menu
    Copy the full SHA
    cb2589e View commit details
    Browse the repository at this point in the history

  2. compose: Factor out "clear input" logic into updateTextInput. 

    This will let us re-use the same logic for the topic input, and for
initializing the inputs on mount when we make them uncontrolled.
While we're here, add some comments explaining the trickier bits
of this logic.

We also add a `setMessageInputValue` wrapper that also makes sure to
call the event handler `handleMessageChange`, for consistency in
behavior with what gets called when a user types text themselves.

The new function adds a call to `setNativeProps`.  This doesn't have
any effect as long as the input is controlled, and it makes a simpler
transition to the uncontrolled version coming next.

The way controlled/uncontrolled state works in TextInput is:

  When controlled (and without this `setNativeProps`):
   * we set message in state
   * invisibly to us, RN calls setNativeProps to reconcile the
     difference between what value we want and what is the current
     value of the native component

  When uncontrolled:
   * we manually call setNativeProps
   * we also set state for `message`, but that is only so we track
     the current value of the input in JS-land

  In this commit:
   * we call setNativeProps with value X
   * we also set state to X
   * React Native invisibly to us checks if desired value differs
     from what the value of the native component is.  X === X so
     RN does not do anything.
    @borisyankov @gnprice
    borisyankov authored and gnprice committed Jun 30, 2018
    Configuration menu
    Copy the full SHA
    cbd180d View commit details
    Browse the repository at this point in the history

  3. compose: Make the message text input uncontrolled. 

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

Then do the following changes to reimplement previous behavior:
 * at the very start in `componentDidMount`, make sure we update
   the component with the initial values (coming from state)
 * ensure that autocomplete changes the input values
 * on editing a message, set the input text to the previous
   message contents
    @borisyankov @gnprice
    borisyankov authored and gnprice committed Jun 30, 2018
    Configuration menu
    Copy the full SHA
    8f643ff View commit details
    Browse the repository at this point in the history

  4. compose: Make topic input uncontrolled. 

    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.
    @borisyankov @gnprice
    borisyankov authored and gnprice committed Jun 30, 2018
    Configuration menu
    Copy the full SHA
    1f9ad56 View commit details
    Browse the repository at this point in the history