CodeMirror scrolls current line to the bottom of the viewport on change

[meyer]

Here’s a GIF:

CodeMirror’s value comes from parent component state, and the handleChange method updates parent component state.

[scniro]

@meyer Hey thanks for opening this up as well as the awesome demo of exactly what's happening. Unfortunately, I am unable to reproduce this on my end. Are you using any other scripts that may be affecting this component?

If you can spin up a quick isolated repo with this happening I'll gladly clone it down and get it fixed in a timely manner. Thanks!

[meyer]

@scniro thanks for the quick reply. I wasn’t able to pin this issue down and it was affecting customers, so I took react-codemirror (what we were using before) and added additional events manually. When I’ve got some time I’ll come back to this and try to get a repro for you.

In toggling between react-codemirror and react-codemirror2, I noticed that setValue was being called twice in react-codemirror2. I think that might have something to do with the issue I was experiencing. It’s only being called once in react-codemirror.

[scniro]

@meyer cool just hit me up when you get it reproducing! It's looking like react-codemirror is abandoned so if there's anyway I can help others migrate I'm totally game. I'll leave this open for as long as you need and I'll touch base again down the road if I don't hear back beforehand so we don't forget about it.

[andi0b]

Same problem here. @meyer: Thanks for the great GIF

I'm using redux, onChange is hooked to an update to the store. And this is followed by an immediate re-render. value is set to the value from the store. If I remove the binding from the store, everything works.

So I guess the problem happens, if you feed the string from onChange back to value.

[scniro]

@andi0b thanks for chiming in on this. However, that's the point of redux, right? If I remove that, everything totally beaks as far as setting the value. As soon as I can get a working repro I'll fix whatever issue is bothering you both 👍

[scniro]

@andi0b hey wanted to follow up, I thought this over again and I see what you're saying now. I did an experiment on my end where I bound value to local state and setState to it in the onChange cb as such...

class Editor extends React.Component {

  constructor(props) {
    super(props);

    this.state = {
      test: 'foo'
    }
  }

  render() {

    return (
      <CodeMirror
        value={this.state.test}
        onChange={(editor, metadata, value) => {

          this.setState({test: value}) // why feed this back to `value` here?
        }}
      />
    )
  }
}

So, I got it reproducing but I'd like to know more about what you are trying to do here. Codemirror has a value/state thats kept internally and the internal value is pumped out.

So I guess the problem happens, if you feed the string from onChange back to value

^totally true, and this is also causing onSet to be invoked again as well. I suspect I can find a fix for this, but I want to pinpoint the exact use case here just so we're not chasing something that's not an ideal implementation anyways.

[justinwilson-apollidon]

Yeah this is definitely a bug. I am getting this as well except mine jumps around on the page on changes as you type. Honestly not sure how to show a demo of this as it is jumping above the textarea portion where you can't even see what you're typing, as you type.

[scniro]

@justinwilson-apollidon what are you doing inside of onChange? I'm trying to figure out why users are trying to do this. The editor will persist the changes, why pass it back to value?

[justinwilson-apollidon]

@scniro So my usage is slightly different than others where I have an array of "sections" that allows the user to Add and Delete sections that include two fields, Title and HTML fields. Then this is stored through an API (besides the point or issue with this editor).

That being said, I used this for the HTML field in particular and I have a this.state.sections.map() that iterates those two fields and component.

onChange simply updates the object within sections: [], in state to save later when the form is submitted. I suppose my use case was slightly different than most as upon digging deeper into the source of your CodeMirror component, I noticed it may be something with refs.

I'm not entirely sure honestly but I ended up switching over to Ace for now as it provides the functionality we require anyway. That being said, I'd be more than happy to help with this project in my personal spare time.

[andi0b]

So I guess the problem happens, if you feed the string from onChange back to value

totally true, and this is also causing onSet to be invoked again as well. I suspect I can find a fix for this, but I want to pinpoint the exact use case here just so we're not chasing something that's not an ideal implementation anyways.

@scniro: I'm not an experienced React developer, but as I understood it, it's a common pattern to keep a components content inside the state of the parent. And refresh the state if the component invokes changed events. This triggers unnecessary refreshes. But that happens all the time with React.

[scniro]

@andi0b @justinwilson-apollidon @meyer thank you all so much for the collaboration. I'm happy to share that I believe I have a fix published up to 2.0.2.

Basically, I was setting the value to the inner codemirror instance via setValue. Made sense to me at the time, but I'm glad I came across this issue: programaticaly adding content makes scrollbars buggy

.focus() does not put the cursor at 0,0. setValue does, but it does not cause the editor to lose focus. If you're putting in completely new content, the old scroll position doesn't mean anything anymore. If you're just making a minor change, don't use setValue, but make the precise change with replaceRange instead.

So, out with setValue and in with replaceRange. All appears to be working just as before, however there should not be any weird resetting of the scroll/viewport. Can you please try out the new version and share with me if you believe it to be fixed?

[scniro]

Closing for now - please reopen should this still be an issue or you can verify this is not fixed in the release referenced above,