SortableJS / react-sortablejs

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

Consider renaming chosen and selected property #243

Open Rc85 opened 2 years ago

Rc85 commented 2 years ago

I noticed these two properties were added to my objects and although it only presented one minor issue, please consider on renaming these two properties to something less common and more unique, such as _chosen and _selected or even sortablejs:chosen and sortablejs:selected. Most of the time, data are retrieved from a database and could have columns with these names, which in turn becomes the properties of an object. Setting it would overwrite these properties in the user's retrieved data.

danieliser commented 1 year ago

From what I gather these are state props that shouldn't be passed out to setList anyways.

They are used so that you can give visual queues to the chosen/selected elements, as well as for internal needs.

Otherwise these should be stripped away before passing it out to the callbacks.

I see no need to preserve this data other than possibly some form of state-saving which is well beyond what should be out of the box experience.

I understand they are passed to setList so that your own app can redraw with the visual indicators noted above, but there should be a helper or similar to strip them.

This becomes extremely messy with nestable sortable lists.

danieliser commented 1 year ago

Currently we are saving setList into a temporary state value, and only when they click the save button do we clean it and pass it back to the app.

MatrixAge commented 1 year ago

+1

makis-x commented 1 year ago

Agreed these should not be passed to setList

mfaheemakhtar commented 9 months ago

The component should have an internal state without mutating the actual data.

It caused our API validation to fail as these properties don't exist. Traced it back to this library.