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

autoScrollCursorOnSet={false} deprecation has no replacement #25

Closed
majg0 opened this issue Oct 18, 2017 · 6 comments · Fixed by neinteractiveliterature/intercode#711 · May be fixed by inhji/cthulhu-web#159
Closed

Comments

@majg0
Copy link

majg0 commented Oct 18, 2017

Using autoScrollCursorOnSet = false on version 2.0.2, my editor does not scroll new content into view.

I can't seem to find any combination of scroll, autoScroll and autoCursor (I tried all six of them) that works the same. I do not want to scroll on new content since it comes from a server which might update it at any time, and I'm there to read in the middle of a big JSON.

Please provide a fix, this is keeping me from upgrading.

By the way, was using the Controlled component version.

@scniro
Copy link
Owner

scniro commented Oct 18, 2017

Well, those props you had mentioned are suppose to be the replacement. Clearly something is not working as expected. I'll look into this shortly.

@majg0
Copy link
Author

majg0 commented Oct 18, 2017

Thanks a lot! Kudos to your profile pic too btw.

@scniro
Copy link
Owner

scniro commented Oct 18, 2017

@martingronlund Hey again. So I want to clarify just a little more. Were you using scroll before? scroll accepts a position passed in which codemirror will be instructed to scroll to that point.

Have you tried both autoCursor={false} and autoScroll={false}. When setting these, I get the following (internally)

editor.getCursor()

{line: 0, ch: 0, sticky: null}

editor.getScrollInfo()

{left: 0, top: 0, [...]

Looking at the changes now, resetCursorOnSet={false} and autoScrollCursorOnSet={false} should be the programmatic equivalent of autoCursor={false} and autoScroll={false}. I could certainly be overlooking something. Please share some more detail if you can.

If I can reproduce this I'll fix it asap.

@majg0
Copy link
Author

majg0 commented Oct 18, 2017

Hey, thanks for getting back so quickly.

No, was not using scroll before.
I did test with autoCursor={false} and autoScroll={false} (with and without scroll={false}).

I tested it by having a JSON of ~300 lines.

  • Let's say initially, lines 0-100 are in view.
  • Scroll down to put cursor on 200.
  • Scroll out of view of cursor but not to top, e.g. to 50-150.
  • Now provide a new external value. 2.0.2 would stay put, 3.0.0 will scroll to top.

Hope it helps!

scniro added a commit that referenced this issue Oct 18, 2017
@scniro
Copy link
Owner

scniro commented Oct 18, 2017

@martingronlund I think I have found the problem and have published up a 3.0.1 release to npm. You should be able to simply specify autoScroll={false} (don't worry about autoCursor or scroll)

This appears to be the same pitfall discovered in #14. When refactoring this I overlooked that one and reverted back to setValue.

Let me know if this looks better?

@majg0
Copy link
Author

majg0 commented Oct 18, 2017

Works! Great job!

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