Click event can get swallowed

[michaelboyles]

Now that the clickable area of the separator will be configurable, your response to this might be a big old "who cares?", which could be totally fine. But it seems kinda interesting and very likely to be unintentional, so I thought I'd raise it anyway just to see what you think.

I changed the very first docs example to this, so clicking anywhere on the panel should log something.

<Group>
    <Panel onClick={() => console.log("click")}>left</Panel>
    <Panel>right</Panel>
</Group>

Now click on somewhere within both the panel and the hit region; nothing will get logged. You can use resizeTargetMinimumSize with a large number to make it even easier.

My first thought was that it must be this within onDocumentPointerDown (sidenote: this will fire even if there was no hitRegion where match was true - maybe worth fixing), because it's the only action done with the event other than to get some values out of it

if (hitRegions.length) {
    event.preventDefault();
}

But commenting this out didn't change anything.

I then went down the rabbit hole of commenting things out until the click listener was firing even in the hit region. So firstly I narrowed it to update, then eventEmitter.emit("interactionStateChange", state.interactionState); within that.

Then looking at the handlers for that event, the handler that causes the issue is the one that calls panel.scheduleUpdate(). That function ultimately comes from useForceUpdate. Updating that hook to stub out the function, while obviously breaking the library, means that the click event finally makes it to the panel.

export function useForceUpdate() {
  const [sigil, setSigil] = useState({});

  const forceUpdate = () => {};

  return [sigil as unknown, forceUpdate] as const;
}

So something about calling the setter from useState in a capturing phase listener causes the click to get swallowed. This was as far as I got. forceUpdate is clearly necessary. Wrapping setSigil in setTimeout(..., 0) or queueMicrotask doesn't fix anything.

Again, maybe we don't care about this. You can just make your hit region not overlap anything clickable. But this is such roundabout behaviour that I'm sure you didn't decide that this is how it should be. There would be much simpler ways to swallow the event.

Thoughts?

[bvaughn]

(sidenote: this will fire even if there was no hitRegion where match was true - maybe worth fixing)

This check is just in place for TypeScript. Hit regions are calculated from mounted groups.

[bvaughn]

My first guess was that React might be removing and re-attaching the "click" handler to the panel in this case, as it re-renders, but even when I imperatively attached a listener in an effect– that doesn't get fired either. I'm not sure what's interrupting it to be honest.

This does not sound like a very high priority issue to me though, as the "click" event in this case should be default-prevented anyway (since it's been handled by the separator). If you'd like to dig into it and submit a PR I'm happy to take a look though.