craft.js icon indicating copy to clipboard operation
craft.js copied to clipboard

Editor actions not working with PrimeReact Buttons

Open ccaspanello opened this issue 3 years ago • 4 comments

First and fore most, I love the concepts behind this library . . . very intuitive and easy to use.

However, I've been fond of PrimeReact recently and trying to migrate from MaterialUI to PrimeReact has been giving me trouble.

While this MIGHT be a PrimeReact bug I'm hoping I can learn more about how CraftJS works so I can work with the PrimeReact team to fix it.

Below is the sample project that I have been trying to convert. You will see that if I use a <button> tag things work as expected. But if I use the <Button> tag from PrimeReact the Drag/Drop Editor actions don't work. https://codesandbox.io/s/xenodochial-sid-r7c20?file=/src/components/Topbar.tsx

In addition if I use the <Button> tag from PrimeReact in the Topbar the Undo/Redo buttons are inconsistent (sometimes they work and sometimes they don't).

Cheers!

ccaspanello avatar Aug 18 '21 05:08 ccaspanello

After playing around with the Undo/Redo buttons some more and changing to regular <button> tags I get inconsistent behavior; so this is probably not a PrimeReact issue

ccaspanello avatar Aug 18 '21 05:08 ccaspanello

Hi,

When I change the code around a bit in Toolbox.tsx:

      <button
        className="p-button p-component"
        ref={(ref: any) => {
          console.log("button", ref);
          connectors.create(ref, <ButtonWidget text="Click me" size="small" />);
        }}
      >
        Button
      </button>
      <Button
        ref={(ref: any) => {
          console.log("PrimeButton", ref);
          connectors.create(ref, <TextWidget text="Hi world" />);
        }}
      >
        Text
      </Button>

We see that the output for the two is different: image

Although the ref is passed down to the button component correctly, the draggable="true" is missing from the PrimeReact Button.

Luckily the sourcecode for the PrimeButton is available.

From there I created a minimal example just handling the ref like they do.

import React, { Component, createRef } from "react";

export class PrimeTestButtonComponent extends Component {
  constructor(props) {
    super(props);

    this.elementRef = createRef(this.props.forwardRef);
  }

  updateForwardRef() {
    let ref = this.props.forwardRef;

    if (ref) {
      if (typeof ref === "function") {
        ref(this.elementRef.current);
      } else {
        ref.current = this.elementRef.current;
      }
    }
  }

  componentDidMount() {
    this.updateForwardRef();
  }

  render() {
    return <button ref={this.elementRef}>{this.props.children}</button>;
  }
}

export const PrimeTestButton = React.forwardRef((props, ref) => (
  <PrimeTestButtonComponent forwardRef={ref} {...props} />
));

It does not work because we are missing a this.updateForwardRef in componentWillUpdate:

import React, { Component, createRef } from "react";

export class PrimeTestButtonComponent extends Component {
  constructor(props) {
    super(props);

    this.elementRef = createRef(this.props.forwardRef);
  }

  updateForwardRef() {
    let ref = this.props.forwardRef;

    if (ref) {
      if (typeof ref === "function") {
        ref(this.elementRef.current);
      } else {
        ref.current = this.elementRef.current;
      }
    }
  }

  componentDidMount() {
    this.updateForwardRef();
  }

  componentDidUpdate() {
    this.updateForwardRef();
  }

  render() {
    return <button ref={this.elementRef}>{this.props.children}</button>;
  }
}

export const PrimeTestButton = React.forwardRef((props, ref) => (
  <PrimeTestButtonComponent forwardRef={ref} {...props} />
));

With the addition it now works. See my CodeSandbox

ankri avatar Aug 18 '21 06:08 ankri

But I can confirm, that something weird happens with the undo and redo stuff. I cannot reproduce this with the basic example, though. :(

When I log out the contents of the history's timeline, then there are too many entries after just adding one button.

But I finally tracked it down to <StrictMode>. It works when you remove it from index.js. CodeSandbox to try it out without <StrictMode>.

Maybe @prevwong can help.

tl;dr History does not seem to work with react's strict mode.

ankri avatar Aug 18 '21 07:08 ankri

Please note that I didn't try with the next branch

ankri avatar Aug 18 '21 07:08 ankri