search

tajo / react-movable

🔀 Drag and drop for your React lists and tables. Accessible. Tiny.
https://react-movable.pages.dev
MIT License
1.53k stars 51 forks source link

De-selecting item on blur #53

Closed corn-eliu closed 3 years ago

corn-eliu commented 3 years ago

I'm not sure this is an issue or it's a design choice but here's what I'm currently experiencing:

I select and item by pressing space so I can move it up and down using the keyboard. Before pressing space to place it, I click somewhere outside of the list. I would expect the item to go back to where it was and be de-selected but instead it stays selected. It seems like the isSelected flag is not set to false when onblur event happens.

Am I missing something and this is desired behaviour or is this a bug?

tajo commented 3 years ago

I didn't really think about this scenario before. I agree the right thing should be returning item to the initial position since the drop was not confirmed.

arturbani commented 3 years ago

Hi @tajo and @corn-eliu ! I'm happy to work on it, if none of you want to take care of this bug. Please let me know by Sunday if any of you are going to work on it, so I can try to fix it.

corn-eliu commented 3 years ago

Hi @tajo and @corn-eliu ! I'm happy to work on it, if none of you want to take care of this bug. Please let me know by Sunday if any of you are going to work on it, so I can try to fix it.

I wouldn't even know where to start from :D so please feel free. Looking forward to it!

arturbani commented 3 years ago

Hi, @corn-eliu

Sorry for the delay, tough month :)

I've opened a PR fixing this issue on #57 ! Please see if I understood the problem correctly!

corn-eliu commented 3 years ago

Hey @arturbani

No worries. It's pretty tough for everyone :)

It all looks good to me, assuming I'm understanding the change properly. It's calling finishDrop on a selected item if it exists and the drop area loses focus? I'm not entirely sure what the if statement at line 151 of List.tsx.

arturbani commented 3 years ago

@corn-eliu Line 151 is stopping the handler execution when there's no target on the click event or the target is disabled. That was the case before, the selected item was just losing focus, but it was still considered a selected item. So the drag was technically "still happening".

So I added another if statement inside this previous statement checking if it has a selected item (!== -1), if yes, we finish the drop and update the selectedItem to -1 (no selected item).