From f65e6534ae523cd44495978b67b9064ecaaa7716 Mon Sep 17 00:00:00 2001 From: Vio Date: Tue, 13 Jun 2023 22:50:57 +0200 Subject: [PATCH 1/5] enhance(ui): MetricsTable - improve totals header --- .../src/components/metrics-table/metrics-table.jsx | 13 +++++++------ .../metrics-table/metrics-table.module.css | 10 +++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/ui/src/components/metrics-table/metrics-table.jsx b/packages/ui/src/components/metrics-table/metrics-table.jsx index 7226578de3..b98dbdec4c 100644 --- a/packages/ui/src/components/metrics-table/metrics-table.jsx +++ b/packages/ui/src/components/metrics-table/metrics-table.jsx @@ -24,8 +24,7 @@ const getHeaderLabelCells = (rows) => (run, runIndex, runs) => { if (!run) { return [ - { children: '-', className }, - ...(!isBaseline ? [{ children: ' ', className: styles.delta }] : []), + { children: '-', className, colSpan: isBaseline ? 1 : 2 }, ]; } @@ -41,11 +40,13 @@ const getHeaderLabelCells = (rows) => (run, runIndex, runs) => { ); + // Value column return [ - // Value column - { children: jobName, className }, - // Delta column - ...(!isBaseline ? [{ children: ' ', className: cx(styles.delta) }] : []), + { + children: jobName, + className, + colSpan: isBaseline ? 1 : 2, + }, ]; }; diff --git a/packages/ui/src/components/metrics-table/metrics-table.module.css b/packages/ui/src/components/metrics-table/metrics-table.module.css index b6874c94f1..bfcdd1907b 100644 --- a/packages/ui/src/components/metrics-table/metrics-table.module.css +++ b/packages/ui/src/components/metrics-table/metrics-table.module.css @@ -27,7 +27,7 @@ .root .delta { padding-left: 0; - text-align: left; + text-align: right; font-size: var(--size-xsmall); vertical-align: baseline; } @@ -36,10 +36,6 @@ line-height: 1.6rem; /* baseline alignment with the value */ } -.root .value.current { - padding-right: var(--space-xxsmall); -} - .jobName { white-space: nowrap; } @@ -52,9 +48,9 @@ } .headerRowTotals th { - padding-top: 0 !important; + padding-top: var(--space-xxxsmall) !important; font-weight: normal; - font-size: var(--size-xsmall); + font-size: var(--size-small); vertical-align: middle; } From 8931a6af74bfa2890d40c5fb7fe56d83bd339f97 Mon Sep 17 00:00:00 2001 From: Vio Date: Tue, 13 Jun 2023 23:26:26 +0200 Subject: [PATCH 2/5] enhance(ui): MetricsTable - show abs delta column --- .../metrics-table/metrics-table.jsx | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/ui/src/components/metrics-table/metrics-table.jsx b/packages/ui/src/components/metrics-table/metrics-table.jsx index b98dbdec4c..29b5b98d61 100644 --- a/packages/ui/src/components/metrics-table/metrics-table.jsx +++ b/packages/ui/src/components/metrics-table/metrics-table.jsx @@ -24,7 +24,7 @@ const getHeaderLabelCells = (rows) => (run, runIndex, runs) => { if (!run) { return [ - { children: '-', className, colSpan: isBaseline ? 1 : 2 }, + { children: '-', className, colSpan: isBaseline ? 1 : 3 }, ]; } @@ -45,7 +45,7 @@ const getHeaderLabelCells = (rows) => (run, runIndex, runs) => { { children: jobName, className, - colSpan: isBaseline ? 1 : 2, + colSpan: isBaseline ? 1 : 3, }, ]; }; @@ -71,12 +71,21 @@ const getHeaderTotalCells = (rows) => (run, runIndex, runs) => { { children: ( ), className: styles.delta, }, + { + children: ( + + ), + className: cx(styles.delta, styles.deltaPercentage), + }, ] : []), ]; @@ -121,11 +130,12 @@ const MetricsTableRow = ({ item, renderRowHeader }) => ( <> - {!isBaseline && } + {!isBaseline && } ); } - const { displayValue, deltaPercentage, displayDeltaPercentage, deltaType } = run; + const { displayValue, deltaPercentage, displayDelta, displayDeltaPercentage, deltaType } = run; return ( <> @@ -133,9 +143,14 @@ const MetricsTableRow = ({ item, renderRowHeader }) => ( {!isBaseline && typeof deltaPercentage === 'number' && ( - - - + <> + + + + + + + )} ); From 1851befd44854198fd41efa75065ea69b5fad8ab Mon Sep 17 00:00:00 2001 From: Vio Date: Mon, 10 Jul 2023 22:59:53 +0200 Subject: [PATCH 3/5] refactor(ui): MetricsTable - inline headers --- .../metrics-table/metrics-table.jsx | 172 +++++++----------- .../metrics-table/metrics-table.module.css | 16 +- .../metrics-table/metrics-table.stories.jsx | 4 + 3 files changed, 74 insertions(+), 118 deletions(-) diff --git a/packages/ui/src/components/metrics-table/metrics-table.jsx b/packages/ui/src/components/metrics-table/metrics-table.jsx index 29b5b98d61..bf332786b7 100644 --- a/packages/ui/src/components/metrics-table/metrics-table.jsx +++ b/packages/ui/src/components/metrics-table/metrics-table.jsx @@ -15,108 +15,68 @@ import styles from './metrics-table.module.css'; const METRIC_TYPE_DATA = getGlobalMetricType(null, METRIC_TYPE_FILE_SIZE); const VISIBLE_COUNT = 500; +const BASELINE_COLUMN_SPAN = 1; +const CURRENT_COLUMN_SPAN = 3; +const BASELINE_TITLE = 'Baseline'; +const CURRENT_TITLE = 'Current'; const getRowsRunTotal = (rows, runIndex) => sum(rows.map((row) => row?.runs?.[runIndex]?.value || 0)); -const getHeaderLabelCells = (rows) => (run, runIndex, runs) => { - const isBaseline = runIndex === runs.length - 1; - const className = cx(styles.value, isBaseline ? styles.baseline : styles.current); +const ColumnJob = ({ run, isBaseline }) => { + const colSpan = isBaseline ? BASELINE_COLUMN_SPAN : CURRENT_COLUMN_SPAN; if (!run) { - return [ - { children: '-', className, colSpan: isBaseline ? 1 : 3 }, - ]; + return ( + - + ); } const { label, internalBuildNumber } = run; - const jobName = ( - - {label} - + return ( + + + {label} + + ); - - // Value column - return [ - { - children: jobName, - className, - colSpan: isBaseline ? 1 : 3, - }, - ]; }; -const getHeaderTotalCells = (rows) => (run, runIndex, runs) => { - const isBaseline = runIndex === runs.length - 1; - const className = cx(styles.value, isBaseline ? styles.baseline : styles.current); - +const ColumnSum = ({ rows, isBaseline, runIndex }) => { const currentRunTotal = getRowsRunTotal(rows, runIndex); const baselineRunTotal = !isBaseline && getRowsRunTotal(rows, runIndex + 1); const infoTotal = getMetricRunInfo(METRIC_TYPE_DATA, currentRunTotal, baselineRunTotal); - return [ - // Value column - { - children: , - className, - }, - - // Delta column - ...(!isBaseline - ? [ - { - children: ( - - ), - className: styles.delta, - }, - { - children: ( - - ), - className: cx(styles.delta, styles.deltaPercentage), - }, - ] - : []), - ]; + return ( + <> + + + + {!isBaseline && ( + <> + + + + + + + + )} + + ); }; -const getHeaderRows = (runs, items, showHeaderSum, title) => [ - { - className: styles.headerRowColumns, - cells: [ - // Metric name column - one empty strying to render the column - { - children: title || ' ', - className: styles.metricName, - rowSpan: showHeaderSum ? 2 : 1, - }, - - // Runs - ...runs.map(getHeaderLabelCells(items)).flat(), - ], - }, - ...(showHeaderSum - ? [ - { - className: styles.headerRowTotals, - cells: runs.map(getHeaderTotalCells(items)).flat(), - }, - ] - : []), -]; - -const MetricsTableRow = ({ item, renderRowHeader }) => ( +const Row = ({ item, renderRowHeader }) => ( {renderRowHeader(item)} @@ -158,7 +118,7 @@ const MetricsTableRow = ({ item, renderRowHeader }) => ( ); -MetricsTableRow.propTypes = { +Row.propTypes = { item: PropTypes.shape({ changed: PropTypes.bool, runs: PropTypes.arrayOf( @@ -181,21 +141,13 @@ export const MetricsTable = ({ items, emptyMessage, showHeaderSum, - headerRows, + headerRows: parentHeaderRows, title, showAllItems, setShowAllItems, ...restProps }) => { - const { headers, columnClassNames } = useMemo(() => { - const headerColumns = getHeaderRows(runs, items, showHeaderSum, title); - - return { - headers: [...headerRows, ...headerColumns], - // First header row has the column class names - columnClassNames: headerColumns[0].cells.map((headerColumn) => headerColumn.className), - }; - }, [headerRows, runs, items, showHeaderSum, title]); + const columnCount = (runs.length - 1) * CURRENT_COLUMN_SPAN + BASELINE_COLUMN_SPAN + 1; const rootClassName = cx( styles.root, @@ -212,24 +164,22 @@ export const MetricsTable = ({ return ( - {headers.map((headerRow) => { - const { cells, className: rowClassName } = headerRow.cells - ? headerRow - : { cells: headerRow }; - - return ( - - {cells.map((header) => ( - - ))} - - ); - })} + + + {title || ' '} + + {runs.map((run, runIndex) => )} + + {showHeaderSum && ( + + {runs.map((run, runIndex) => )} + + )} {showEmpty && ( - +
@@ -243,12 +193,12 @@ export const MetricsTable = ({ {showItems && ( <> {visibleItems.map((item) => ( - + ))} {hasHiddenItems && ( - + {showAllItems ? (