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

Error because this.cursorPos is undefined #3

Closed
stiang opened this issue Jul 21, 2017 · 3 comments
Closed

Error because this.cursorPos is undefined #3

stiang opened this issue Jul 21, 2017 · 3 comments

Comments

@stiang
Copy link

stiang commented Jul 21, 2017

First of all - thanks for stepping up now that react-codemirror appears to be unmaintained!

I get an error when I include a <CodeMirror> element. The reason appears to be that this.cursorPos is undefined, but is still being used as an argument for this.editor.setCursor().

Looking at the source, it will indeed be undefined if the first outer if statement doesn’t trigger, but the second outer does:

componentWillReceiveProps(nextProps) {

    if (this.props.value !== nextProps.value) {
      this.hydrated = false;

      if (!this.props.resetCursorOnSet) {
        this.cursorPos = this.editor.getCursor();
      }
    }

    this.hydrate(nextProps);

    if (!this.props.resetCursorOnSet) {
      this.editor.setCursor(this.cursorPos);
    }
  }

I was able to solve this by initializing this.cursorPos in componentDidMount:

  this.cursorPos = {line: 0, ch: 0};

At this point the library appears to work, but I encountered another problem (the page scrolls down whenever the prop that controls the editor value is changed, on a page with multiple editors), but I’ll file that as a separate issue if it turns out to be related to the library.

scniro added a commit that referenced this issue Jul 21, 2017
@scniro
Copy link
Owner

scniro commented Jul 21, 2017

@stiang thank you for opening this up! Yup, this indeed appears to be a blatant bug and I have removed the initial check for !this.props.resetCursorOnSet outside of the conditional it was in. 0.0.10 should be published up via npm and hopefully resolves your issue. When you get some time, could you pull it down and report your findings? Thanks!

@stiang
Copy link
Author

stiang commented Jul 23, 2017

Thanks, that worked!

I haven’t had time to look deeply at the page scrolling issue yet, but it was still there with 0.0.10, at least. Have you tried to have multiple <CodeMirror>s on a page (so many that at least one of them is below the fold), and then changing the props which their values come from? If so, did the page scroll for you too or is it just here?

@scniro
Copy link
Owner

scniro commented Jul 23, 2017

@stiang thanks for the feedback and sharing more details on this secondary issue! I indeed get the page jump you describe when I have enough instances on the page. This side effect seems to have been introduced from the fix I implemented for #1 - specifically, preserving and setting the cursor position. The page is jumping down because it's trying to set the cursor for the last instance, even if untouched and is internally at 0,0.

Without a change, there is a workaround currently but I'm not thrilled with it. If you specify resetCursorOnSet={true} - you'll be okay and the page won't jump. However, you'll not be getting a cursor position saved/set for your editor. There may be a better way to address this now that both sides of the above mentioned fix have surfaced. In the least, I'll be renaming this prop soon (I don't think it's very descriptive). Also please give #1 a look for some background on the issue should my suggestion not make total sense here.

However, let me know if the cursor position is something you can't be without and I'll try my best to overhaul a solution for everyone. Thanks again for your help on these!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants