Closed tsufiev closed 10 months ago
vercel[bot]
commented
10 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| react-resizable-panels | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Dec 6, 2023 4:09pm |
tsufiev
commented
10 months ago I have to agree that guarding against nullish dragState so deep in the logic is not ideal, since most likely the root cause lies somewhere inside PanelResizeHandle.ts file. I presume that getting nullish dragState in the code that handles panel resizing means that resizeHandler has been invoked without the prior invocation of startDragging. However due to the overall logic of binding and unbinding event listeners inside PanelResizeHandle.ts being a bit convoluted, I cannot understand when and why that happens :/.
tsufiev
commented
10 months ago @bvaughn I've managed to reproduce the issue 2 times out of ~100, so it's quite rare. Both times it happened during dragging the panel to collapsed state. The attempt of resize while dragState stored in committedValuesRef is null happens inside onMove handler.
My mental picture of this is that when resize handle gets released and cursor is moved at the same time, sometimes the onMove handler gets one more chance to get executed before useEffect clean-up code kicks in and removes all the listeners.
bvaughn
commented
10 months ago Interesting. Would be super nice to capture a case of this happening in Replay.
bvaughn
commented
10 months ago Cherry picking this change into my release branch caused 11 e2e tests to fail (same as with CI).
This highlights that there are several triggers for resize actions, and drag is only one of them. This change would effectively block keyboard and imperative API resizes.
I will think on this change some more before the v1 release but we can't land the thing proposed here.
Edit ddb4a52474d50149bbee46600f6d558a3fc2de9b
Resolves #223