Skip to content

Commit

Permalink
Change the format of d3 effect hooks in the plot
Browse files Browse the repository at this point in the history
I'm not 100% sure I'm doing this right, but here is what I did, and here
is why I did it.

1) I started passing refs to DOM elements to the hooks rather than
`hook.current`, and I took took that ref out of the dependency list
for the hooks. The latter is easy: `ref.current` *never* changes because
it always refers to the same object, so it doesn't make sense to have it
as an effect dependency.

<facebook/react#14387 (comment)>

`useEffect` runs *after* the <svg> node has already been added to the
DOM, so now we don't have to check if it is `null`.

2) I'm using `useLayoutEffect` rather than `useEffect`, because all of
these hooks affect the state of the DOM-- they are just being managed by
D3 rather than React.
  • Loading branch information
ptgolden committed Apr 14, 2021
1 parent 3660a96 commit 3a2fee1
Showing 1 changed file with 61 additions and 60 deletions.
121 changes: 61 additions & 60 deletions src/components/MAPlot/Plot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type InteractionActions =
'drag' |
'zoom'

const { useState, useEffect, useRef } = React
const { useState, useLayoutEffect, useEffect, useRef } = React

const TRANSCRIPT_BIN_MULTIPLIERS = [
0,
Expand Down Expand Up @@ -40,11 +40,11 @@ type PlotProps = {
>

function useAxes(
svgEl: SVGSVGElement | null,
svgRef: React.RefObject<SVGSVGElement>,
dimensions: PlotDimensions
) {
useEffect(() => {
if (svgEl === null) return
useLayoutEffect(() => {
const svgEl = svgRef.current

const { xScale, yScale } = dimensions

Expand Down Expand Up @@ -81,7 +81,7 @@ function useAxes(
.attr('stroke', '#eee')
.attr('stroke-width', 1)
})
}, [svgEl, dimensions])
}, [dimensions])
}

type BinData = {
Expand All @@ -92,7 +92,7 @@ type BinData = {
}

function useBins(
svgEl: SVGSVGElement | null,
svgRef: React.RefObject<SVGSVGElement>,
dimensions: PlotDimensions,
{
loading,
Expand All @@ -102,10 +102,11 @@ function useBins(
) {
const ref = useRef<d3.Selection<SVGRectElement, BinData, SVGElement, unknown>>()

useEffect(() => {
if (!svgEl || loading) return;
useLayoutEffect(() => {
if (loading) return;

const { xScale, yScale } = dimensions
const svgEl = svgRef.current
, { xScale, yScale } = dimensions

if (pairwiseData === null) {
d3.select(svgEl)
Expand Down Expand Up @@ -197,23 +198,24 @@ function useBins(
.select('.squares > g')
.remove()
}
}, [ svgEl, dimensions, pairwiseData, pValueThreshold ])
}, [ dimensions, pairwiseData, pValueThreshold ])

return ref
}

function useWatchedTranscripts(
svgEl: SVGSVGElement | null,
svgRef: React.RefObject<SVGSVGElement>,
dimensions: PlotDimensions,
{
savedTranscripts,
pairwiseData,
}: PlotProps
) {
useEffect(() => {
if (!svgEl || !pairwiseData) return
useLayoutEffect(() => {
if (!pairwiseData) return

const { xScale, yScale } = dimensions
const svgEl = svgRef.current
, { xScale, yScale } = dimensions

d3.select(svgEl)
.select('.saved-transcripts')
Expand All @@ -236,57 +238,56 @@ function useWatchedTranscripts(
.selectAll('circle')
.remove()
}
}, [ svgEl, dimensions, pairwiseData, savedTranscripts ])
}, [ dimensions, pairwiseData, savedTranscripts ])
}

function useHoveredTranscriptMarker(
svgEl: SVGSVGElement | null,
svgRef: React.RefObject<SVGSVGElement>,
dimensions: PlotDimensions,
{
hoveredTranscript,
pairwiseData,
}: PlotProps
) {
useEffect(() => {
if (!svgEl) return

const { xScale, yScale } = dimensions
, container = d3.select(svgEl).select('.hovered-marker')

if (hoveredTranscript === null) return;
if (pairwiseData === null) return;

const data = pairwiseData.get(hoveredTranscript)

if (!data) return

const { logATA, logFC } = data

if (logATA === null || logFC === null) return

container.append('circle')
.attr('cx', xScale(logATA))
.attr('cy', yScale(logFC))
.attr('r', 20)
.attr('opacity', 1)
.attr('fill', 'none')
.attr('stroke', 'coral')
.attr('stroke-width', 2)

container.append('circle')
.attr('cx', xScale(logATA))
.attr('cy', yScale(logFC))
.attr('opacity', 1)
.attr('r', 3)
.attr('fill', 'coral')

return () => {
container.selectAll('circle')
.transition()
.duration(360)
.ease(d3.easeCubicOut)
.style('opacity', 0)
.remove()
useLayoutEffect(() => {
const svgEl = svgRef.current
, { xScale, yScale } = dimensions
, container = d3.select(svgEl).select('.hovered-marker')

if (hoveredTranscript === null) return;
if (pairwiseData === null) return;

const data = pairwiseData.get(hoveredTranscript)

if (!data) return

const { logATA, logFC } = data

if (logATA === null || logFC === null) return

container.append('circle')
.attr('cx', xScale(logATA))
.attr('cy', yScale(logFC))
.attr('r', 20)
.attr('opacity', 1)
.attr('fill', 'none')
.attr('stroke', 'coral')
.attr('stroke-width', 2)

container.append('circle')
.attr('cx', xScale(logATA))
.attr('cy', yScale(logFC))
.attr('opacity', 1)
.attr('r', 3)
.attr('fill', 'coral')

return () => {
container.selectAll('circle')
.transition()
.duration(360)
.ease(d3.easeCubicOut)
.style('opacity', 0)
.remove()
}
}, [ hoveredTranscript, pairwiseData, dimensions ])
}
Expand All @@ -306,12 +307,12 @@ export default function Plot(props: PlotProps) {
transform,
})

useAxes(svgEl, dimensions)
useAxes(svgRef, dimensions)

const binSelectionRef = useBins(svgEl, dimensions, props)
const binSelectionRef = useBins(svgRef, dimensions, props)

useHoveredTranscriptMarker(svgEl, dimensions, props)
useWatchedTranscripts(svgEl, dimensions, props)
useHoveredTranscriptMarker(svgRef, dimensions, props)
useWatchedTranscripts(svgRef, dimensions, props)

return (
h('div', [
Expand Down

0 comments on commit 3a2fee1

Please sign in to comment.