Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Disabling word wrap can leave a big blank gap #3206

Closed
TomMalbran opened this issue Mar 21, 2013 · 9 comments
Closed

Disabling word wrap can leave a big blank gap #3206

TomMalbran opened this issue Mar 21, 2013 · 9 comments

Comments

@TomMalbran
Copy link
Contributor

Problem:

  1. Open a file with long files. I tested with src/thirdparty/CodeMirror2/modes/sql/sql.js.
  2. Scroll to the end of the file.
    3.1. If word wrap is enabled, disable it.
    3.2. If word wrap is disabled, enable it, scroll back to the end of the file and disable it.
  3. Select the last line
  4. Re-enable word wrap

Result:
3. You only see lines 260-269, since those where the lines rendered before, leaving a big gap on the top. Scrolling solves this issue.
5. You can see all the text, but the cursor is not on the screen anymore. Not a big issue, but there seems to be a big scroll difference.

Expected:
3. No blank lines.
5. Scroll stays at the bottom of the screen.

@TomMalbran
Copy link
Contributor Author

I believe 3 can be easily fixing by doing a refresh on the current document.

@ghost ghost assigned RaymondLim Mar 22, 2013
@pthiess
Copy link
Contributor

pthiess commented Mar 22, 2013

Reviewed @RaymondLim

@RaymondLim
Copy link
Contributor

This has the same root cause as #3241. If I comment out the changes in codemirror/codemirror5@9338ba4, then the rendering issue is fixed, but not the scrolling issue after step 5 which I believe have to fix in our code.

@RaymondLim
Copy link
Contributor

The main issue in step 3 is fixed in CM, but I still need to fix the issue in step 5 in my code where we turn Word Wrap on.

@TomMalbran
Copy link
Contributor Author

I checked back into this and both issues seem fixed. The scroll doesn't move at all enabling and disabling the word wrap option. The cursor might get off the screen, but I think that should be expected if it wants to keep the scroll as before. It should be more important to keep the scroll as it was than the cursor in view.

@njx
Copy link

njx commented Mar 26, 2013

I had to roll back the CM SHA due to the latest upstream breaking our unit tests (see comment in #3241), so let's keep this open for now.

@njx
Copy link

njx commented Mar 28, 2013

FYI, the latest CM is merged again, so that aspect of this should be fixed now.

@TomMalbran
Copy link
Contributor Author

Yes, I see the problem fixed again.

@RaymondLim
Copy link
Contributor

Closing.

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

4 participants