From 08562ebe1117c0510b0b6cf35dcacf0c6cbccd67 Mon Sep 17 00:00:00 2001 From: kielosz Date: Mon, 8 Aug 2022 16:05:43 +0200 Subject: [PATCH 1/2] Display minus sign in trends Signed-off-by: kielosz --- .changeset/popular-ears-allow.md | 5 +++++ plugins/cost-insights/api-report.md | 5 ++++- .../components/CostGrowth/CostGrowthIndicator.tsx | 7 +++++-- .../CostOverviewCard/CostOverviewLegend.tsx | 4 ++-- .../ProductInsightsCard/ProductEntityTable.tsx | 10 ++++++++-- plugins/cost-insights/src/utils/formatters.test.ts | 3 +++ plugins/cost-insights/src/utils/formatters.ts | 12 ++++++++++-- 7 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 .changeset/popular-ears-allow.md diff --git a/.changeset/popular-ears-allow.md b/.changeset/popular-ears-allow.md new file mode 100644 index 0000000000..1af2fcbd81 --- /dev/null +++ b/.changeset/popular-ears-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-cost-insights': patch +--- + +Display minus sign in trends in `CostOverviewCard` diff --git a/plugins/cost-insights/api-report.md b/plugins/cost-insights/api-report.md index 94cab949c7..19d94c7058 100644 --- a/plugins/cost-insights/api-report.md +++ b/plugins/cost-insights/api-report.md @@ -320,7 +320,10 @@ export const CostGrowthIndicator: ({ // @public (undocumented) export type CostGrowthIndicatorProps = TypographyProps & { change: ChangeStatistic; - formatter?: (change: ChangeStatistic) => Maybe; + formatter?: ( + change: ChangeStatistic, + returnAbsoluteValue: boolean, + ) => Maybe; }; // Warning: (ae-missing-release-tag) "CostGrowthProps" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx index 592f57d389..bb303956a3 100644 --- a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx +++ b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx @@ -25,7 +25,10 @@ import { useCostGrowthStyles as useStyles } from '../../utils/styles'; export type CostGrowthIndicatorProps = TypographyProps & { change: ChangeStatistic; - formatter?: (change: ChangeStatistic) => Maybe; + formatter?: ( + change: ChangeStatistic, + returnAbsoluteValue: boolean, + ) => Maybe; }; export const CostGrowthIndicator = ({ @@ -44,7 +47,7 @@ export const CostGrowthIndicator = ({ return ( - {formatter ? formatter(change) : change.ratio} + {formatter ? formatter(change, true) : change.ratio} {growth === GrowthType.Excess && } {growth === GrowthType.Savings && } diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx index 2df80f4780..ddd31201a6 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx @@ -59,7 +59,7 @@ export const CostOverviewLegend = ({ {dailyCostData.change && ( - {formatChange(dailyCostData.change)} + {formatChange(dailyCostData.change, false)} )} @@ -70,7 +70,7 @@ export const CostOverviewLegend = ({ title={`${metric.name} Trend`} markerColor={theme.palette.magenta} > - {formatChange(metricData.change)} + {formatChange(metricData.change, false)} ) { if (b.id === 'total') return 1; if (field === 'label') return a.label.localeCompare(b.label); if (field === 'change') { - if (formatChange(a[field]) === '∞' || formatChange(b[field]) === '-∞') + if ( + formatChange(a[field], false) === '∞' || + formatChange(b[field], false) === '-∞' + ) return 1; - if (formatChange(a[field]) === '-∞' || formatChange(b[field]) === '∞') + if ( + formatChange(a[field], false) === '-∞' || + formatChange(b[field], false) === '∞' + ) return -1; return a[field].ratio! - b[field].ratio!; } diff --git a/plugins/cost-insights/src/utils/formatters.test.ts b/plugins/cost-insights/src/utils/formatters.test.ts index 9fe17f277f..754760e5cb 100644 --- a/plugins/cost-insights/src/utils/formatters.test.ts +++ b/plugins/cost-insights/src/utils/formatters.test.ts @@ -73,6 +73,9 @@ describe.each` ${0.123123} | ${'12%'} ${1.123} | ${'112%'} ${10.123} | ${'>1000%'} + ${-0.123123} | ${'-12%'} + ${-1.123} | ${'-112%'} + ${-10.123} | ${'>1000%'} `('formatPercent', ({ ratio, expected }) => { it(`correctly formats ${ratio} as ${expected}`, () => { expect(formatPercent(ratio)).toBe(expected); diff --git a/plugins/cost-insights/src/utils/formatters.ts b/plugins/cost-insights/src/utils/formatters.ts index 70782123cd..e66b6bc75c 100644 --- a/plugins/cost-insights/src/utils/formatters.ts +++ b/plugins/cost-insights/src/utils/formatters.ts @@ -81,9 +81,17 @@ export function formatCurrency(amount: number, currency?: string): string { return currency ? `${numString} ${pluralize(currency, n)}` : numString; } -export function formatChange(change: ChangeStatistic): string { +export function formatChange( + change: ChangeStatistic, + returnAbsoluteValue: boolean, +): string { if (notEmpty(change.ratio)) { - return formatPercent(Math.abs(change.ratio)); + return formatPercent( + returnAbsoluteValue ? Math.abs(change.ratio) : change.ratio, + ); + } + if (returnAbsoluteValue) { + return '∞'; } return change.amount >= 0 ? '∞' : '-∞'; } From 0c027eb524be00bccf349880038f712a314349b3 Mon Sep 17 00:00:00 2001 From: kielosz Date: Wed, 10 Aug 2022 17:51:07 +0200 Subject: [PATCH 2/2] Use options parameter Signed-off-by: kielosz --- plugins/cost-insights/api-report.md | 4 +++- .../src/components/CostGrowth/CostGrowthIndicator.tsx | 4 ++-- .../components/CostOverviewCard/CostOverviewLegend.tsx | 4 ++-- .../ProductInsightsCard/ProductEntityTable.tsx | 10 ++-------- plugins/cost-insights/src/utils/formatters.test.ts | 2 +- plugins/cost-insights/src/utils/formatters.ts | 8 ++++---- 6 files changed, 14 insertions(+), 18 deletions(-) diff --git a/plugins/cost-insights/api-report.md b/plugins/cost-insights/api-report.md index 19d94c7058..2a4cecc4eb 100644 --- a/plugins/cost-insights/api-report.md +++ b/plugins/cost-insights/api-report.md @@ -322,7 +322,9 @@ export type CostGrowthIndicatorProps = TypographyProps & { change: ChangeStatistic; formatter?: ( change: ChangeStatistic, - returnAbsoluteValue: boolean, + options?: { + absolute: boolean; + }, ) => Maybe; }; diff --git a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx index bb303956a3..e7746495d6 100644 --- a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx +++ b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.tsx @@ -27,7 +27,7 @@ export type CostGrowthIndicatorProps = TypographyProps & { change: ChangeStatistic; formatter?: ( change: ChangeStatistic, - returnAbsoluteValue: boolean, + options?: { absolute: boolean }, ) => Maybe; }; @@ -47,7 +47,7 @@ export const CostGrowthIndicator = ({ return ( - {formatter ? formatter(change, true) : change.ratio} + {formatter ? formatter(change, { absolute: true }) : change.ratio} {growth === GrowthType.Excess && } {growth === GrowthType.Savings && } diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx index ddd31201a6..2df80f4780 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewLegend.tsx @@ -59,7 +59,7 @@ export const CostOverviewLegend = ({ {dailyCostData.change && ( - {formatChange(dailyCostData.change, false)} + {formatChange(dailyCostData.change)} )} @@ -70,7 +70,7 @@ export const CostOverviewLegend = ({ title={`${metric.name} Trend`} markerColor={theme.palette.magenta} > - {formatChange(metricData.change, false)} + {formatChange(metricData.change)} ) { if (b.id === 'total') return 1; if (field === 'label') return a.label.localeCompare(b.label); if (field === 'change') { - if ( - formatChange(a[field], false) === '∞' || - formatChange(b[field], false) === '-∞' - ) + if (formatChange(a[field]) === '∞' || formatChange(b[field]) === '-∞') return 1; - if ( - formatChange(a[field], false) === '-∞' || - formatChange(b[field], false) === '∞' - ) + if (formatChange(a[field]) === '-∞' || formatChange(b[field]) === '∞') return -1; return a[field].ratio! - b[field].ratio!; } diff --git a/plugins/cost-insights/src/utils/formatters.test.ts b/plugins/cost-insights/src/utils/formatters.test.ts index 754760e5cb..8bf0a6a9d7 100644 --- a/plugins/cost-insights/src/utils/formatters.test.ts +++ b/plugins/cost-insights/src/utils/formatters.test.ts @@ -75,7 +75,7 @@ describe.each` ${10.123} | ${'>1000%'} ${-0.123123} | ${'-12%'} ${-1.123} | ${'-112%'} - ${-10.123} | ${'>1000%'} + ${-10.123} | ${'>-1000%'} `('formatPercent', ({ ratio, expected }) => { it(`correctly formats ${ratio} as ${expected}`, () => { expect(formatPercent(ratio)).toBe(expected); diff --git a/plugins/cost-insights/src/utils/formatters.ts b/plugins/cost-insights/src/utils/formatters.ts index e66b6bc75c..e9b19097a4 100644 --- a/plugins/cost-insights/src/utils/formatters.ts +++ b/plugins/cost-insights/src/utils/formatters.ts @@ -83,14 +83,14 @@ export function formatCurrency(amount: number, currency?: string): string { export function formatChange( change: ChangeStatistic, - returnAbsoluteValue: boolean, + options?: { absolute: boolean }, ): string { if (notEmpty(change.ratio)) { return formatPercent( - returnAbsoluteValue ? Math.abs(change.ratio) : change.ratio, + options?.absolute ? Math.abs(change.ratio) : change.ratio, ); } - if (returnAbsoluteValue) { + if (options?.absolute) { return '∞'; } return change.amount >= 0 ? '∞' : '-∞'; @@ -103,7 +103,7 @@ export function formatPercent(n: number): string { } if (Math.abs(n) > 10) { - return `>1000%`; + return `>${n < 0 ? '-' : ''}1000%`; } return `${(n * 100).toFixed(0)}%`;