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

Add top ignore scroll offset prop to the scroller #337

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Add top ignore scroll offset prop to the scroller #337

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd545bd
Add top ignore scroll offset prop to the scroller
diksipav Mar 12, 2024
f323ce3
Add back piece for reverse scrolling
diksipav Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 46 additions & 22 deletions shared/common/ui/customScrollbar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export interface CustomScrollbarsProps {
verticalBarClass?: string;
innerClass: string | Element | null;
headerPadding?: number;
scrollIgnoreLength?: number;
bottomScrollBarOffset?: number;
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

reverse?: boolean;
hideVertical?: boolean;
hideHorizontal?: boolean;
Expand All @@ -31,45 +33,62 @@ export function CustomScrollbars({
verticalBarClass,
innerClass,
headerPadding = 0,
scrollIgnoreLength = 0,
bottomScrollBarOffset = 8,
reverse,
hideVertical,
hideHorizontal,
}: PropsWithChildren<CustomScrollbarsProps>) {
const ref = useRef<HTMLDivElement>(null);

const scrollSizes = useRef<[number, number]>(defaultScrollSizes);
const [rawScrollOffset, setRawScrollOffset] = useState(0);
const [scrollOffsets, setScrollOffsets] = useState<[number, number]>(() => [
0,
0,
0, 0,
]);
const _scrollOffsets = useRef(scrollOffsets);
const [dragging, setDragging] = useState(false);

const scrollBarTopOffset =
rawScrollOffset < scrollIgnoreLength
? rawScrollOffset
: scrollIgnoreLength;

const onScroll = useCallback(
(el: HTMLElement) => {
const scrollTop =
Math.max(
0,
reverse
? el.scrollHeight + el.scrollTop - el.clientHeight
: el.scrollTop - scrollIgnoreLength
) /
(el.scrollHeight - scrollIgnoreLength - el.clientHeight);

_scrollOffsets.current = [
((reverse
? el.scrollHeight + el.scrollTop - el.clientHeight
: el.scrollTop) /
(el.scrollHeight - el.clientHeight)) *
Comment on lines -51 to -54
Copy link
Member

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.

scrollTop *
(el.clientHeight -
scrollSizes.current[0] -
(scrollSizes.current[1] === -1 ? 8 : 14) -
scrollSizes.current[0] +
scrollBarTopOffset -
(scrollSizes.current[1] === -1 ? bottomScrollBarOffset : 14) -
headerPadding),
(el.scrollLeft / (el.scrollWidth - el.clientWidth)) *
(el.clientWidth -
scrollSizes.current[1] -
(scrollSizes.current[0] === -1 ? 8 : 14)),
];
setScrollOffsets(_scrollOffsets.current);
setRawScrollOffset(el.scrollTop);
},
[headerPadding, reverse]
[headerPadding, reverse, scrollBarTopOffset]
);

const onResize = useCallback(() => {
const scrollEl = (scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild) as HTMLElement;
const scrollEl = (
scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild
) as HTMLElement;
if (!scrollEl) return;

const hasV = scrollEl.scrollHeight > scrollEl.clientHeight;
Expand All @@ -79,7 +98,8 @@ export function CustomScrollbars({
? Math.max(
28,
(scrollEl.clientHeight / scrollEl.scrollHeight) *
(scrollEl.clientHeight - (hasH ? 14 : 8) - headerPadding)
(scrollEl.clientHeight - (hasH ? 14 : 8)) -
headerPadding
)
: -1,
hasH
Expand All @@ -105,9 +125,11 @@ export function CustomScrollbars({
useResize(innerRef, onResize);

useEffect(() => {
const scrollEl = (scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild) as HTMLElement;
const scrollEl = (
scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild
) as HTMLElement;
if (scrollEl) {
const listener = (e: Event) => onScroll(e.target as HTMLElement);

Expand All @@ -129,9 +151,11 @@ export function CustomScrollbars({
e.stopPropagation();
e.preventDefault();

const el = (scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild) as HTMLElement;
const el = (
scrollClass
? ref.current?.querySelector(`.${scrollClass}`)
: ref.current!.firstChild
) as HTMLElement;
const initial = vScroll ? e.clientY : e.clientX;
const initialScroll = vScroll ? el.scrollTop : el.scrollLeft;
const barSize = vScroll
Expand Down Expand Up @@ -210,14 +234,14 @@ export function CustomScrollbars({
<div
className={cn(styles.verticalBar, verticalBarClass)}
style={{
top: headerPadding,
bottom: scrollSizes.current[1] === -1 ? 0 : 6,
top: headerPadding - scrollBarTopOffset,
bottom: scrollSizes.current[1] === -1 ? bottomScrollBarOffset : 6,
}}
>
<div
className={styles.scroller}
style={{
height: scrollSizes.current[0],
height: scrollSizes.current[0] + scrollBarTopOffset,
transform: `translateY(${scrollOffsets[0]}px)`,
}}
/>
Expand Down
Loading