Dont interfere with outside handlers #115
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -82,6 +82,7 @@ export function useWindowSplitterPanelGroupBehavior({ | |||
| handle.setAttribute("aria-valuenow", "" + Math.round(parseInt(flexGrow))); | |||
|
|
|||
| const onKeyDown = (event: KeyboardEvent) => { | |||
| event.stopPropagation(); | |||
There was a problem hiding this comment.
I don't think this is the right fix.
Maybe the "Enter" case below should call event.preventDefault() to signal that it's handled the event, but I don't think it the component should swallow all keyboard events. Can you explain why you think that change is needed?
There was a problem hiding this comment.
My issue is that I have an key up listener outside of the component like here (global shortcuts):
https://codesandbox.io/s/react-resizable-panels-forked-vywou9
When resizing via the keyboard, the outside listener get's triggered as well.
event.preventDefault() would not resolve the issue.
Another option would be to be able to pass in a custom onKeyUp handler from the outside and handle it from the consumer side?
There was a problem hiding this comment.
event.preventDefault()would not resolve the issue.
Your key handler would need to respect event.defaultPrevented
Another option would be to be able to pass in a custom onKeyUp handler from the outside and handle it from the consumer side?
This doesn't sound like something I'd want to add. Feels like it adds unnecessary complexity to the API of this component.
There was a problem hiding this comment.
Interesting, I've never encountered this approach.
It works, but it's hard to introduce this change in a giant code base, where this wasn't previously considered (which is of course not your responsibility, just trying to provide context 😸 ).
Right now this is blocking me from using this library, as I can't even disable the focus ability or change the handlers.
There was a problem hiding this comment.
I think defaultPrevented would be a reasonable strategy here to signal that the event has already been "handled". Alternately you could block this event in your own code by wrapping the resize handle, e.g.
<div onKeyDown={trapEvent}>
<PanelResizeHandle {...props} />
</div>There was a problem hiding this comment.
Ah that's also a solution, thank you!
I'm investigating how to include defaultPrevented and think it's a good way to go, I've changed the line in the PR now!
|
Published in 0.0.38 |
When focusing on the resizehandler and resizing with the keyboard, outside event keyhandlers would be executed as well.