allen-cell-animated / website-3d-cell-viewer

Other
5 stars 4 forks source link

Convert transfer function editor to function component #276

Closed frasercl closed 3 months ago

frasercl commented 3 months ago

Review time: medium-large (45-60min). Advances #80.

The remaining work on implementing @lynwilhelm's designs for an updated channel settings interface (available here) involves adding new visual elements and functionality to the transfer function editor. This component is currently implemented as a React class component, though I believe it has been adapted from an older framework at least once, and draws its plot imperatively with D3 outside of React's normal render/commit cycle. Owing to the many lives this component has lived, it now contains a fair bit of unused or contradictory code and tangled imperative updates that are difficult to follow or reason about. Facing the possibility of adding to this tangle, and encouraged by D3's own example of how the library can integrate more naturally with React's declarative style, I decided it would be worth the time to adapt the component to this style first.

The new component contains about half the code of the old and should be a near-match visually and functionally, though I am aware of at least a few changes:

Steps to test:

ShrimpCryptid commented 3 months ago

image

FYI I'm getting some text selection behavior when I click and drag on a point in Chrome. If text is selected, dragging the control point again causes the 🚫 mouse icon to appear.

frasercl commented 3 months ago

FYI I'm getting some text selection behavior when I click and drag on a point in Chrome. If text is selected, dragging the control point again causes the 🚫 mouse icon to appear.

I saw this as well and tried to resolve it with various combos of stopPropagations, preventDefaults, and handlers for new events (onMouseDown, onClick, etc.) which just contained stopPropagations and preventDefaults. With the current combo I'm unable to replicate the highlighting issue on Chrome, so if you're on the most up-to-date version of this branch and still seeing the issue I may want to investigate it further on your machine.

frasercl commented 3 months ago

Following up on the above: turns out I was using a slightly older version of Chrome (122 I believe). Upgrading to Chrome 126 introduced the text highlighting issue for me as well. The CSS changes added in my most recent commit address the issue.