-
Notifications
You must be signed in to change notification settings - Fork 136
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
[4/5] Adding an object that tracks the scrolling state. #35
[4/5] Adding an object that tracks the scrolling state. #35
Conversation
Pull Request Test Coverage Report for Build 61
💛 - Coveralls |
Pull Request Test Coverage Report for Build 24
💛 - Coveralls |
1 similar comment
Pull Request Test Coverage Report for Build 24
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package is slightly abstract to follow. I believe some comment improvements would remove that issue.
Feel free to do this in a different change.
// scrollPage stores user requests to scroll up (negative) or down | ||
// (positive) by a page of content. E.g. -1 means up by one page and 2 | ||
// means down by two pages. | ||
scrollPage int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is not clear to me is how the scroll and scrollPage are maintained. If you scroll 2 lines up then scroll = -2. But if you scroll 2 pages and 1 line up does scroll = -1 and scrollPage = -2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have expanded the comment for the object itself which now hopefully covers this.
widgets/text/scroll.go
Outdated
return rollingPaused | ||
} | ||
|
||
// lastLineVisible returns true if the last line is visible given drawing that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we re-write this comment to clarify what the last line means. Is it the last line in the buffer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment rewritten. Please let me know if this doesn't help.
[5/5] Implementing the text widget.
Works on #4