From 39d7ebc7d13cb05d21bfa877bb034c7e408625fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 9 Feb 2021 14:18:08 +0100 Subject: [PATCH] BarGauge: Improvements to value sizing and table inner width calculations (#30990) * BarGauge: Increase min value width and fix height when setting manual text size * updated snapshot * Big improvement to bar gauge value sizing and fixing table gauge sizing * removed unused const * added a unit test --- .../src/components/BarGauge/BarGauge.test.tsx | 15 +++++++++++ .../src/components/BarGauge/BarGauge.tsx | 27 ++++++++++++------- .../__snapshots__/BarGauge.test.tsx.snap | 1 + .../src/components/Table/BarGaugeCell.tsx | 11 ++------ .../grafana-ui/src/components/Table/Table.tsx | 2 ++ .../src/components/Table/TableCell.tsx | 12 ++++++++- .../grafana-ui/src/components/Table/styles.ts | 8 +++--- .../grafana-ui/src/components/Table/types.ts | 1 + packages/grafana-ui/src/utils/scrollbar.ts | 1 + 9 files changed, 53 insertions(+), 25 deletions(-) diff --git a/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx b/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx index 251f2ce9d4a..7eaa5dbe45d 100644 --- a/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx +++ b/packages/grafana-ui/src/components/BarGauge/BarGauge.test.tsx @@ -10,6 +10,7 @@ import { getTitleStyles, getValuePercent, BarGaugeDisplayMode, + calculateBarAndValueDimensions, } from './BarGauge'; import { getTheme } from '../../themes'; @@ -211,4 +212,18 @@ describe('BarGauge', () => { expect(wrapper).toMatchSnapshot(); }); }); + + describe('calculateBarAndValueDimensions', () => { + it('valueWidth should including paddings in valueWidth', () => { + const result = calculateBarAndValueDimensions( + getProps({ + height: 30, + width: 100, + value: getValue(1, 'AA'), + orientation: VizOrientation.Horizontal, + }) + ); + expect(result.valueWidth).toBe(21); + }); + }); }); diff --git a/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx b/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx index e8d0ca0735b..34411ff6281 100644 --- a/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx +++ b/packages/grafana-ui/src/components/BarGauge/BarGauge.tsx @@ -30,7 +30,6 @@ import { Themeable } from '../../types'; const MIN_VALUE_HEIGHT = 18; const MAX_VALUE_HEIGHT = 50; -const MIN_VALUE_WIDTH = 50; const MAX_VALUE_WIDTH = 150; const TITLE_LINE_HEIGHT = 1.5; const VALUE_LINE_HEIGHT = 1; @@ -373,9 +372,15 @@ interface BarAndValueDimensions { wrapperWidth: number; } -function calculateBarAndValueDimensions(props: Props): BarAndValueDimensions { - const { height, width, orientation, text } = props; +/** + * @internal + * Only exported for unit tests + **/ +export function calculateBarAndValueDimensions(props: Props): BarAndValueDimensions { + const { height, width, orientation, text, alignmentFactors } = props; const titleDim = calculateTitleDimensions(props); + const value = alignmentFactors ?? props.value; + const valueString = formattedValueToString(value); let maxBarHeight = 0; let maxBarWidth = 0; @@ -384,25 +389,27 @@ function calculateBarAndValueDimensions(props: Props): BarAndValueDimensions { let wrapperWidth = 0; let wrapperHeight = 0; + // measure text with title font size or min 14px + const fontSizeToMeasureWith = text?.valueSize ?? Math.max(titleDim.fontSize, 12); + const realTextSize = measureText(valueString, fontSizeToMeasureWith); + const realValueWidth = realTextSize.width + VALUE_LEFT_PADDING * 2; + if (isVertical(orientation)) { if (text?.valueSize) { valueHeight = text.valueSize * VALUE_LINE_HEIGHT; } else { valueHeight = Math.min(Math.max(height * 0.1, MIN_VALUE_HEIGHT), MAX_VALUE_HEIGHT); } + valueWidth = width; maxBarHeight = height - (titleDim.height + valueHeight); maxBarWidth = width; wrapperWidth = width; wrapperHeight = height - titleDim.height; } else { - if (text?.valueSize) { - valueHeight = text.valueSize * VALUE_LINE_HEIGHT; - } else { - valueHeight = height - titleDim.height; - } + valueHeight = height - titleDim.height; + valueWidth = Math.max(Math.min(width * 0.2, MAX_VALUE_WIDTH), realValueWidth); - valueWidth = Math.max(Math.min(width * 0.2, MAX_VALUE_WIDTH), MIN_VALUE_WIDTH); maxBarHeight = height - titleDim.height; maxBarWidth = width - valueWidth - titleDim.width; @@ -479,7 +486,6 @@ export function getBasicAndGradientStyles(props: Props): BasicAndGradientStyles if (isBasic) { // Basic styles barStyles.background = `${tinycolor(valueColor).setAlpha(0.35).toRgbString()}`; - barStyles.borderTop = `2px solid ${valueColor}`; } else { // Gradient styles @@ -499,6 +505,7 @@ export function getBasicAndGradientStyles(props: Props): BasicAndGradientStyles // shift empty region back to fill gaps due to border radius emptyBar.left = '-3px'; + emptyBar.width = `${maxBarWidth - barWidth}px`; if (isBasic) { // Basic styles diff --git a/packages/grafana-ui/src/components/BarGauge/__snapshots__/BarGauge.test.tsx.snap b/packages/grafana-ui/src/components/BarGauge/__snapshots__/BarGauge.test.tsx.snap index cb2c15faf80..eed60db60c9 100644 --- a/packages/grafana-ui/src/components/BarGauge/__snapshots__/BarGauge.test.tsx.snap +++ b/packages/grafana-ui/src/components/BarGauge/__snapshots__/BarGauge.test.tsx.snap @@ -62,6 +62,7 @@ exports[`BarGauge Render with basic options should render 1`] = ` "flexGrow": 1, "left": "-3px", "position": "relative", + "width": "180px", } } /> diff --git a/packages/grafana-ui/src/components/Table/BarGaugeCell.tsx b/packages/grafana-ui/src/components/Table/BarGaugeCell.tsx index 653ee7c5cb6..421b85e8fae 100644 --- a/packages/grafana-ui/src/components/Table/BarGaugeCell.tsx +++ b/packages/grafana-ui/src/components/Table/BarGaugeCell.tsx @@ -19,7 +19,7 @@ const defaultScale: ThresholdsConfig = { }; export const BarGaugeCell: FC = (props) => { - const { field, column, tableStyles, cell, cellProps } = props; + const { field, innerWidth, tableStyles, cell, cellProps } = props; let config = getFieldConfigWithMinMax(field, false); if (!config.thresholds) { @@ -38,17 +38,10 @@ export const BarGaugeCell: FC = (props) => { barGaugeMode = BarGaugeDisplayMode.Basic; } - let width; - if (column.width) { - width = (column.width as number) - tableStyles.cellPadding * 2; - } else { - width = tableStyles.cellPadding * 2; - } - return (
= memo((props: Props) => { tableStyles={tableStyles} cell={cell} onCellFilterAdded={onCellFilterAdded} + columnIndex={index} + columnCount={row.cells.length} /> ))}
diff --git a/packages/grafana-ui/src/components/Table/TableCell.tsx b/packages/grafana-ui/src/components/Table/TableCell.tsx index 8a089fc3c51..c2a400149d1 100644 --- a/packages/grafana-ui/src/components/Table/TableCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableCell.tsx @@ -9,9 +9,11 @@ export interface Props { field: Field; tableStyles: TableStyles; onCellFilterAdded?: TableFilterActionCallback; + columnIndex: number; + columnCount: number; } -export const TableCell: FC = ({ cell, field, tableStyles, onCellFilterAdded }) => { +export const TableCell: FC = ({ cell, field, tableStyles, onCellFilterAdded, columnIndex, columnCount }) => { const cellProps = cell.getCellProps(); if (!field.display) { @@ -23,6 +25,13 @@ export const TableCell: FC = ({ cell, field, tableStyles, onCellFilterAdd cellProps.style.justifyContent = (cell.column as any).justifyContent; } + let innerWidth = ((cell.column.width as number) ?? 24) - tableStyles.cellPadding * 2; + + // last child sometimes have extra padding if there is a non overlay scrollbar + if (columnIndex === columnCount - 1) { + innerWidth -= tableStyles.lastChildExtraPadding; + } + return ( <> {cell.render('Cell', { @@ -30,6 +39,7 @@ export const TableCell: FC = ({ cell, field, tableStyles, onCellFilterAdd tableStyles, onCellFilterAdded, cellProps, + innerWidth, })} ); diff --git a/packages/grafana-ui/src/components/Table/styles.ts b/packages/grafana-ui/src/components/Table/styles.ts index a3a3fd22d88..02f43620116 100644 --- a/packages/grafana-ui/src/components/Table/styles.ts +++ b/packages/grafana-ui/src/components/Table/styles.ts @@ -13,7 +13,7 @@ export const getTableStyles = stylesFactory((theme: GrafanaTheme) => { const bodyFontSize = 14; const cellHeight = cellPadding * 2 + bodyFontSize * lineHeight; const rowHoverBg = styleMixins.hoverColor(theme.colors.bg1, theme); - const scollbarWidth = getScrollbarWidth(); + const lastChildExtraPadding = Math.max(getScrollbarWidth(), cellPadding); const buildCellContainerStyle = (color?: string, background?: string) => { return css` @@ -29,10 +29,7 @@ export const getTableStyles = stylesFactory((theme: GrafanaTheme) => { &:last-child { border-right: none; - - > div { - padding-right: ${scollbarWidth + cellPadding}px; - } + padding-right: ${lastChildExtraPadding}px; } &:hover { @@ -54,6 +51,7 @@ export const getTableStyles = stylesFactory((theme: GrafanaTheme) => { cellHeight, buildCellContainerStyle, cellPadding, + lastChildExtraPadding, cellHeightInner: bodyFontSize * lineHeight, rowHeight: cellHeight + 2, table: css` diff --git a/packages/grafana-ui/src/components/Table/types.ts b/packages/grafana-ui/src/components/Table/types.ts index 8fc7702c8ea..8ab7ae71876 100644 --- a/packages/grafana-ui/src/components/Table/types.ts +++ b/packages/grafana-ui/src/components/Table/types.ts @@ -45,6 +45,7 @@ export interface TableCellProps extends CellProps { cellProps: CSSProperties; field: Field; onCellFilterAdded: TableFilterActionCallback; + innerWidth: number; } export type CellComponent = FC; diff --git a/packages/grafana-ui/src/utils/scrollbar.ts b/packages/grafana-ui/src/utils/scrollbar.ts index 55bd84f62e9..21a07ec99aa 100644 --- a/packages/grafana-ui/src/utils/scrollbar.ts +++ b/packages/grafana-ui/src/utils/scrollbar.ts @@ -29,5 +29,6 @@ export function getScrollbarWidth() { } else { scrollbarWidth = 0; } + return scrollbarWidth || 0; }