react-sortablejs icon indicating copy to clipboard operation
react-sortablejs copied to clipboard

[bug] Setting disabled as a prop has no effect

Open cronco opened this issue 5 years ago • 11 comments

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 <ReactSortable /> 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

cronco avatar May 13 '20 14:05 cronco

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

joaocmfcm avatar Jun 12 '20 17:06 joaocmfcm

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

jordanandree avatar Jul 03 '20 05:07 jordanandree

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

cronco avatar Jul 06 '20 08:07 cronco

How do you change css to disable it? @cronco

hungdev avatar Jul 28 '20 08:07 hungdev

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.

mboynes avatar Sep 21 '20 13:09 mboynes

May be you to close this a bug? @waynevanson

oh-klahoma avatar Dec 03 '20 15:12 oh-klahoma

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

bvanheukelom avatar Jul 01 '21 12:07 bvanheukelom

works for me now without hack?

evanjmg avatar Sep 16 '21 17:09 evanjmg

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>
  );
}

sprout2000 avatar Oct 25 '22 04:10 sprout2000

Still actual

mrgazanfarli avatar Jan 24 '24 21:01 mrgazanfarli