-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add top ignore scroll offset prop to the scroller #337
Conversation
Add bottom offset to the scroll bar and thumb.
((reverse | ||
? el.scrollHeight + el.scrollTop - el.clientHeight | ||
: el.scrollTop) / | ||
(el.scrollHeight - el.clientHeight)) * |
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.
We need to keep this reverse scroll logic, so scrollTop
should include this bit reverse ? el.scrollHeight + el.scrollTop - el.clientHeight : el.scrollTop
in place of el.scrollTop
I think.
@@ -17,6 +17,8 @@ export interface CustomScrollbarsProps { | |||
verticalBarClass?: string; | |||
innerClass: string | Element | null; | |||
headerPadding?: number; | |||
scrollIgnoreLength?: number; | |||
bottomScrollBarOffset?: number; |
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.
Why do we need this?
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.
How do you mean why? :D These are the props that tells if we want to ignore some scrolling at the top, and if we want to have some offset at the bottom of the scroll bar/thumb. we need both for SideNav but none for other places where we use CustomScrollbars which is why they defaults to 0.
Did you have other idea for this?
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.
Sorry, I meant just the bottomScrollBarOffset
prop. I thought we only needed to inset the decorative track line for the docs SideNav, which doesn't need the actual scrollbar to be inset?
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 think the scroller should be inset because the thumb also should not go until the screen edge or below.
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.
if we inset only the decorative line (scroll bar) the thumb will continue scroll after the line finish which is weird, we want to finish scrolling before the screen edge
eb6581b
to
f323ce3
Compare
Add bottom offset to the scroll bar and thumb.