spectrum-web-components
spectrum-web-components copied to clipboard
[Bug]: SWC uses keyup handlers more often than it should
Code of conduct
- [X] I agree to follow this project's code of conduct.
Impacted component(s)
Overlay and many more
Expected behavior
SWC should default to using keydown events unless there's a specific reason to use keyup.
Actual behavior
There's many places where SWC handles events with keyup. This makes it difficult if you have another handler for that key registered to keydown. Rather than being able to use event bubbling you'd need to add specific logic based on the handling SWC is doing.
The example we hit was the escape key handling in the overlay implementation. We have a keydown handler that was triggering for a component under the overlay.
Screenshots
No response
What browsers are you seeing the problem in?
No response
How can we reproduce this issue?
- Go to '...'
- Click on '....'
- Scroll to '....'
- Check console
- See error
Sample code that illustrates the problem
No response
Logs taken while reproducing problem
No response
In talking with @charlessuh more about this, I think there's also a case for modal overlays to trap the escape
key in the capture phase.
We recently updated SWC and saw some additional problems around this area. We found that our dialogs were no longer intercepting key presses as a result of this SWC update, not sure the cause, but we found that we needed to explicitly stop propagation of keypress events. It seemed like it was related to the above, would be good to standardize on keyup for everything if possible.
Getting a catalog of this together. keyup
is used as the event name in ActionButton.ts
, ButtonBase.ts
, ColorArea.ts
, overlay-stack.ts
, and Radio.ts
.
ActionButton.ts
: leverages keyup
to track the length of the press action to synthesize longpress
and cannot be removed without a very different approach. In theory we could do some work with counting keypress
events instead, but that seems less reliable that what we're doing now. Chance of change: none.
ButtonBase.ts
: leverage keyup
to synthesize click
events on Button elements and its derivatives. I guess this could be moved to keydown
but would not correctly map to what you get out of a native <button>
element. Chance of change: low.
ColorArea.ts
: keyup
is used to track which modifier keys are down at any one time. Probably can move to https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/getModifierState and manage those keys in a possibly more stable way. Chance of change: good.
overlay-stack.ts
: manages whether to close a modal or not. It seems that the <dialog>
element does handle this on keydown
, so happy to move this into alignment with that. Chance of change: High.
Radio.ts
: leverage keyup
to synthesize click
events on the element. I guess this could be moved to keydown
but would not correctly map to what you get out of a native <button>
element. Chance of change: low.
With this in mind we'd definitely accept a PR with updates to Color Area and the Overlay Stack, if you were interested in hurrying this forward. If this is blocking for you, we can definitely raise the priority on our end, but can pretty solidly booked for at least the next two weeks.
I'd take reasoning or expanded ideas on the other elements that have keyup
handlers, but am no very likely to accept changes there without a really solid alternative.
Thoughts?