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

Cursor jumps to end of controlled input #955

Closed
ericvicenti opened this issue Jan 23, 2014 · 52 comments
Closed

Cursor jumps to end of controlled input #955

ericvicenti opened this issue Jan 23, 2014 · 52 comments

Comments

@ericvicenti
Copy link

When an input element is "controlled" by a model, the cursor will jump to the end of the line on every change. This makes it impossible to edit text that is not at the end of the input.

A quick demo: https://gist.github.com/ericvicenti/46f97f47c1cfe46040c8

      var ExampleApplication = React.createClass({
        render: function() {
          var model = this.props.model;
          return <input onChange={this.nameChange} value={model.name} />;
        },
        nameChange: function(evt) {
          this.props.model.name = evt.target.value;
        }
      });
      var myModel = {
        name: 'Input is funky'
      };
      setInterval(function() {
        React.renderComponent(
          <ExampleApplication model={myModel} />,
          document.getElementById('container')
        );
      }, 50);

It should be noted that this is only when using an external model, not when using the view's state. Maybe there is something wrong with my usage here?

As a suggested fix, maybe the input should not be overridden unless the value differs? Otherwise, the cursor position should be manually preserved.

Also, see this SO entry which documents the ability to grab and preserve the cursor position: http://stackoverflow.com/questions/1080532/prevent-default-behavior-in-text-input-while-pressing-arrow-up

@sophiebits
Copy link
Collaborator

Your problem is that you're not rerendering with the new value. If you have React rerender with the new input value, then it'll know not to revert the value. (As it is, it will revert to the old value until it gets the new value after 50 ms. Changing 50 to a larger number like 1000 will make this much more apparent.) The following code should work:

var ExampleApplication = React.createClass({
  render: function() {
    var model = this.props.model;
    return <input onChange={this.nameChange} value={model.name} />;
  },
  nameChange: function(evt) {
    this.props.model.name = evt.target.value;
    rerender();
  }
});
var myModel = {
  name: 'Input is funky'
};
function rerender() {
  React.renderComponent(
    <ExampleApplication model={myModel} />,
    document.getElementById('container')
  );
}
setInterval(rerender, 50);

(The normal way to do this when making a reusable component is to pass down a callback in props to ask the parent component to rerender.)

@ericvicenti
Copy link
Author

Ah, thanks for the help!

I'm not a big fan of passing a callback down, so I'm going to try and re-render from the parent immediately on model change events.

@sophiebits
Copy link
Collaborator

Great -- that should work too.

meiersi added a commit to meiersi/ghcjs-vdom that referenced this issue Aug 27, 2014
@marnusw
Copy link

marnusw commented May 14, 2015

I came across this problem when writing a Flux application. I updated the local state and dispatched an action to update the store. This local state update caused a re-render which still used the current "old" value in the store and thus changed the DOM and moved the cursor.

This Stack Overflow answer helped me understand what was going on, so I thought I'd reference it here since this was the first result I found on Google, but didn't quite help me understand what was going on.

@tsheaff
Copy link

tsheaff commented Oct 23, 2015

@spicyj I'm seeing a similar issue of the cursor jumping to the end whenever I format / modify a controlled input's value between renders.

Here's my code:

export default class GiftPurchaseForm extends Component {
  constructor(props) {
    super(props);
    this.state = {
      cardNumber: '',
    };
  }

  cardNumberChanged(event) {
    this.state.cardNumber = event.target.value;
    this.setState(this.state);
  }

  render() {
    return (
      <div id="gift-purchase-form">
        <input
          className="card-number"
          type="text"
          placeholder="Card Number"
          value={creditcard.parse(this.state.cardNumber).formatted}
          onChange={::this.cardNumberChanged}
        />
      </div>
    );
  }
}

creditcard.parse(*).formatted simply turns a number like 4444555566667777 into a card number like 4444 5555 6666 7777

If I remove call to creditcard.parse(*).formatted and just pass this.state.cardNumber the cursor doesn't jump. But the cursor still jumps even if I pass this.state.cardNumber + '5'; as the next value, so it seems any modification of the string between renders means middle edits (edits before modification point in string) cause cursor to jump to end.

I'm on react@0.14.0

@export-mike
Copy link

@tsheaff did you find a solution to this in the end? I'm also doing some onChange formatting to my text field.

@sophiebits
Copy link
Collaborator

@tsheaff @mikeljames The problem is that React doesn't have enough information to do something intelligent. Assuming ^ represents the cursor, suppose the input looks like

401^8 8888 8881 881

(that is, between "1" and "8" in "4018"). Then I type a "2". Momentarily, the input looks like

4012^8 8888 8881 881

but immediately, creditcard.parse(*).formatted returns

4012 8888 8888 1881

Where should the cursor go? Even as a human looking at this, it's unclear: it could go before or after the space:

4012^ 8888 8888 1881
4012 ^8888 8888 1881

Figuring this out programmatically seems impossible to me in the general case. Even if we were content to return either of those, it's hard for me to imagine an algorithm that might work reliably. Let me know if I'm missing something.

Because this is impossible and requires more knowledge about the specific problem space, React doesn't attempt to do anything intelligent with the cursor; you can set .selectionStart/.selectionEnd manually as appropriate for your domain. In the case of filtering out some characters it would be possible to write a more general solution but I think that may still be better left to a third-party component.

@tsheaff
Copy link

tsheaff commented Nov 30, 2015

Yes @mikeljames it's impossible in general as @spicyj explains.

You can almost certainly do better than jumping to the end for your specific case of formatting using some simple heuristic. Doesn't make sense for React to burn extra cycles attempting to do this in general (e.g. formatting could be additive or subtractive).

Perhaps linking this issue or a related discussion could be helpful in the official docs as I'd imagine formatting (phone numbers, credit cards, SSNs) is quite common.

@sophiebits
Copy link
Collaborator

Actually: Stripe's jQuery.payment library preserves cursor position except if your cursor is already at the end, in which case it keeps your cursor at the end of the input. This generally feels pretty reasonable; try:

http://stripe.github.io/jquery.payment/example/

That example flickers but that's not inherent to the strategy so we could do better. This might be more predictable in the common case, at the expense of introducing edge-case bugs because you now don't need to think about what should happen. For example, this strategy does feel broken in the case that you're typing before a space. If you have:

4018^ 8888 8881 881

and type a 7 you have

40187^ 8888 8881 881

which gets changed to

4018 ^7888 8888 1881

which feels wrong because your cursor should be after the 7, not before. But maybe this behavior is still better.

cc @zpao for a second opinion.

@sophiebits sophiebits reopened this Dec 1, 2015
@sophiebits
Copy link
Collaborator

That is: I'm entertaining the idea of React's <input> doing this by default.

@sophiebits
Copy link
Collaborator

Wait: this wouldn't work at all for filtering out chars because your cursor would move over whenever you type an invalid character. Maintaining the distance from the end might work though?

@sophiebits
Copy link
Collaborator

Here you can try it:

http://jsbin.com/dunutajuqo/edit?js,output

The numbers input works great. For the CC input: If you type "1234 5678" and then try to type a character after the space, your cursor is in the wrong place. Similarly, typing "1234 567" and then typing a character before the space does the wrong thing.

@tsheaff
Copy link

tsheaff commented Dec 1, 2015

Yes it's possible to do this well if React knows more about the format than simply what the value was and what it has changed to.

Could borrow heavily from jQuery's mask plugin which is pretty great

@sophiebits
Copy link
Collaborator

@tsheaff That's a good opportunity for a third-party plugin but we won't put it in the core. Maybe a "React mask component". :)

@tsheaff
Copy link

tsheaff commented Dec 1, 2015

Where do you draw the line on something like this? I agree that fully including/re-building jQuery Mask feels wrong & bloated, but it's unclear if there's a middle ground here that solves a meaningful part of the problem without introducing more unintuitive bugs.

@syranide
Copy link
Contributor

syranide commented Dec 1, 2015

@tsheaff If you ask me, having an input that also accepts a selection (and reports it on change), but browsers being the way they are and events in React being reported as-is, this seems kind of fragile and quirky for core behavior... and the same thing can be implemented as a third-party component, so that seems preferable at least given the circumstances.

@joshma
Copy link
Contributor

joshma commented Jan 27, 2016

Any chance core can support the case where the input does not change and there's no selection? I think that's a pretty common edge case - user types an invalid character, so I reject the change. Currently this results in the cursor jumping to the end, when instead it'd be nice for the cursor to not move.

@sophiebits
Copy link
Collaborator

My last example posted on Nov 30 keeps the cursor in the same place if you reject a change, at least.

@thedamon
Copy link

thedamon commented Feb 9, 2016

This is an excellent example... almost feel like it should be part of the main documentation. This was a weird issue to try and nail down in a controlled form with an attempted mask!

bartkusa added a commit to bartkusa/Ex3eTracker that referenced this issue Mar 5, 2016
Working around facebook/react#955 by forcing a
synchronous rerender on each keystroke, instead of letting
PersistentCharacterStore defer the publication of its state change.
@export-mike
Copy link

Just wondering if there there is a react mask component people can recommend for this?

@Guria
Copy link

Guria commented May 9, 2016

@spicyj
Just want to note a bug in http://jsbin.com/dunutajuqo/edit?js,output
It's impossible to erase char with backspace in credit card input if cursor is not at the end and right after space.

@sophiebits
Copy link
Collaborator

@Guria Thanks for pointing that out.

@danielrob
Copy link

danielrob commented May 8, 2018

@sophiebits I've posted @tibbus's request as a first class issue. Incidentally, all roads seem to lead back to this issue when searching in this area, and this issue seems to point to your comment on November 30th which no longer exists.

While it seems understandable React cannot predict this case, and so simply places the cursor at the end, it would be ideal to have a best practice to overcome this when it is desired (to change the value after an onChange). I have tried many of the solutions posted above to no avail, perhaps due to the changes that react 16.3 brings.

I've also found that workarounds of setting the cursor position manually conflict with React, perhaps my renders are too long, but even resetting the cursor position on the next tick causes a noticeable cursor jump.

sindresorhus added a commit to atomiclabs/hyperdex that referenced this issue May 24, 2018
Since our state updates are async, it creates a race condition where React cannot correctly diff the updates and it results in the cursor being pushed to the end.

See this React issue: facebook/react#955 In short, just another React annoyance.

Fixes #237
sindresorhus added a commit to atomiclabs/hyperdex that referenced this issue May 25, 2018
…307)

Since our state updates are async, it creates a race condition where React cannot correctly diff the updates and it results in the cursor being pushed to the end.

See this React issue: facebook/react#955 In short, just another React annoyance.

Fixes #237
@amirouche
Copy link

It's slowing down development time 👎

@amirouche
Copy link

I will try ant design kit to see if the issue is present.

@nerdo
Copy link

nerdo commented Aug 8, 2018

I don't know if this is still relevant, but I was able to solve this issue by just keeping track of the carat position and updating it when necessary.

Note: This may or may not compile as-is. I pasted it from a more complex Component and whittled it down to the [mostly] bare essentials to show how it works.

import React from 'react';

class Input extends React.Component {
    static defaultProps = {
        shouldCommitValue: value => true
    }

    constructor(props) {
        super(props);

        this.inputRef = React.createRef();

        this.state = {
            value: '',
            inputSelection: false,
            updateSelection: false
        };
    }

    render() {
        const otherProps = {shouldCommitValue, onFocus, onKeyDown, onChange, ...otherProps} = this.props;
        this.onFocus = onFocus;
        this.onChange = onChange;
        this.onKeyDown = onKeyDown;

        return (
            <input
                ref={this.inputRef}
                value={this.state.value}
                onFocus={this.handleInputFocus}
                onKeyDown={this.handleInputKeyDown}
                onChange={this.handleInputChange}
                {...otherProps}
            />
        );
    }

    handleInputFocus = (...args) => {
        // Save the initial cursor position when the user focuses the input.
        this.saveCursorPosition();

        if (this.onFocus instanceof Function) {
            this.onFocus(...args);
        }
    }

    handleInputKeyDown = (...args) => {
        // Save the updated cursor position as the user interacts with the input.
        this.saveCursorPosition();

        if (this.onFocus instanceof Function) {
            this.onKeyDown(...args);
        }
    }

    handleInputChange = (event, ...otherArgs) => {
        const value = event.target.value;

        if (this.props.shouldCommitValue(value)) {
            // The value should be committed, business as usual...
            this.setState({ value });
        } else {
            // The value was ignored (or altered).
            // Since DOM input element's input state and the value used during the re-render will be different,
            // the input field will be out of sync and the result is the cursor will jump to the end.
            // ...so signal to componentDidUpdate() that we want to update the selection afterwards!
            this.setState({
                updateSelection: { ...this.state.inputSelection }
            });
        }

        if (this.onChange instanceof Function) {
            this.onChange(event, ...otherArgs);
        }
    }

    componentDidUpdate() {
        // If there was a request to update the selection via setState...
        if (this.state.updateSelection) {
            // Update the selection.
            const selection = this.state.updateSelection;
            this.inputRef.current.selectionStart = selection.start;
            this.inputRef.current.selectionEnd = selection.end;

            // Important! Clear out the update request, otherwise you will end up with an infinite loop.
            this.setState({updateSelection: false});
        }
    }

    saveCursorPosition = () => {
        this.setState({
            inputSelection: {
                start: this.inputRef.current.selectionStart,
                end: this.inputRef.current.selectionEnd
            }
        });
    }
}

This is an example of just disallowing certain types of strings to bet set as the value. For example, to limit the input to valid CSS selectors:

<Input
    type="text" 
    name="cssClass"
    shouldCommitValue={value => value.match(/^[a-zA-Z0-9_\-]*$/)}
/>

@juan-ee
Copy link

juan-ee commented Aug 16, 2018

What do you think about this guys ?
I realized that It can be managed by using React Refs but it is necessary to use a Statefull Component because you need to use the callback function after have been updated the state.

class App extends Component {

    constructor(props){
        super(props);
        this.state = {
            value: ""
        };

    }

    change1 = (newValue) =>{
        this.setState({value:newValue});
    };

    render(){
        return (<CustomInput change1={this.change1} value={this.state.value}/>)
    }
}

class CustomInput extends React.Component {
    constructor(props){
        super(props);
        this.state = {
            cursor: 0
        };

        this.textInput = React.createRef();
    }
    componentDidUpdate() {
        if (this.textInput.current !== null)
            this.textInput.current.selectionStart = this.textInput.current.selectionEnd = this.state.cursor;
    }

    change2 = (event) =>{
        let cursor = event.target.selectionStart;
        let value = event.target.value;
        if (/^[ A-Za-z]*$/.test(value)) {
            value = value.toUpperCase();
        }else{
            value = this.props.value;
            cursor -= 1;
        }
        this.setState({cursor:cursor},()=>{
            if (value !== this.props.value) this.props.change1(value);
        });
    };

    render(){
       return <input ref={this.textInput} onChange={this.change2} value={this.props.value}/>;
    }
}

@paul-cheung
Copy link

@nerdo thanks, it helped.

@Aprillion
Copy link

Aprillion commented Oct 1, 2018

for me, cursor jumping started when I refactored from componentWillReceiveProps to
static getDerivedStateFromProps, but does NOT happen with getDerivedStateFromProps that isn't static

could it be a babel/webpack problem?

(reproducible with React 16.4, react-autosuggest 9.4.2, react-scripts 1.1.5)

edit: sorry, without static it just doesn't do anything so the cursor won't jump but it's not a workaround, I will keep using good old (== working) componentWillReceiveProps until there is something better that works.

@ahce
Copy link

ahce commented Oct 21, 2018

Hi! this is my solution https://codesandbox.io/s/9208ol4r8y

@emkayy
Copy link

emkayy commented Nov 1, 2018

I don't see why you would use state here. This is not data that should cause your component to re-render. You just want to update the selection on your ref.

I implemented it like this:

  constructor() {

    super();
    this.inputRef = React.createRef();
    this.selection = {
      start: false,
      end: false
    };

    this.handleChange = this.handleChange.bind(this);
  }

  componentDidUpdate() {

    const { selectionStart, selectionEnd } = this.inputRef.current;
    const update = (this.selection.start !== false && this.selection.start !== selectionStart)
      || (this.selection.end !== false && this.selection.end !== selectionEnd);

    if (update) {
      this.inputRef.current.selectionStart = this.selection.start;
      this.inputRef.current.selectionEnd = this.selection.end;
    }
  }

  handleChange(event) {

    const input = this.inputRef.current;
    this.selection = {
      start: input.selectionStart,
      end: input.selectionEnd
    };
    this.props.onChange(event);
  }

Quick and simple. Works perfectly fine for me.

@binomialstew
Copy link

Does anyone have any ideas for dealing with this on an input with type="email"? The problem is selectionStart, selectionEnd and setSelectionRange are not available for this input type. I thought about changing type to text before getting and updating the selection, but this isn't really practical because it would have to run through multiple renders to change the type and then get the selection range. Even then, it's not really feasible because the type needs to be changed back to email and would require another setState in componentDidUpdate, which is ugly. I even wonder if this will cause problems with iOS keyboard types due to the switch between email and text while the user is typing.

@binomialstew
Copy link

For anyone else looking for a solution, instead of changing type only during setSelection, I ended up switching to a text type input on focus and back to original input type on blur. I don't think this is an ideal solution, but it works, and I don't see any major problems with it. I would be happy to be able to remove these parts of my component if this issue is ever addressed because it adds a lot of complexity.

@dhiraj1site
Copy link

If anyone is still facing this problem, the easiest way is to preserve the cursor position before updating state and use the cursor position AFTER updating the state (not through callback)

onChange={(event) => {
  const caretStart = event.target.selectionStart;
  const caretEnd = event.target.selectionEnd;
  // update the state and reset the caret
  this.updateState();
  event.target.setSelectionRange(caretStart, caretEnd);
}}

@ahuglajbclajep
Copy link

ahuglajbclajep commented Jan 14, 2019

Probably the smallest complete example with TypeScript:

import * as React from "react";
import * as ReactDOM from "react-dom";

class App extends React.Component<{}, { text: string }> {
  private textarea: React.RefObject<HTMLTextAreaElement>;
  constructor(props) {
    super(props);
    this.state = { text: "" };
    this.textarea = React.createRef();
  }

  handleChange(e: React.ChangeEvent<HTMLTextAreaElement>) {
    const cursor = e.target.selectionStart;
    this.setState({ text: e.target.value }, () => {
      if (this.textarea.current != null)
        this.textarea.current.selectionEnd = cursor;
    });
  }

  render() {
    return (
      <textarea
        ref={this.textarea}
        value={this.state.text}
        onChange={this.handleChange.bind(this)}
      />
    );
  }
}

ReactDOM.render(<App />, document.getElementById("root"));

@taniarascia
Copy link

@dhiraj1site I had to add event.persist() for the code to work.

onChange={(event) => {
  event.persist()
  const caretStart = event.target.selectionStart;
  const caretEnd = event.target.selectionEnd;
  // update the state and reset the caret
  this.updateState();
  event.target.setSelectionRange(caretStart, caretEnd);
}}

@gaearon
Copy link
Collaborator

gaearon commented Mar 4, 2019

I'm going to lock because a lot of new solutions in this thread look suspicious and likely point to other misunderstandings or bugs.

The canonical solution is to make sure you're calling setState with e.target.value during the onChange event. That should be enough to preserve the cursor.

If it's not enough for you, please file a new issue with your reproducing case.

@facebook facebook locked as resolved and limited conversation to collaborators Mar 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests