From 984204c93a65c2ff9a2b255e3717117b8b59e18f Mon Sep 17 00:00:00 2001 From: David Miculit Date: Mon, 3 Aug 2026 17:10:17 +0300 Subject: [PATCH 1/5] feat: optimize array mapping and sibling rerenders --- ui/perfherder/graphs/LegendCard.jsx | 78 ++++++++++++++++------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx index 8ef6cdbfc74..077fd19aba0 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -1,4 +1,4 @@ - +import React from 'react'; import PropTypes from 'prop-types'; import { Badge, Button, Form, CloseButton } from 'react-bootstrap'; @@ -22,42 +22,47 @@ const LegendCard = ({ const newSymbols = [...symbols]; const errorMessages = []; let updates; - const newTestData = [...testData].map((item) => { - if (item.signature_id === series.signature_id) { - const isVisible = !item.visible; - - if (isVisible && newColors.length && newSymbols.length) { - item.color = newColors.pop(); - item.symbol = newSymbols.pop(); - item.visible = isVisible; - item.data = item.data.map((test) => ({ - ...test, - z: item.color[1], - _z: item.symbol, - })); - } else if (!isVisible) { - newColors.push(item.color); - newSymbols.push(item.symbol); - item.color = ['border-secondary', '']; - item.symbol = ['circle', 'outline']; - item.visible = isVisible; - item.data = item.data.map((test) => ({ - ...test, - z: item.color[1], - _z: item.symbol, - })); - } else { - errorMessages.push( - "The graph supports viewing 6 tests at a time. To select and view a test that isn't currently visible, first deselect a visible test", - ); - } - } - return item; - }); + const targetIndex = testData.findIndex((item) => item.signature_id === series.signature_id); + const item = testData[targetIndex]; + const isVisible = !item.visible; + let updatedItem = { ...item }; + + if (isVisible && newColors.length && newSymbols.length) { + updatedItem.color = newColors.pop(); + updatedItem.symbol = newSymbols.pop(); + updatedItem.visible = isVisible; + updatedItem.data = item.data.map((test) => ({ + ...test, + z: updatedItem.color[1], + _z: updatedItem.symbol, + })); + } else if (!isVisible) { + newColors.push(item.color); + newSymbols.push(item.symbol); + updatedItem.color = ['border-secondary', '']; + updatedItem.symbol = ['circle', 'outline']; + updatedItem.visible = isVisible; + updatedItem.data = item.data.map((test) => ({ + ...test, + z: updatedItem.color[1], + _z: updatedItem.symbol, + })); + } else { + errorMessages.push( + "The graph supports viewing 6 tests at a time. To select and view a test that isn't currently visible, first deselect a visible test", + ); + } if (errorMessages.length) { updates = { errorMessages, visibilityChanged: false }; } else { + // rebuild the array by slicing around the updated item + const newTestData = [ + ...testData.slice(0, targetIndex), + updatedItem, + ...testData.slice(targetIndex + 1), + ]; + updates = { testData: newTestData, colors: newColors, @@ -228,4 +233,9 @@ LegendCard.propTypes = { selectedDataPoint: PropTypes.shape({}), }; -export default LegendCard; +const areEqual = (prev, next) => + prev.series.signature_id === next.series.signature_id && + prev.series.visible === next.series.visible && + prev.colors === next.colors; + +export default React.memo(LegendCard, areEqual); \ No newline at end of file From f851a43ac4abfc99a34a612397403dab1a7fcf0e Mon Sep 17 00:00:00 2001 From: David Miculit Date: Fri, 7 Aug 2026 11:41:59 +0300 Subject: [PATCH 2/5] fix: areEqual logic --- ui/perfherder/graphs/LegendCard.jsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx index 077fd19aba0..e57b8e7127b 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -22,7 +22,9 @@ const LegendCard = ({ const newSymbols = [...symbols]; const errorMessages = []; let updates; - const targetIndex = testData.findIndex((item) => item.signature_id === series.signature_id); + const targetIndex = testData.findIndex( + (item) => item.signature_id === series.signature_id, + ); const item = testData[targetIndex]; const isVisible = !item.visible; let updatedItem = { ...item }; @@ -62,7 +64,7 @@ const LegendCard = ({ updatedItem, ...testData.slice(targetIndex + 1), ]; - + updates = { testData: newTestData, colors: newColors, @@ -234,8 +236,10 @@ LegendCard.propTypes = { }; const areEqual = (prev, next) => - prev.series.signature_id === next.series.signature_id && - prev.series.visible === next.series.visible && - prev.colors === next.colors; + prev.series === next.series && + prev.testData === next.testData && + prev.colors === next.colors && + prev.symbols === next.symbols && + prev.selectedDataPoint === next.selectedDataPoint; -export default React.memo(LegendCard, areEqual); \ No newline at end of file +export default React.memo(LegendCard, areEqual); From a5d4bb27fd09b99dbe873b40df1346c5721eb2ac Mon Sep 17 00:00:00 2001 From: David Miculit Date: Fri, 7 Aug 2026 11:46:11 +0300 Subject: [PATCH 3/5] fix: change updateItem from let to const as it is not modified --- ui/perfherder/graphs/LegendCard.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx index e57b8e7127b..03ffaeb99e3 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -27,7 +27,7 @@ const LegendCard = ({ ); const item = testData[targetIndex]; const isVisible = !item.visible; - let updatedItem = { ...item }; + const updatedItem = { ...item }; if (isVisible && newColors.length && newSymbols.length) { updatedItem.color = newColors.pop(); From 31bdd1ba10deac7ab02919fd442161d3d0d445d3 Mon Sep 17 00:00:00 2001 From: David Miculit Date: Fri, 7 Aug 2026 17:45:25 +0300 Subject: [PATCH 4/5] fix: identity comparison correctly sees a change and minor visual bug with the card colors --- ui/perfherder/graphs/LegendCard.jsx | 48 +++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx index 03ffaeb99e3..f0743b091cd 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -113,24 +113,46 @@ const LegendCard = ({ // when removing a test, check to see if the next test in the queue had a color; // if it had secondary and was deselected, reset its color and visibility to // the removed test's color, otherwise push that color back into the colors list + + const promoteIndex = graphColors.length - 1; + if ( - newData[graphColors.length - 1] && - newData[graphColors.length - 1].color[0] === 'border-secondary' + newData[promoteIndex] && + newData[promoteIndex].color[0] === 'border-secondary' ) { - newData[graphColors.length - 1].color = series.color; - newData[graphColors.length - 1].visible = true; - newData[graphColors.length - 1].data = newData[ - graphColors.length - 1 - ].data.map((item) => ({ - ...item, - z: series.color[1], - })); - resetParams(newData); + const promoted = newData[promoteIndex]; + let nextColor; + let nextSymbol; + const newColors = [...colors]; + const newSymbols = [...symbols]; + + // if the removed test was disabled, its real color/symbol are in the pools. + if (series.color[0] === 'border-secondary') { + nextColor = newColors.pop(); + nextSymbol = newSymbols.pop(); + } else { + nextColor = series.color; + nextSymbol = series.symbol; + } + + newData[promoteIndex] = { + ...promoted, + color: nextColor, + symbol: nextSymbol, + visible: true, + data: promoted.data.map((item) => ({ + ...item, + z: nextColor[1], + _z: nextSymbol, + })), + }; + resetParams(newData, newColors, newSymbols); } else if (series.color[0] === 'border-secondary') { resetParams(newData); } else { - const newColors = [...colors, ...[series.color]]; - resetParams(newData, newColors); + const newColors = [...colors, series.color]; + const newSymbols = [...symbols, series.symbol]; + resetParams(newData, newColors, newSymbols); } }; From 2f56a1d52d514d6b711c744f63a7548b47a1fdd5 Mon Sep 17 00:00:00 2001 From: David Miculit Date: Tue, 11 Aug 2026 15:22:03 +0300 Subject: [PATCH 5/5] fix: redone broken removeTest logic --- ui/perfherder/graphs/LegendCard.jsx | 66 ++++++++++++----------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx index f0743b091cd..b96cc10b0e2 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import { Badge, Button, Form, CloseButton } from 'react-bootstrap'; import { getFrameworkName } from '../perf-helpers/helpers'; -import { graphColors } from '../perf-helpers/constants'; import { Perfdocs } from '../perf-helpers/perfdocs'; import GraphIcon from '../../shared/GraphIcon'; @@ -110,50 +109,39 @@ const LegendCard = ({ newData.splice(index, 1); - // when removing a test, check to see if the next test in the queue had a color; - // if it had secondary and was deselected, reset its color and visibility to - // the removed test's color, otherwise push that color back into the colors list - - const promoteIndex = graphColors.length - 1; - - if ( - newData[promoteIndex] && - newData[promoteIndex].color[0] === 'border-secondary' - ) { - const promoted = newData[promoteIndex]; - let nextColor; - let nextSymbol; - const newColors = [...colors]; - const newSymbols = [...symbols]; + // removing a disabled test frees nothing, since it never held a + // color, just drop it. Removing a visible test frees its + // color: promote the first currently-disabled test to take its + // place, or return the color/symbol to the pool if none is waiting. + if (series.color[0] === 'border-secondary') { + resetParams(newData); + return; + } - // if the removed test was disabled, its real color/symbol are in the pools. - if (series.color[0] === 'border-secondary') { - nextColor = newColors.pop(); - nextSymbol = newSymbols.pop(); - } else { - nextColor = series.color; - nextSymbol = series.symbol; - } + const promoteIndex = newData.findIndex( + (item) => item.color[0] === 'border-secondary', + ); - newData[promoteIndex] = { - ...promoted, - color: nextColor, - symbol: nextSymbol, - visible: true, - data: promoted.data.map((item) => ({ - ...item, - z: nextColor[1], - _z: nextSymbol, - })), - }; - resetParams(newData, newColors, newSymbols); - } else if (series.color[0] === 'border-secondary') { - resetParams(newData); - } else { + if (promoteIndex === -1) { const newColors = [...colors, series.color]; const newSymbols = [...symbols, series.symbol]; resetParams(newData, newColors, newSymbols); + return; } + + const promoted = newData[promoteIndex]; + newData[promoteIndex] = { + ...promoted, + color: series.color, + symbol: series.symbol, + visible: true, + data: promoted.data.map((item) => ({ + ...item, + z: series.color[1], + _z: series.symbol, + })), + }; + resetParams(newData); }; const subtitleStyle = 'p-0 mb-0 border-0 text-secondary text-start';