From 2fcc82a95cc179c40d702e5e7cf396b295e5cbfa Mon Sep 17 00:00:00 2001 From: Ryan Vazquez Date: Tue, 10 Nov 2020 15:11:35 -0500 Subject: [PATCH] emphasize divider background use fixed title for project growth alert map product option to entity id in client remove conditional from test name update tooltip verbiage for unlabeled costs add default tooltip tooltip cleanup remove unused palette property --- plugins/cost-insights/src/client.ts | 4 +- .../src/components/BarChart/BarChart.tsx | 23 +++++++++- .../src/components/BarChart/helpers.tsx | 42 ------------------- .../CostGrowth/CostGrowthIndicator.test.tsx | 4 +- .../ProductInsightsChart.tsx | 6 +-- .../ProjectGrowthAlertChart.tsx | 11 +---- .../UnlabeledDataflowAlertCard.tsx | 7 +--- plugins/cost-insights/src/types/Theme.ts | 1 - plugins/cost-insights/src/utils/styles.ts | 5 +-- 9 files changed, 33 insertions(+), 70 deletions(-) delete mode 100644 plugins/cost-insights/src/components/BarChart/helpers.tsx diff --git a/plugins/cost-insights/src/client.ts b/plugins/cost-insights/src/client.ts index 50e925bc28..740a44fcbd 100644 --- a/plugins/cost-insights/src/client.ts +++ b/plugins/cost-insights/src/client.ts @@ -196,7 +196,7 @@ export class ExampleCostInsightsClient implements CostInsightsApi { productInsightsOptions: ProductInsightsOptions, ): Promise { const projectProductInsights = await this.request(productInsightsOptions, { - id: 'project-product', + id: productInsightsOptions.product, aggregation: [80_000, 110_000], change: { ratio: 0.375, @@ -243,7 +243,7 @@ export class ExampleCostInsightsClient implements CostInsightsApi { }); const productInsights: Entity = await this.request(productInsightsOptions, { - id: 'product', + id: productInsightsOptions.product, aggregation: [200_000, 250_000], change: { ratio: 0.2, diff --git a/plugins/cost-insights/src/components/BarChart/BarChart.tsx b/plugins/cost-insights/src/components/BarChart/BarChart.tsx index 30e8ad71f8..93f738a38b 100644 --- a/plugins/cost-insights/src/components/BarChart/BarChart.tsx +++ b/plugins/cost-insights/src/components/BarChart/BarChart.tsx @@ -30,6 +30,8 @@ import { import { Box, useTheme } from '@material-ui/core'; import { BarChartTick } from './BarChartTick'; import { BarChartStepper } from './BarChartStepper'; +import { BarChartTooltip } from './BarChartTooltip'; +import { BarChartTooltipItem } from './BarChartTooltipItem'; import { currencyFormatter } from '../../utils/formatters'; import { BarChartData, @@ -37,8 +39,27 @@ import { DataKey, CostInsightsTheme, } from '../../types'; +import { notEmpty } from '../../utils/assert'; import { useBarChartStyles } from '../../utils/styles'; import { resourceSort } from '../../utils/sort'; +import { isInvalid, titleOf, tooltipItemOf } from '../../utils/graphs'; + +export const defaultTooltip: ContentRenderer = ({ + label, + payload = [], +}) => { + if (isInvalid({ label, payload })) return null; + + const title = titleOf(label); + const items = payload.map(tooltipItemOf).filter(notEmpty); + return ( + + {items.map((item, index) => ( + + ))} + + ); +}; export type BarChartProps = { resources: ResourceData[]; @@ -55,7 +76,7 @@ export const BarChart = ({ responsive = true, displayAmount = 6, options = {}, - tooltip, + tooltip = defaultTooltip, onClick, onMouseMove, }: BarChartProps) => { diff --git a/plugins/cost-insights/src/components/BarChart/helpers.tsx b/plugins/cost-insights/src/components/BarChart/helpers.tsx deleted file mode 100644 index 66389af771..0000000000 --- a/plugins/cost-insights/src/components/BarChart/helpers.tsx +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import React from 'react'; -import { - ContentRenderer, - TooltipProps as RechartsTooltipProps, -} from 'recharts'; -import { BarChartTooltip } from './BarChartTooltip'; -import { BarChartTooltipItem } from './BarChartTooltipItem'; -import { notEmpty } from '../../utils/assert'; -import { isInvalid, titleOf, tooltipItemOf } from '../../utils/graphs'; - -export const renderDefaultTooltip: ContentRenderer = ({ - label, - payload = [], -}) => { - if (isInvalid({ label, payload })) return null; - - const title = titleOf(label); - const items = payload.map(tooltipItemOf).filter(notEmpty); - return ( - - {items.map((item, index) => ( - - ))} - - ); -}; diff --git a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.test.tsx b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.test.tsx index 5aabb09c7d..8be5e238cc 100644 --- a/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.test.tsx +++ b/plugins/cost-insights/src/components/CostGrowth/CostGrowthIndicator.test.tsx @@ -30,9 +30,7 @@ describe.each` ${ChangeThreshold.upper + 0.01} | ${EngineerThreshold} | ${'excess'} ${ChangeThreshold.upper + 0.01} | ${EngineerThreshold + 0.1} | ${'excess'} `('growthOf', ({ ratio, amount, ariaLabel }) => { - it(`should display the correct indicator for ${ - ariaLabel ? ariaLabel : 'negligible' - }`, async () => { + it(`should display the correct indicator for ${ariaLabel}`, async () => { const { getByLabelText } = await renderInTestApp( , ); diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsChart.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsChart.tsx index 1392b05b34..6050dae9c7 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsChart.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsChart.tsx @@ -144,7 +144,7 @@ export const ProductInsightsChart = ({ const activeEntity = findAlways(entity.entities, e => e.id === id); const ratio = activeEntity.change.ratio; const skus = activeEntity.entities; - const subtitle = `${skus.length} ${pluralOf(skus.length, 'SKU', 'SKUs')}`; + const subtitle = `${skus.length} ${pluralOf(skus.length, 'SKU')}`; if (skus.length) { return ( @@ -179,14 +179,14 @@ export const ProductInsightsChart = ({ topRight={ } content={ id ? null - : `This service is generating costs that can't be attributed to any known entity.` + : "This product has costs that are not labeled and therefore can't be attributed to a specific entity." } > {items.map((item, index) => ( diff --git a/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertChart.tsx b/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertChart.tsx index dffed24992..53e52f18e8 100644 --- a/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertChart.tsx +++ b/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertChart.tsx @@ -20,7 +20,6 @@ import { Box } from '@material-ui/core'; import { BarChart, BarChartLegend } from '../BarChart'; import { LegendItem } from '../LegendItem'; import { CostGrowth } from '../CostGrowth'; -import { renderDefaultTooltip } from '../BarChart/helpers'; import { BarChartOptions, Duration, ProjectGrowthData } from '../../types'; import { useBarChartLayoutStyles as useStyles } from '../../utils/styles'; import { resourceOf } from '../../utils/graphs'; @@ -46,17 +45,11 @@ export const ProjectGrowthAlertChart = ({ return ( - + - + ); }; diff --git a/plugins/cost-insights/src/components/UnlabeledDataflowAlertCard/UnlabeledDataflowAlertCard.tsx b/plugins/cost-insights/src/components/UnlabeledDataflowAlertCard/UnlabeledDataflowAlertCard.tsx index 5384db566c..e56b3b044e 100644 --- a/plugins/cost-insights/src/components/UnlabeledDataflowAlertCard/UnlabeledDataflowAlertCard.tsx +++ b/plugins/cost-insights/src/components/UnlabeledDataflowAlertCard/UnlabeledDataflowAlertCard.tsx @@ -18,7 +18,6 @@ import React from 'react'; import { InfoCard } from '@backstage/core'; import { Box } from '@material-ui/core'; import { BarChart, BarChartLegend } from '../BarChart'; -import { renderDefaultTooltip } from '../BarChart/helpers'; import { UnlabeledDataflowData, ResourceData } from '../../types'; import { pluralOf } from '../../utils/grammar'; import { useBarChartLayoutStyles as useStyles } from '../../utils/styles'; @@ -54,11 +53,7 @@ export const UnlabeledDataflowAlertCard = ({ costEnd={alert.unlabeledCost} options={options} /> - + ); diff --git a/plugins/cost-insights/src/types/Theme.ts b/plugins/cost-insights/src/types/Theme.ts index fecaa17abb..9053283351 100644 --- a/plugins/cost-insights/src/types/Theme.ts +++ b/plugins/cost-insights/src/types/Theme.ts @@ -30,7 +30,6 @@ type CostInsightsPaletteAdditions = { tooltip: CostInsightsTooltipOptions; navigationText: string; alertBackground: string; - textSecondary: string; }; export type CostInsightsPalette = BackstagePalette & diff --git a/plugins/cost-insights/src/utils/styles.ts b/plugins/cost-insights/src/utils/styles.ts index 4f8ac7fa98..de743d8459 100644 --- a/plugins/cost-insights/src/utils/styles.ts +++ b/plugins/cost-insights/src/utils/styles.ts @@ -15,6 +15,7 @@ */ import { createStyles, + emphasize, darken, getLuminance, lighten, @@ -400,9 +401,7 @@ export const useTooltipStyles = makeStyles( fontSize: `.75rem`, }, divider: { - /* TODO: tooltip divider color should be the inverse of theme. - Dark mode should appear dark, light mode should appear light */ - backgroundColor: darken(theme.palette.divider, 1), + backgroundColor: emphasize(theme.palette.divider, 1), }, subtitle: { fontStyle: 'italic',