Merge pull request #1566 from nextstrain/measurements-improvements

measurements: order means by legend values
nextstrain · Oct 11, 2022 · e8e1e74 · e8e1e74
2 parents 275e68e + 9a03e6b
commit e8e1e74
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 21 deletions.
diff --git a/src/components/measurements/index.js b/src/components/measurements/index.js
@@ -127,6 +127,7 @@ const filterMeasurements = (measurements, treeStrainVisibility, filters) => {
 const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => {
   // Use `lodash.isEqual` to deep compare object states to prevent unnecessary re-renderings of the component
   const { treeStrainVisibility, treeStrainColors } = useSelector((state) => treeStrainPropertySelector(state), isEqual);
+  const legendValues = useSelector((state) => state.controls.colorScale.legendValues);
   const colorings = useSelector((state) => state.metadata.colorings);
   const colorBy = useSelector((state) => state.controls.colorBy);
   const groupBy = useSelector((state) => state.controls.measurementsGroupBy);
@@ -212,9 +213,9 @@ const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => {
   useEffect(() => {
     addColorByAttrToGroupingLabel(d3Ref.current, treeStrainColors);
     colorMeasurementsSVG(d3Ref.current, treeStrainColors);
-    drawMeansForColorBy(d3Ref.current, svgData, treeStrainColors);
+    drawMeansForColorBy(d3Ref.current, svgData, treeStrainColors, legendValues);
     addHoverPanelToMeasurementsAndMeans(d3Ref.current, handleHover, treeStrainColors);
-  }, [svgData, treeStrainColors, handleHover]);
+  }, [svgData, treeStrainColors, legendValues, handleHover]);
 
   // Display raw/mean measurements when SVG is re-drawn, colors have changed, or display has changed
   useEffect(() => {

diff --git a/src/components/measurements/measurementsD3.js b/src/components/measurements/measurementsD3.js
@@ -280,7 +280,7 @@ export const colorMeasurementsSVG = (ref, treeStrainColors) => {
     .style("fill", (d) => getBrighterColor(treeStrainColors[d.strain].color));
 };
 
-export const drawMeansForColorBy = (ref, svgData, treeStrainColors) => {
+export const drawMeansForColorBy = (ref, svgData, treeStrainColors, legendValues) => {
   const { xScale, groupingOrderedValues, groupedMeasurements } = svgData;
   const svg = select(ref);
   // Re move all current color by means
@@ -305,24 +305,28 @@ export const drawMeansForColorBy = (ref, svgData, treeStrainColors) => {
     // 2 x subplotPadding for padding around the overall mean display
     const ySpacing = (layout.subplotHeight - 4 * layout.subplotPadding) / (numberOfColorByAttributes - 1);
     let yValue = layout.subplotPadding;
-    Object.entries(colorByGroups).forEach(([attribute, {color, values}]) => {
-      drawMeanAndStandardDeviation(
-        values,
-        subplot,
-        classes.colorMean,
-        {attribute, color},
-        xScale,
-        yValue
-      );
-      // Increate yValue for next attribute mean
-      yValue += ySpacing;
-      // If the next yValue is near the overall mean display,
-      // shift to below the overall mean display + subplotPadding
-      if (yValue > (layout.overallMeanYValue - layout.subplotPadding) &&
-          yValue < (layout.overallMeanYValue + layout.subplotPadding)) {
-        yValue = layout.overallMeanYValue + layout.subplotPadding;
-      }
-    });
+    // Order the color groups by the legend value order so that we have a stable order for the means
+    legendValues
+      .filter((attribute) => String(attribute) in colorByGroups)
+      .forEach((attribute) => {
+        const {color, values} = colorByGroups[attribute];
+        drawMeanAndStandardDeviation(
+          values,
+          subplot,
+          classes.colorMean,
+          {attribute, color},
+          xScale,
+          yValue
+        );
+        // Increase yValue for next attribute mean
+        yValue += ySpacing;
+        // If the next yValue is near the overall mean display,
+        // shift to below the overall mean display + subplotPadding
+        if (yValue > (layout.overallMeanYValue - layout.subplotPadding) &&
+            yValue < (layout.overallMeanYValue + layout.subplotPadding)) {
+          yValue = layout.overallMeanYValue + layout.subplotPadding;
+        }
+      });
   });
 };