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

🐞 Scrollbar twitches around when scrolling large file #184

Closed
Eliulm opened this issue Apr 30, 2023 · 4 comments · Fixed by #211
Closed

🐞 Scrollbar twitches around when scrolling large file #184

Eliulm opened this issue Apr 30, 2023 · 4 comments · Fixed by #211
Labels
bug Something isn't working

Comments

@Eliulm
Copy link
Contributor

Eliulm commented Apr 30, 2023

Description

When scrolling large files, the scrollbar twitches around the place. Only if the whole document has been completely viewed, the scrollbar will behave as expected.

Upon further investigation, I found that the textview.frame.height property changes a lot during scrolling. It might be the reason, why the scrollbar behaves weirdly.

To Reproduce

  1. Open file with content that does not fit in viewport.
  2. Scroll

Expected Behavior

Scrollbar animation should be smooth and self.textView.frame.height should stay constant during scrolling.

Version Information

CodeEditTextView: 0.6.2
macOS: 13.3.1
Xcode: 14.3

Additional Context

No response

Screenshots

Screen.Recording.2023-04-30.at.20.25.07.mov
@Eliulm Eliulm added the bug Something isn't working label Apr 30, 2023
@Eliulm Eliulm mentioned this issue Apr 30, 2023
@austincondiff
Copy link
Collaborator

austincondiff commented Apr 30, 2023

Not sure if this is related or not, but when the scroll reaches line 100, things shift to the right to account for the additional digit. We should account for maximum ruler width upon file open, not on scroll.

It seems like we do this for wrapped lines as well. My theory is that we assume all lines do not wrap. When we scroll to wrapped lines, this adds additional height to the overall scroll height and will adjust the scroll indicator to account for this additional height (again this is just a theory).

After watching your video, it seems like the indicator gets bigger then smaller then bigger then smaller, etc. I believe this is because the wrapped lines are scrolling out of view so it is again assumed that they will be the unwrapped height.

Edit: I can validate this theory by scrolling an wide window with no wrapped lines and a narrower window with wrapped lines.

Large window with no wrapped lines:

Screen.Recording.2023-04-30.at.2.04.27.PM.mp4

Smaller window with wrapped lines:

Screen.Recording.2023-04-30.at.2.04.42.PM.mp4

Notice the scroll indicator in the larger window with no wrapped lines remains consistently sized as I scroll down the page while the scroll indicator in the smaller window with wrapped lines does not.

@Eliulm
Copy link
Contributor Author

Eliulm commented Apr 30, 2023

Interesting. However, I just tried this too, but I still get the twitching in a file without line wraps:

Screen.Recording.2023-04-30.at.21.31.09.mov

@thecoolwinter
Copy link
Collaborator

There's two issues happening here:

  • The issue brought up by @Eliulm is due to the text view not correctly estimating it's own height initially. There's some code that attempts to do this in STTextView here which updates as you scroll, apparently causing the last text element in the document to adjust it's y value as the document is scrolled.
  • The ruler adjusting its width is due to the ruler view not counting lines until it comes across them, which was just a design decision by STTextView. Before it reaches the 100th line the ruler's width is determined by a 2-character width. Then when it loads the 3-character one it adjusts its width. This could be fixed with a pull request to STTextView, or it may be an opportunity to implement an in-house ruler view.

@austincondiff
Copy link
Collaborator

@thecoolwinter That makes a lot more sense. Well if it is any indication, when there is not enough text to fill the viewport, the view is vertically scrollable, the height is the viewport height plus the CodeEdit status bar height (which I want to say is a bottom edge inset. So maybe we are not taking edge insets into account?

@thecoolwinter thecoolwinter mentioned this issue Nov 18, 2023
41 tasks
austincondiff added a commit that referenced this issue Dec 9, 2023
<!--- IMPORTANT: If this PR addresses multiple unrelated issues, it will
be closed until separated. -->

### Description

Replaces `STTextView` with a custom TextView implementation. Creates a
new `TextView` and `TextViewController` classes that manage rendering
text, handling text input, and keybinds. `TextViewController` replaces
the `STTextViewController` class, connecting existing TextFormation
classes, syntax highlighting, and other existing text view extensions.

### Related Issues

* closes #208 
* closes #195 
* closes #184 
* closes #57 

### Checklist

TextView TODOs:
- [X] load text
- [X] render text
- [X] scroll
- [X] wrap text
- [X] resize
- [x] syntax highlighting
- [x] cursor
- [x] edit text
    - [x] isEditable
    - [x] Insert
    - [x] Delete
        - [x] Delete line
        - [x] Delete word
        - [x] Delete character
        - [x] Delete across lines
    - [x] Paste
- [x] [Marked
Text](https://developer.apple.com/library/archive/documentation/TextFonts/Conceptual/CocoaTextArchitecture/TextEditing/TextEditing.html#//apple_ref/doc/uid/TP40009459-CH3-SW26)
- [x] Line Numbers
- [x] Select text
    - [x] Copy
- [x] Multiple cursors
- [x] Keyboard navigation
    - [x] Arrow keys
    - [x] Command & control arrow keys
    - [ ] Page up and down
- [x] Tab widths & indents
- [x] Live parameter updating
- [x] Undo/redo
- [x] Sync system appearance
- [x] Highlight brackets
- [x] TextFormation integration
- [ ] ~MacOS Sonoma cursor~ Leaving for future PR. Will require rework
of cursor view system.
- [x] Update text from SwiftUI Binding (two-way binding)
- [x] Accessibility
- [x] Drag and Drop (bad, will need a rework but okay for now)

--
- [x] I read and understood the [contributing
guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md)
as well as the [code of
conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md)
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code

### Screenshots

`// TODO`

---------

Co-authored-by: Austin Condiff <austin.condiff@gmail.com>
Co-authored-by: Wesley de Groot <email@wesleydegroot.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🏁 Complete
Development

Successfully merging a pull request may close this issue.

3 participants