Closed yusufozgul closed 1 year ago
Great work!
One suggestion though:
The current solution takes a fixed number of lines for the over scroll which might be more than the visible area of the text view. For example let's say the window has a height of 600pt and the over scroll line count is 70. For a font size of 13 pt this would result in a too large over scroll -> no content is visible once scrolled down all the way.
Let's try to instead provide a double (0.0-1.0) which indicates in percent of the views visible height how large the over scroll is. This should also respect window size changes and recalculate the content inset based on that. The last line of text should always be visible at a value of 1.0
.
Thank you for review :)
Yes, you are right, I made changes, also I realized a bug while changing the content inset, I should have changed contentView's bottom inset not scrollView. Otherwise after updating once, the UI does not change
This is my fault for not adding a description to the issue but I'd like to discuss how this works. I see now what @lukepistrol is suggesting. The first question is, does it make sense to allow that much flexibility?
As an example, Nova only has Small, Medium, and Large
VS Code only has a checkbox
My second question is, how are we displaying this in our Settings panel?
@austincondiff I actually thought Xcode like (none, small, medium, large). And even Nova the same. For ide, values will be just constant percentage. Xcode percentages: 0.25, 0.5, 0.75
@yusufozgul I am for this approach so we can be consistent with Xcode.
@lukepistrol does this work for you?
@austincondiff I would either go with the way Xcode handles it (none, small, medium, large) or a slider with detents (0% - 100% in 10% steps).
A fixed number of lines becomes problematic when the window height becomes smaller than said amount. And a on-off toggle is not enough flexibility.
Ok, then let's go with the Xcode approach. We should rename the variable though. Here are a few options
overscroll
overscrollAmount
editorOverscroll
editorOverscrollAmount
Doesn't "amount" imply a number? Will this now be set to an enum - .none
, .small
, .medium
, .large
?
In SwiftUI for example they keep it simple. .opacity(0.5)
is not opacityAmount
or opactyPercentage
.
Sorry to get hung up on a small detail. As they say...
"There are only two hard things in Computer Science: cache invalidation and naming things"
Doesn't "amount" imply a number?
Yeah I agree. editorOverscrollSetting
or editorOverscrollConfig
comes to mind for me. But perhaps editorOverscroll
is also fine.
I mean, "Editor Overscroll" is what the label says, so editorOverscroll
makes sense to me
It seems editorOverscroll
nice for everyone
Related Issue:
63