Question 73904 in Reactjs Question posted in
The new location for the React Documentation can be found here.

Is there any way to refactor this custom component to reduce code duplication? – Reactjs

I have this wrapper component for the mui Tooltip component with the added functionality that it automatically closes the tooltip when the table around it is being scrolled. The code works well but I want to improve the code quality since there are a few duplicate lines. Also I believe there should be a way to find the reference to the table once and use it in all three function calls. Any help would be appreciated and I apologise for such a generic question. I don’t know how to make it more specific.

import { TooltipProps } from '@mui/material';
import { useRef, useState } from 'react';
import { Tooltip } from '../tooltip/Tooltip';

interface TableTooltipProps extends TooltipProps {
  selector?: string;
}

export function TableTooltip({
  selector = '.MuiTableContainer-root',
  children,
  ...props
}: TableTooltipProps) {
  const [isScrolling, setIsScrolling] = useState(false);
  const [timeoutId, setTimeoutId] = useState<number>(null);
  const ref = useRef<any>();

  const handleScroll = () => {
    console.warn('scrolling');
    setIsScrolling(true);
    if (ref.current) {
      ref.current.closest(selector).removeEventListener('scroll', handleScroll);
      console.warn('aborted');
    }
    clearTimeout(timeoutId);
    setTimeoutId(
      window.setTimeout(() => {
        setIsScrolling(false);
      }, 500),
    );
  };

  const attachToScrollEvent = () => {
    if (ref.current) {
      console.warn('added');
      const tableElement = ref.current.closest(selector);
      tableElement.addEventListener('scroll', handleScroll);
    }
  };

  const detachFromScrollEvent = () => {
    if (ref.current) {
      console.warn('removed');
      const tableElement = ref.current.closest(selector);
      tableElement.removeEventListener('scroll', handleScroll);
    }
  };

  return !isScrolling ? (
    <>
      <Tooltip
        onOpenCallback={attachToScrollEvent}
        onCloseCallback={detachFromScrollEvent}
        {...props}
      >
        {children}
      </Tooltip>
      <td className="html-element-dummy" style={{ display: 'none' }} ref={ref} />
    </>
  ) : (
    <>{children}</>
  );
}
Tags:

2

Answers


  1. Chosen as BEST ANSWER
    0

    I figured out a way to improve my own code. The answer by @Wesley unfortunately had some undesirable side effects.

    I am just not sure whether useCallback is necessary here.

    import { TooltipProps } from '@mui/material';
import { useCallback, useEffect, useRef, useState } from 'react';
import { Tooltip } from '../tooltip/Tooltip';

interface TableTooltipProps extends TooltipProps {
  selector?: string;
}

export function TableTooltip({
  selector = '.MuiTableContainer-root',
  children,
  ...props
}: TableTooltipProps): JSX.Element {
  const [isScrolling, setIsScrolling] = useState<boolean>();
  const [timeoutId, setTimeoutId] = useState<number>();
  const [tableElement, setTableElement] = useState<Element>();
  const ref = useRef<HTMLTableCellElement>();

  useEffect(() => {
    setTableElement(ref.current?.closest(selector));
  }, [ref, selector]);

  const handleScroll = useCallback(() => {
    tableElement.removeEventListener('scroll', handleScroll);
    setIsScrolling(true);
    clearTimeout(timeoutId);
    setTimeoutId(
      window.setTimeout(() => {
        setIsScrolling(false);
      }, 500),
    );
  }, [tableElement, timeoutId]);

  const attachToScrollEvent = useCallback(() => {
    tableElement.addEventListener('scroll', handleScroll);
  }, [tableElement, handleScroll]);

  const detachFromScrollEvent = useCallback(() => {
    tableElement.removeEventListener('scroll', handleScroll);
  }, [tableElement, handleScroll]);

  return !isScrolling ? (
    <>
      <Tooltip
        onOpenCallback={attachToScrollEvent}
        onCloseCallback={detachFromScrollEvent}
        {...props}
      >
        {children}
      </Tooltip>
      <td className="html-element-dummy" style={{ display: 'none' }} ref={ref} />
    </>
  ) : (
    <>{children}</>
  );
}

  2. 0

    A simple refactor using optional chaining would allow you to reduce code by ignoring the error when the ref is initially undefined for an insignificant period of time, on the .closest and the addEventListener / removeEventListener calls.

    import { TooltipProps } from '@mui/material';
import { useRef, useState } from 'react';
import { Tooltip } from '../tooltip/Tooltip';

interface TableTooltipProps extends TooltipProps {
  selector?: string;
}

export function TableTooltip({
  selector = '.MuiTableContainer-root',
  children,
  ...props
}: TableTooltipProps) {
  const [isScrolling, setIsScrolling] = useState(false);
  const [timeoutId, setTimeoutId] = useState<number>(null);
  const ref = useRef<any>();
  const tableElement = ref?.current?.closest(selector);

  const handleScroll = () => {
    setIsScrolling(true);
    tableElement?.removeEventListener('scroll', handleScroll);
    clearTimeout(timeoutId);
    setTimeoutId(window.setTimeout(() => setIsScrolling(false), 500));
  };

  const attachToScrollEvent = () => tableElement?.addEventListener('scroll', handleScroll);
  const detachFromScrollEvent = () => tableElement?.removeEventListener('scroll', handleScroll);

  return !isScrolling ? (
    <>
      <Tooltip
        onOpenCallback={attachToScrollEvent}
        onCloseCallback={detachFromScrollEvent}
        {...props}
      >
        {children}
      </Tooltip>
      <td className="html-element-dummy" style={{ display: 'none' }} ref={ref} />
    </>
  ) : (
    <>{children}</>
  );
}
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search