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.

Sign up for GitHub

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

Jump to bottom

ReadOnly TextView's broken scrolling after version update #2688

Closed
chr1st0scli opened this issue May 30, 2023 · 5 comments · Fixed by #2752
Closed

ReadOnly TextView's broken scrolling after version update #2688

chr1st0scli opened this issue May 30, 2023 · 5 comments · Fixed by #2752
Labels
bug

Comments

@chr1st0scli
Copy link

Describe the bug
Another breaking change I came across when upgrading from v1.9.0 to 1.12.1, is that I cannot scroll my read-only TextView correctly any more.

To Reproduce
Steps to reproduce the behavior:

  1. Add a quite short TextView, so that you don't have to insert a lot of text to reproduce the error.
  2. Set it's ReadOnly property to true and its Text property to several lines, such that they exceed its height.
  3. Scroll down to the end and then try to scroll up again, using the keyboard's up and down arrows.
  4. Notice that you cannot scroll back to the top. If you type a character or use the mouse wheel, it's as if it gets unstuck for a while and then you might be able to scroll to the top.
  5. Also, notice that it's sluggish, the cursor might move a little right or left when changing lines and get stuck "randomly".

Expected behavior
To scroll seamlessly, like I used to.

Screenshots
Here, I demonstrate the different behavior between the two versions. The read-only is the bottom one, in the "output" frame.

ReadOnlyTextViewScrollingProblem

Desktop (please complete the following information):

  • OS: Windows
  • Version 10
@tig tig added the bug label May 30, 2023
@tig
Copy link
Collaborator

tig commented May 30, 2023

Thank you for reporting this. Really appreciate you providing repro instructions.

BDisp added a commit to BDisp/Terminal.Gui that referenced this issue May 30, 2023
@BDisp
Fixes gui-cs#2688. ReadOnly TextView's broken scrolling after version…
90132d9 
… update.
BDisp added a commit to BDisp/Terminal.Gui that referenced this issue May 30, 2023
@BDisp
Fixes gui-cs#2688. ReadOnly TextView's broken scrolling after version…
25df55f 
… update.
@BDisp
Copy link
Collaborator

BDisp commented May 30, 2023

This is also happening in the v2.

@BDisp
Copy link
Collaborator

BDisp commented May 30, 2023

Only to clarify that this bug is also happen if ReadOnly is false.

@tig tig closed this as completed in 58ad65f Jun 7, 2023
tig pushed a commit that referenced this issue Jul 6, 2023
@BDisp @tznind
Fixes #2680. Make the TextView API more extensible. (#2682)
325180a 
* Fixes #2680. Make the TextView API more extensible.

* Remove unnecessary using.

* Add GetLine method.

* Change RuneCell Attribute property to ColorScheme property.

* Add LoadRuneCells method and unit test.

* Add helper method to set all the Colors.ColorSchemes with the same attribute.

* Change RuneCell to class.

* Add IEquatable<RuneCell> interface.

* Fix unit test.

* Still fixing unit test.

* Fixes #2688. ReadOnly TextView's broken scrolling after version update.

* keyModifiers must be reset after key up was been processed.

* Trying fix server unit test error.

* Prevents throw an exception if RuneCell is null.

* Still trying fix this unit test.

* Cleaning code.

* Fix when the RuneCell is null.

* Fix throwing an exception if current column position is greater than the line length.

* Fixes #2689. Autocomplete doesn't popup after typing the first character.

* Fix Used on TextField.

* Always use the original ColorScheme if RuneCell.ColorScheme is null.

* Fix Used on TextView.

* Add RuneCellEventArgs and draw colors events.

* Add two more samples to the scenario.

* Fix a bug which was causing unit tests with ColorScheme fail.

* Fix a issue when WordWrap is true by always loading the old text.

* Improves debugging in RuneCell.

* WordWrap is now preserving the ColorScheme of the unwrapped lines.

* Simplifying unit test.

* Ensures the foreground and background colors are never the same if Used is false.

* Remove nullable from the parameter.

* Merge syntax highlighting of quotes and keywords together

* Add IdxRow property into the RuneCellEventArgs.

* Fix pos calculation on windows
(where newline in Text is \r\n not \n)

* Fix events not being cleared when toggling samples.

* Change Undo and Redo to a public method.

* Changes some methods names to be more explicit.

* Call OnContentsChanged on needed methods and fix some more bugs.

* Adds InheritsPreviousColorScheme to allow LoadRuneCells uses personalized color schemes.

* Serializes and deserializes RuneCell to a .rce extension file.

* Prevents throwing if column is bigger than the line.

* Avoids create a color attribute without one of the foreground or background values. In Linux using -1 throws an exception.

* Replace SetAllAttributesBasedOn method with a ColorScheme constructor.

* Move RuneCell string extensions to TextView.cs

* Reverted parameter name from cell to rune.

* Change Row to UnwrappedPosition which provide the real unwrapped text position within the Col.

* Add brackets to Undo and Redo methods.

* Replace all the LoadXXX with Load and rely on the param type to differentiate.

* Open a file inside a using.

* Proves that the events run twice for WordWrap disabled and the enabled.

* Remove GetColumns extension for RuneCell.

* Add braces to Undo an Redo.

* Change comment.

* Add braces.

* Delete remarks tag.

* Explaining used color and ProcessInheritsPreviousColorScheme.

* Fix comment.

* Created a RuneCellTests.cs file.

* Rename to StringToLinesOfRuneCells.

* Make ToRuneCells private.

---------

Co-authored-by: Thomas <tznind@dundee.ac.uk>
Co-authored-by: Thomas Nind <31306100+tznind@users.noreply.github.com>
@chr1st0scli
Copy link
Author

Hi @tig and @BDisp

I was anxiously waiting for the next release, i.e. 1.13.0 for this bug fix which is now closed. In my understanding, related changes were merged only in v2_develop. Isn't there going to be a v1 patch for this?

Especially if you consider that there was a v1 version that did not have this problem, which was introduced by a later v1 version.

Thank you so much.

@tig
Copy link
Collaborator

tig commented Jul 18, 2023

If someone can do a PR for this into develop I'll grab it and release v1.13.2 asap!

@tig tig reopened this Jul 18, 2023
BDisp added a commit to BDisp/Terminal.Gui that referenced this issue Jul 18, 2023
@BDisp
Fixes gui-cs#2688. ReadOnly TextView's broken scrolling after version…
9e2e8e1 
… update.
@BDisp BDisp mentioned this issue Jul 18, 2023
Merged
8 tasks
@tig tig closed this as completed in #2752 Jul 18, 2023
tig pushed a commit that referenced this issue Jul 18, 2023
@BDisp @tig
Fixes #2688. ReadOnly TextView's broken scrolling after version update.
cd119ea
@chr1st0scli chr1st0scli mentioned this issue Jul 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #2688. ReadOnly TextView's broken scrolling after version update. BDisp/Terminal.Gui
3 participants
@tig @chr1st0scli @BDisp