Closed CXBoyy closed 1 month ago
Thank you for your contribution to this repository. I truly appreciate your effort. I’ve suggested some improvements to enhance readability, including reducing nested ternary operations and consistently using
target
instead ofwindow
, astarget
defaults towindow
when not specified. I will also conduct tests across different React versions before merging the changes.
Hi @SMAKSS
Glad that you liked my addition to your project! I think I have fixed the issues you addressed, although I did leave two comments on some things I was a bit unsure about. The fixes were added in commit d0eed75. I have once again tested it on my own project, and it seems to work!
Just give me a holler if there is anything else I should add or change!
Any updates on this? Would love to add this to https://glama.ai.
One problem that I ran into while trying this out, is that I am getting window is not defined
error. I understand why, but it would be nice if the hook incorporated some logic for guarding against it.
Just experimenting more with this, I think it would be more correct if target
accepted React.RefObject<HTMLDivElement>
instead of HTMLDivElement
, because that would force to handle cases where the element is not set. At the moment, it quietly falls back to window
in those cases.
So this is by no means an equivalent implementation, but what I ended up with for a use case of only needing this for elements.
/**
* @license https://github.com/SMAKSS/react-scroll-direction
*/
import { useCallback, useEffect, useRef, useState } from 'react';
/**
* Enumeration for axis values
*/
export enum Axis {
/**
* The x-axis represents the horizontal direction.
*/
X = 'x',
/**
* The y-axis represents the vertical direction.
*/
Y = 'y',
}
/**
* Enumeration for direction values
*/
export enum Direction {
/**
* The down direction represents the scroll direction moving towards the bottom.
*/
Down = 'down',
/**
* The left direction represents the scroll direction moving towards the left.
*/
Left = 'left',
/**
* The right direction represents the scroll direction moving towards the right.
*/
Right = 'right',
/**
* The still direction represents the scroll direction when the user is not scrolling.
*/
Still = 'still',
/**
* The up direction represents the scroll direction moving towards the top.
*/
Up = 'up',
}
type ScrollPosition = {
/**
* The bottom position represents the distance from the bottom edge of the page.
*/
bottom: number;
/**
* The left position represents the distance from the left edge of the page.
*/
left: number;
/**
* The right position represents the distance from the right edge of the page.
*/
right: number;
/**
* The top position represents the distance from the top edge of the page.
*/
top: number;
};
/**
* Type declaration for the returned scroll information
*/
type ScrollInfo = {
/**
* The scrollDir represents the current scroll direction.
*/
scrollDir: Direction;
/**
* The scrollPosition represents the current scroll position.
*/
scrollPosition: ScrollPosition;
};
/**
* Type declaration for scroll properties
*/
type ScrollProps = {
/**
* The axis represents the scroll axis (x or y).
*/
axis?: Axis;
/**
* The onScroll function is called when scroll threshold is reached.
*/
onScroll: (scrollInfo: ScrollInfo) => void;
/**
* The scrollDown represents the scroll direction when moving down.
*/
scrollDown?: Direction;
/**
* The scrollUp represents the scroll direction when moving up.
*/
scrollUp?: Direction;
/**
* The still represents the scroll direction when the user is not scrolling.
*/
still?: Direction;
/**
* The target represents the scrollable element to check for scroll detection.
*/
target: React.RefObject<HTMLDivElement>;
/**
* The thr represents the threshold value for scroll detection.
*/
threshold?: number;
};
export const useDetectScroll = (props: ScrollProps) => {
const {
target: targetRef,
threshold = 0,
axis = Axis.Y,
scrollUp = axis === Axis.Y ? Direction.Up : Direction.Left,
scrollDown = axis === Axis.Y ? Direction.Down : Direction.Right,
still = Direction.Still,
onScroll,
} = props;
const [scrollDir, setScrollDir] = useState<Direction>(still);
const [scrollPosition, setScrollPosition] = useState<ScrollPosition>({
bottom: 0,
left: 0,
right: 0,
top: 0,
});
const ticking = useRef(false);
const lastScroll = useRef(0);
/**
* Function to update scroll direction
*/
const updateScrollDir = useCallback(() => {
const target = targetRef.current;
if (target === null) {
return;
}
const scroll =
axis === Axis.Y
? (target as HTMLDivElement).scrollTop
: (target as HTMLDivElement).scrollLeft;
if (Math.abs(scroll - lastScroll.current) >= threshold) {
setScrollDir(scroll > lastScroll.current ? scrollDown : scrollUp);
lastScroll.current = Math.max(0, scroll);
}
ticking.current = false;
}, [axis, scrollDown, scrollUp, targetRef, threshold]);
useEffect(() => {
const target = targetRef.current as HTMLDivElement;
if (target === null) {
return () => {};
}
const updateScrollPosition = () => {
const top = target.scrollTop;
const left = target.scrollLeft;
const bottom = target.scrollHeight - (top + target.clientHeight);
const right = target.scrollWidth - (left + target.clientWidth);
setScrollPosition({ bottom, left, right, top });
};
/**
* Call the update function when the component mounts
*/
updateScrollPosition();
const targetElement = target as EventTarget;
targetElement.addEventListener('scroll', updateScrollPosition);
return () => {
targetElement.removeEventListener('scroll', updateScrollPosition);
};
}, [targetRef]);
useEffect(() => {
const target = targetRef.current;
if (target === null) {
return () => {};
}
lastScroll.current =
axis === Axis.Y
? (target as HTMLDivElement).scrollTop
: (target as HTMLDivElement).scrollLeft;
const onScroll = () => {
if (!ticking.current) {
if (typeof window === 'undefined') {
return;
}
window.requestAnimationFrame(updateScrollDir);
ticking.current = true;
}
};
const targetElement = target as EventTarget;
targetElement.addEventListener('scroll', onScroll);
return () => targetElement.removeEventListener('scroll', onScroll);
}, [axis, targetRef, updateScrollDir]);
useEffect(() => {
onScroll({ scrollDir, scrollPosition });
}, [scrollDir, scrollPosition]);
};
@SMAKSS I would also consider refactoring the API of the hook to something like this, to make it more universal:
useDetectScroll({
axis: Axis.Y,
scrollDown: Direction.Down,
scrollUp: Direction.Up,
still: Direction.Still,
target: listRef,
threshold: 100,
onScroll: ({scrollDir, scrollPosition}) => {
console.log('scroll');
},
});
Sorry for taking over the thread!
Just experimenting more with this, I think it would be more correct if
target
acceptedReact.RefObject<HTMLDivElement>
instead ofHTMLDivElement
, because that would force to handle cases where the element is not set. At the moment, it quietly falls back towindow
in those cases.
This seems to be a bug specific to server-side-rendered frameworks such as Next.js, right? Unfortunately, I did not encounter this bug as I use Vite without SSR.
Reading about this bug here, it seems the appropriate solution would be to use the useEffect
hook, as you appear to have done in your solution. Maybe you could fork my fork/PR and apply your changes into a new PR? I don't know if this is proper etiquette though. @SMAKSS could probably give better guidance here.
@punkpeye Thanks for your detailed work and suggestions. The initial PR by @CXBoyy was approved because it was specifically for pure React apps. Since they volunteered for this feature, I wanted to avoid pushing them to handle additional edge cases. I attempted to address these myself but haven't had enough time, which has delayed merging the PR.
To provide context for the next release, we must implement a way to handle window
properly in SSR applications. Additionally, the bottom scroll position isn't working correctly for targeted elements other than window
. It looks like you've covered both of these cases. Could you please create another PR on top of this one with the necessary changes? Let me know once it's ready for review. Thanks a lot.
Resolves #263.
This project originally provides the ability for a developer to track the scroll direction of the standard window of their application. However, for certain cases, the developer may want to track the scroll direction of one or more specific scrollable elements, rather than the whole window. This PR introduces this functionality.
WHAT does this PR introduce or fix?
The major change is the addition of an optional parameter to the
ScrollProps
type. The added parameter istarget?: HTMLDivElement
. The parameter defaults towindow
if it is not provided:Furthermore, the code has been updated to allow for correct directional tracking depending on if the
target
has the default valuewindow
or is set to a specific element. Some of these changes are shown below:No other changes were made to the calculations or logic of the directional tracking.
QA Checklist
README.md
and related documentation, if necessaryI have tested the new function on my own WIP website without any issues. The hook successfully tracks the scrolling direction of a specified element and correctly gives the scroll position as well.
Usage-specific Checklist
Release Notes
Note: once this PR is merged, a new release or update might be required for the users.