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

Bug with KeyboardDragListener with event bubbling and tabbing with shift #1461

Closed
jessegreenberg opened this issue Oct 5, 2022 · 3 comments

Comments

@jessegreenberg
Copy link
Contributor

I noticed this while testing in ph-scale. If you shift tab back to the dropper node, it will move slowly as if shift is still pressed down. @pixelzoom fyi.

This is happening in ph-scale because the dropper button comes after the dropper node in the traversal order and the dropper button is a child of the dropper node.

  • Shift key is pressed while focus is on the dropper button and shift gets added to the KeyboardDragListener.keyState.
  • Tab is pressed on the dropper button and focus moves to the dropper Node
  • Shift is released and we never get a keyup event for it

So shift is still in the keystate and the KeyboardDragListener is stuck in slow mode. I think the lack of keyup event for shift release after shift + tab is a browser behavior that we cannot fight. This isn't happening for most other usages of KeyboardDragListener because the keystate is cleared on blur. But blur doesn't bubble so we don't get the blur event when tabbing back from the child to the parent. I think the fix is to clear the keystate on focusout which does bubble.

@jessegreenberg
Copy link
Contributor Author

focusout works well and fixes the problem. @zepumph can you please check this change? I tested local test-server, usages of GrabDragInteraction and other usages of KeyboardDragListener. It seems pretty safe to me.

@zepumph
Copy link
Member

zepumph commented Oct 5, 2022

So if I understand correctly, this was occurring because of how the button was on a child node. Makes sense! Good and elegant fix. I can't think of any other spots where having a focusout there will be buggy or too powerful. Ready to close. Thank you

@zepumph zepumph assigned jessegreenberg and unassigned zepumph Oct 5, 2022
@jessegreenberg
Copy link
Contributor Author

You understand correctly! Thank you for reviewing. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants