final-form / react-final-form-arrays

A component for rendering and editing arrays 🏁 React Final Form
MIT License
204 stars 69 forks source link

React Beautiful DnD example doesn't behave as expected #112

Open jwld opened 5 years ago

jwld commented 5 years ago

Are you submitting a bug report or a feature request?

Bug

What is the current behavior?

1) Add 3 items to the array in this sandbox: https://codesandbox.io/s/8k0ykyr7z9 2) Swap the 1st and 3rd item, and the list ends up reading 3-2-1 (because of the use of .swap() rather than .move())

What is the expected behavior?

The list should read 3-1-2.

Sandbox Link

https://codesandbox.io/s/8k0ykyr7z9

NateRadebaugh commented 4 years ago

Could this be because of the way the examples for react-final-form-arrays set the key to be name, which is based on index, which the react docs explicitly caution against:

We don’t recommend using indexes for keys if the order of items may change. This can negatively impact performance and may cause issues with component state. Check out Robin Pokorny’s article for an in-depth explanation on the negative impacts of using an index as a key. If you choose not to assign an explicit key to list items then React will default to using indexes as keys.

However, what other options do we have since the data in the form is not available to us?

jwld commented 4 years ago

It works as expected if .move() is used instead. It's just confusing if you copy the code from that example because at some point you'll realise it's swapping the source and destination cards, rather than just moving the source, which is probably the intended behaviour in most situations.

I probably should've just made a PR because it's a simple change, will do that later!

gadiGuesty commented 4 years ago

@jwld in the docs example it's with .move()and it still does not work. any updates on this?

gadiGuesty commented 4 years ago

@NateRadebaugh it works only in newer versions of react-final-form-arrays when fields allows to extract other properties like id in the example.Example should be fixed as:

{fields.map((name, index) => {
                const {id} = fields.value[index];
                return(
                  <Draggable
                    key={id}
                    draggableId={id}
                    index={index}>
                    ...
gadiGuesty commented 4 years ago

Duplicate of #94 Duplicate of #150

noveogroup-amorgunov commented 3 years ago

@gadiGuesty you saved my day, thank you a lot