Tree click interaction fixes

[georgefst]

These small changes all started as part of #833, but whereas what remains there is at least controversial, these all seem like no-brainers.

[brprice]

This all sounds sensible to me, and the code looks good. However, I have not tested it, as the deployment does not seem to be working (there is a "view deployment" button which brings up a session list, but I cannot create a new session -- it seems to be 404ing trying to hit a backend. I am a bit out of the loop on the current state of PR deployments, so maybe this is expected?)

[georgefst]

However, I have not tested it, as the deployment does not seem to be working

Turns out this was because I hadn't added the label. The deployment is now working, in case you want to test it.

[brprice]

The deployment is now working, in case you want to test it.

Thanks. Looks good.

[dhess]

I think these changes aren't quite no-brainers:

• Rather than removing edge selection, I think a better UX improvement would be to select the child node of an edge when an edge is selected. I can especially see this being helpful on small screens with complicated trees, where edges might be quite long due to layout, and their corresponding child nodes somewhere off-screen. Personally, I find completely inert bits of the tree really frustrating to deal with (see recent discussions about wanting to make pattern constructors selectable), so IMO we should have really good reasons before adding new elements to that set.

• Speaking of edge selection, and I'm not sure whether this was present before or not, but for the dotted edges from definition node to its type & term nodes, the cursor flickers between draggable cursor and "inert" cursor depending on whether you're hovering over a dot or a gap. Regardless of whether we make edges selectable or not, I think we should treat these dotted edges as a uniform object, and not subject to whether you're hovering over a gap or a dot.

(Additionally, on the first point, if we do decide to select a the child node of an edge when the edge is selected, we might want to think about highlighting the edge, as well. That might be useful for keeping track of who the parent is in complex trees.)

[dhess]

One more thing while I'm thinking about it, and this is not a criticism of this PR, just a related idea: is there any good reason why we don't support dragging the canvas regardless of what the cursor is hovering over? Since we're not planning to use drag interactions on nodes or edges, why not just make it really easy to drag the canvas, rather than requiring the student to mouse over/touch an inert part of the canvas before it can be dragged?

[dhess]

See comments above. I could be talked out of the selectable edge suggestion, but I think the dotted-edge behavior should be fixed, regardless.

[georgefst]

Rather than removing edge selection, I think a better UX improvement would be to select the child node of an edge when an edge is selected.

See the latest version (though the implementation needs cleaning up a tad). I kind of like it, although I agree that we should ideally somehow highlight edges, or maybe highlight the node when the edge is hovered (which might be awkward to implement), and I'm not really sure what that should look like. I also think the "interaction width" around edges is a little high, but that's easily fixed.

Speaking of edge selection, and I'm not sure whether this was present before or not, but for the dotted edges from definition node to its type & term nodes, the cursor flickers between draggable cursor and "inert" cursor depending on whether you're hovering over a dot or a gap.

This is new, and I hadn't noticed it, and agree it's annoying. As it happens, the previous fix solves it as a side-effect. I'm not actually sure of another simple way to do so.

One more thing while I'm thinking about it, and this is not a criticism of this PR, just a related idea: is there any good reason why we don't support dragging the canvas regardless of what the cursor is hovering over?

Because ReactFlow doesn't support it, unfortunately.

[dhess]

Rather than removing edge selection, I think a better UX improvement would be to select the child node of an edge when an edge is selected.

See the latest version (though the implementation needs cleaning up a tad). I kind of like it, although I agree that we should ideally somehow highlight edges, or maybe highlight the node when the edge is hovered (which might be awkward to implement), and I'm not really sure what that should look like.

Yeah, if we're going to make the edges selectable, then we should probably highlight them when hovered, as we do for nodes (though obviously not via the ring effect). But we can merge selectable edges without the highlighting support.

I see that the current implementation is marked as a hack, so it's understood that you didn't intend for it to be proper support. With that in mind, there are a few issues with it. There seems to be some "right-bias", in that (on Safari, anyway) as the edges start to converge near the parent, at some point the right edge is selected even when the cursor is pretty clearly over the left edge. Here are a few examples, the first of which occurs around 15s into this screen capture:

Screen.Recording.2023-03-22.at.5.49.20.PM.mov

Hopefully this can be fixed by making the selection area smaller or something like that.

I figured that one issue with the suggestion that edges be selectable would be, "which one is selected when the edges converge at the root?" I think I'd have preferred the left edge be the one selected, but it seems to be the right one. It's not the end of the world, though.

[dhess]

Speaking of edge selection, and I'm not sure whether this was present before or not, but for the dotted edges from definition node to its type & term nodes, the cursor flickers between draggable cursor and "inert" cursor depending on whether you're hovering over a dot or a gap.

This is new, and I hadn't noticed it, and agree it's annoying. As it happens, the previous fix solves it as a side-effect. I'm not actually sure of another simple way to do so.

I can confirm that on Safari on macOS, this appears to be fixed now, which is I guess another reason to support selecting edges :)

One more thing while I'm thinking about it, and this is not a criticism of this PR, just a related idea: is there any good reason why we don't support dragging the canvas regardless of what the cursor is hovering over?

Because ReactFlow doesn't support it, unfortunately.

Hmm, that's unfortunate. Is there an existing issue in upstream for this?

[georgefst]

Note to self: when revisiting this, consider how it interacts with #931.