Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

De-selecting item on blur #53

Closed
corn-eliu opened this issue Oct 2, 2020 · 6 comments · Fixed by #57
Closed

De-selecting item on blur #53

corn-eliu opened this issue Oct 2, 2020 · 6 comments · Fixed by #57
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@corn-eliu
Copy link

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
Copy link
Owner

tajo commented Oct 2, 2020

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.

@tajo tajo added bug Something isn't working good first issue Good for newcomers labels Oct 2, 2020
@arturbani
Copy link
Contributor

arturbani commented Oct 3, 2020

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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.

@tajo tajo closed this as completed in #57 Oct 16, 2020
@arturbani
Copy link
Contributor

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants