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.

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

feat: add the ability to specify a scrollable target element for tracking scroll direction #311

Merged
merged 7 commits into from
Jul 26, 2024

Conversation

CXBoyy
Copy link
Contributor

@CXBoyy CXBoyy commented Jul 6, 2024

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 is target?: HTMLDivElement. The parameter defaults to window if it is not provided:

const {
    target = window,
    thr = 0,
    axis = Axis.Y,
    scrollUp = axis === Axis.Y ? Direction.Up : Direction.Left,
    scrollDown = axis === Axis.Y ? Direction.Down : Direction.Right,
    still = Direction.Still
  } = props;

Furthermore, the code has been updated to allow for correct directional tracking depending on if the target has the default value window or is set to a specific element. Some of these changes are shown below:

const scroll =
      target instanceof Window
        ? axis === Axis.Y
          ? window.scrollY
          : window.scrollX
        : axis === Axis.Y
          ? target.scrollTop
          : target.scrollLeft;
const top = target instanceof Window ? target.scrollY : target.scrollTop;
      const left =
        target instanceof Window ? target.scrollX : target.scrollLeft;
      const bottom =
        target instanceof Window
          ? document.documentElement.scrollHeight - window.innerHeight - top
          : document.documentElement.scrollHeight - target.scrollHeight - top;
      const right =
        target instanceof Window
          ? document.documentElement.scrollWidth - window.innerWidth - left
          : document.documentElement.scrollHeight - target.scrollWidth - left;
target instanceof Window
      ? window.addEventListener('scroll', onScroll)
      : target.addEventListener('scroll', onScroll);

No other changes were made to the calculations or logic of the directional tracking.

QA Checklist

  • I have tested this change on multiple React versions
  • I have verified compatibility with different React setups (e.g., Create React App, Next.js, etc.)
  • I have added/updated relevant tests, if applicable
  • I have updated the README.md and related documentation, if necessary

I 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

  • The hook's API remains consistent and user-friendly
  • Any new features have been documented with clear examples

Release Notes

Note: once this PR is merged, a new release or update might be required for the users.

  • Introduction of a feature to track the scroll direction of specific elements.

Copy link
Owner

@SMAKSS SMAKSS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of window, as target defaults to window when not specified. I will also conduct tests across different React versions before merging the changes.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@CXBoyy
Copy link
Contributor Author

CXBoyy commented Jul 7, 2024

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 of window, as target defaults to window 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!

Copy link
Owner

@SMAKSS SMAKSS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for applying the changes. LGTM! 🎉

I will release this version after more testing.

@punkpeye
Copy link

punkpeye commented Jul 20, 2024

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.

@punkpeye
Copy link

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.

@punkpeye
Copy link

punkpeye commented Jul 20, 2024

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]);
};

@punkpeye
Copy link

@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!

@CXBoyy
Copy link
Contributor Author

CXBoyy commented Jul 20, 2024

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.

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.

@SMAKSS
Copy link
Owner

SMAKSS commented Jul 20, 2024

@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.

@SMAKSS SMAKSS changed the base branch from master to develop July 26, 2024 22:20
@SMAKSS SMAKSS merged commit 2d044a0 into SMAKSS:develop Jul 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: allow specifying a custom scrollable element to track
3 participants