rc-dock icon indicating copy to clipboard operation
rc-dock copied to clipboard

Wrong DropDirection and multiple onLayoutChange calls in Controlled-Layout

Open Prapti-044 opened this issue 3 years ago • 3 comments

When using a controlled layout, and I try to drag a floating tab to dock it, that tab is getting vanished. In the console, I discovered that when docking a floating tab/panel, onLayoutChange event is called twice (first with proper newLayout and direction, and later with move as a direction where newLayout is actually wrong.

See demo below:

https://user-images.githubusercontent.com/31363580/180181236-4a6ea4b4-1fa9-4c70-afa8-7652fb85c553.mp4

Below is the same example as this example in typescript except adaptation for my local end. I have also tested it using ECMA and getting the same result:

import DockLayout, { DropDirection, LayoutBase, LayoutData } from 'rc-dock';
import React from 'react'
import "rc-dock/dist/rc-dock.css";

let tab0 = {
  title: 'Controlled Layout',
  content: (
    <div>
      <p>When you use <b>layout</b> instead of <b>defaultLayout</b> on &lt;DockLayout&gt;</p>
      <p>DockLayout will work as a controlled component</p>
    </div>
  )
};

let box: LayoutBase = {
  dockbox: {
    mode: 'horizontal',
    children: [
      {
        mode: 'vertical',
        children: [
          {
            tabs: [{id: 't0'}],
          },
          {
            tabs: [{id: 'protect1'}, {id: 't4'}, {id: 't5'}, {id: 't6'}],
          }
        ]
      },
      {
        tabs: [{id: 't7'}, {id: 't8'}, {id: 't9'}],
      },
    ]
  }
};

class App extends React.Component {
  state = {layout: box};

  loadTab = ({id}: {id: string}) => {
    switch (id) {
      case 't0':
        return {...tab0, id};
      case 'protect1' :
        return {
          id, title: 'Protect',
          closable: true,
          content: <div>
            <p>Removal of this tab will be rejected</p>
            This is done in the onLayoutChange callback
          </div>
        };
    }

    return {
      id, title: id,
      content: <div>Tab Content</div>
    };
  };

  onLayoutChange = (newLayout: LayoutData, currentTabId: string, direction: DropDirection) => {
    // control DockLayout from state
    console.log(currentTabId, newLayout, direction);
    if (currentTabId === 'protect1' && direction === 'remove') {
      alert('removal of this tab is rejected');
    } else {
      this.setState({layout: newLayout});
    }
  };

  render() {
    return (
      <DockLayout layout={this.state.layout} loadTab={this.loadTab} onLayoutChange={this.onLayoutChange}
                  style={{position: 'absolute', left: 10, top: 10, right: 10, bottom: 10}}/>
    );
  }
}

export default App;

Prapti-044 avatar Jul 21 '22 09:07 Prapti-044

@Prapti-044 would you mind checking which version of React you are using? I just created #159. It's only guessing on my part for now, but I figured maybe both issues are related.

ldegen avatar Aug 11 '22 13:08 ldegen

@ldegen, My stripped package.json is:

{
  "dependencies": {
    "@reduxjs/toolkit": "^1.8.3",
    "@types/jest": "^28.1.5",
    "@types/node": "^18.0.4",
    "@types/react": "^18.0.15",
    "@types/react-dom": "^18.0.6",
    "bootstrap": "^5.1.3",
    "class-transformer": "^0.5.1",
    "graphviz-react": "^1.2.0",
    "rc-dock": "^3.2.10",
    "react": "^18.1.0",
    "react-bootstrap": "^2.4.0",
    "react-dom": "^18.1.0",
    "react-grid-layout": "^1.3.4",
    "react-redux": "^8.0.2",
    "react-scripts": "5.0.1",
    "react-syntax-highlighter": "^15.5.0",
    "react-toastify": "^9.0.7",
    "redux": "^4.2.0",
    "reflect-metadata": "^0.1.13",
    "typescript": "^4.7.4"
  }
}

Prapti-044 avatar Aug 12 '22 02:08 Prapti-044

Thanks @Prapti-044. I see you are using React 18 l, as I suspected. Same here.

The reason why I think our problems could be related is that you also mention a second change event with direction "move" and a wrong/outdated layout. This is exactly what is happening in #159.

It's only a theory for now, but I think the code that fires both events assumes that the effect of the first event is immediately reflected in the layout prop. This assumption by itself may be somewhat problematic and point to a larger problem, but at least this seemed to work fine with React 17.x, which explains why the controlled-layout example works, while our code does not.

So this may be a workaround if downgrading to React 17 is an option for you? For me it's not, I'm afraid.

I'm still hoping that there is some proper way to fix this, but obviously I don't understand enough of the code base to assess how likely or difficult this would be. Only the author(s) can tell.

ldegen avatar Aug 12 '22 07:08 ldegen

fixed in 3.2.12

rinick avatar Sep 24 '22 08:09 rinick