Better API for rich components

[ChocolateLoverRaj]

trafficstars

The props injected into child components can conflict with the other props of the child component. Example:

import { forwardRef, useRef } from "react";
import Draggable from "react-draggable";
import { italic } from "./App.module.css";

const Component1 = forwardRef((props, ref) => {
  const { className } = props;

  console.log(props);

  return (
    <div {...props} ref={ref}>
      <span>This should not be italic</span>
      <br />
      <span className={className}>This should be italic</span>
    </div>
  );
});

export default function App() {
  const ref = useRef(null);

  return (
    <Draggable nodeRef={ref}>
      <Component1 ref={ref} className={italic} />
    </Draggable>
  );
}

Link to codesandbox: https://codesandbox.io/s/react-draggable-bug-3-p99yd?file=/src/App.js

In this example, creating <div {...props} /> results in the div getting unwanted props.

There should be a better API for components like Component1, preferably without modifying the props. This could be done by lifting the state up into the component that renders <Draggable /> and <Component1 />. This is what a 'better API' could look like:

import { forwardRef, useRef } from "react";
import Draggable, { useRichComponent } from "react-draggable";
import { italic } from "./App.module.css";

const Component1 = forwardRef((props, ref) => {
  const { draggableProps, className } = props;

  return (
    <div {...draggableProps} ref={ref}>
      <span>This should not be italic</span>
      <br />
      <span className={className}>This should be italic</span>
    </div>
  );
});

export default function App() {
  const ref = useRef(null);
  const { childProps, internalControl } = useRichComponent();

  return (
    <Draggable nodeRef={ref} internalControl={internalControl}>
      <Component1 ref={ref} className={italic} draggableProps={childProps} />
    </Draggable>
  );
}

The above example uses a hook for the <Draggable /> to control the props of <Component1 /> without modifying the jsx element props. This is similar to Form.useForm() in Ant Design.

This issue is related to #585 because I don't think the solution to the issue is a good API, as it can cause problems demonstrated in the sandbox.

[STRML]

What does internalControl do? And why not just do this:

const Component1 = forwardRef((props, ref) => {
  const { className, ...restProps } = props;

  console.log(props);

  return (
    <div {...restProps} ref={ref}>
      <span>This should not be italic</span>
      <br />
      <span className={className}>This should be italic</span>
    </div>
  );
});

[ChocolateLoverRaj]

@STRML internalControl is some function / object that allows the draggable component to control the value of childProps. Internally, it could be using useState to get and set the props.

[ChocolateLoverRaj]

@STRML your example would not work because then the draggable class name won't be on the div. The component would need to extract the draggable class names from the custom class names, and put the draggable class names in the div and the custom class name on the 2nd span. I'm not sure of the class name prop is necessary for the dragging to work, but I'm assuming not having the class name on the div can break things.

[STRML]

It doesn't need the class.

Another way we could work around this would be to set the event handlers on the ref instead of the child, so the props don't even need to be passed.

On Wed, Jul 21, 2021 at 10:25 PM Rajas Paranjpe @.***> wrote:

@STRML https://github.com/STRML your example would not work because then the draggable class name won't be on the div. The component would need to extract the draggable class names from the custom class names, and put the draggable class names in the div and the custom class name on the 2nd span. I'm not sure of the class name prop is necessary for the dragging to work, but I'm assuming not having the class name on the div can break things.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/react-grid-layout/react-draggable/issues/586#issuecomment-884616154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJEKPZD6JPGED5GJZXPVW3TY56SNANCNFSM5AY2FDKA .

[ChocolateLoverRaj]

@STRML I think that's the best solution.

[STRML]

There's going to be some performance loss as a result of this unfortunately, as event handlers would be directly attached to every single Draggable rather than delegated by React. For simple use cases this is fine, but for larger use cases like React-Grid-Layout (where there may be hundreds of draggables between the grid items & the resize handles), I am not sure this is an appropriate tradeoff.

[ChocolateLoverRaj]

@STRML then I think my first solution would work best, with the useRichComponent hook. That would let React do the event handler stuff while still being flexible.