SortableJS / react-sortablejs

React bindings for SortableJS
http://sortablejs.github.io/react-sortablejs/
MIT License
2.03k stars 210 forks source link

[bug] Setting disabled as a prop has no effect #153

Open cronco opened 4 years ago

cronco commented 4 years ago

Describe the bug When setting disabled as a prop that might be changed, it has no effects

To Reproduce Steps to reproduce the behavior:

  1. Create a element
  2. set disabled prop to a dynamic property
  3. Change disabled property.
  4. No change in behaviour.

Expected behavior If changing disabled from false to true, expect sortable not to work anymore.

Information Not a lot more to say. I've tried it with handles and no handles and have created an example. For my particular example I've made do by changing the class of the handle, in effect disabling dragging by that route, but this is still an issue.

Versions - Look in your package.json for this information: react-sortable = ^2.0.11 react = ^16.13.0

Additional context

I have created an example in this PR

joaocmfcm commented 4 years ago

I'm currently facing the same issue. Any news on this?

jordanandree commented 4 years ago

I ran into this issue and found a workaround using the Context API and a forwardRef that sets state using Sortable.get. It's not the most elegant solution.

Here is an example of this working together: https://codesandbox.io/s/sad-joliot-x3oqs?file=/src/App.js

cronco commented 4 years ago

My workaround, as stated in the original description was "changing the class of the handle, in effect disabling dragging by that route"

hungdev commented 4 years ago

How do you change css to disable it? @cronco

mboynes commented 4 years ago

I ran into a similar issue and resolved it by setting the key prop (in my case, I set it on a wrapping component, but I assume the same would work on the ReactSortable component) to a value that took the enabled/disabled state into account, e.g. `sortable.${disabled}`. By changing the key when disabled toggles, we tell React that this is now a new element and React unmounts the old one and remounts (re-initializes might be more accurate) a new one. This is not an ideal practice since, as I understand it, we're bypassing React's shadow DOM efficiencies in exchange for inefficient DOM manipulations. That said, in my case it hasn't been even remotely noticeable.

oh-klahoma commented 3 years ago

May be you to close this a bug? @waynevanson

bvanheukelom commented 3 years ago

This is based on what I saw in ninja grizzlys fork and it works for me:

class FixedReactSortable<T extends ItemInterface> extends ReactSortable<T> {

    componentDidUpdate(prevProps: ReactSortableProps<T>): void {
        if (this["sortable"] && (prevProps.disabled !== this.props.disabled)) {
            this["sortable"].option('disabled', this.props.disabled);
        }
    }

}

Then just use FixedReactSortable instead of ReactSortable

evanjmg commented 3 years ago

works for me now without hack?

sprout2000 commented 1 year ago

Same in 2022...

https://codesandbox.io/s/react-sortablejs-demo-1ruwpl

export default function App() {
  const [list, setList] = useState([
    { id: 0, value: "I hate myself." },
    { id: 1, value: "Gonna live forever!" }
  ]);
  const [sortable, setSortable] = useState(true);

  const handleOnClick = () => {
    setSortable(!sortable);
  };

  return (
    <div>
      <button onClick={handleOnClick}>{sortable ? "Disable" : "Enable"}</button>
      <ReactSortable
        disabled={!sortable}
        dragClass="sortable-drag"
        ghostClass="sortable-ghost"
        animation={350}
        tag="ul"
        list={list}
        setList={setList}
      >
        {list.map((item) => (
          <li key={item.id}>{item.value}</li>
        ))}
      </ReactSortable>
    </div>
  );
}
mrgazanfarli commented 8 months ago

Still actual