From af5e599dab13c0f07468ee3da6ac8bab15bc0fc8 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Mon, 3 Feb 2025 20:32:45 -0700 Subject: [PATCH] make a chart card --- .../api/asset_interfaces/metric/defaults.ts | 3 +- .../MetricViewChart/MetricChartEvaluation.tsx | 2 +- .../MetricViewChart/MetricViewChart.tsx | 24 ++++++-- .../MetricViewChartContent.tsx | 25 ++++++++- .../test/chartjs/chartjsundefined/page.tsx | 4 +- web/src/app/test/chartjs/page.tsx | 4 +- web/src/components/charts/BusterChart.tsx | 14 ++--- .../charts/BusterChartErrorWrapper.tsx | 4 +- .../components/charts/BusterChartWrapper.tsx | 18 +----- web/src/components/charts/config.tsx | 15 +---- web/src/components/charts/helpers.ts | 4 +- .../interfaces/chartComponentInterfaces.ts | 1 - .../charts/interfaces/chartConfigProps.ts | 3 +- web/src/components/charts/interfaces/enum.ts | 5 -- .../MetricData/BusterMetricDataProvider.tsx | 14 +++-- web/src/context/MetricData/MOCK_DATA.ts | 11 +++- .../context/Metrics/BusterMetricsProvider.tsx | 10 ++-- web/src/context/Metrics/MOCK_METRIC.ts | 55 ++++++++++++++++++- .../messageAutoChartHandler.spec.ts | 1 - web/src/utils/imageGeneration.tsx | 5 -- 20 files changed, 133 insertions(+), 89 deletions(-) diff --git a/web/src/api/asset_interfaces/metric/defaults.ts b/web/src/api/asset_interfaces/metric/defaults.ts index 43e4071f4..dcabefdac 100644 --- a/web/src/api/asset_interfaces/metric/defaults.ts +++ b/web/src/api/asset_interfaces/metric/defaults.ts @@ -1,6 +1,6 @@ import type { IBusterMetricChartConfig } from './requireInterfaces'; import type { ColumnSettings } from '../../../components/charts/interfaces/columnInterfaces'; -import { ChartType, ViewType } from '../../../components/charts/interfaces/enum'; +import { ChartType } from '../../../components/charts/interfaces/enum'; import { DEFAULT_CHART_THEME } from '../../../components/charts/configColors'; import type { ColumnLabelFormat } from '../../../components/charts/interfaces/columnLabelInterfaces'; import type { ColumnMetaData } from './interfaces'; @@ -8,7 +8,6 @@ import type { ColumnMetaData } from './interfaces'; export const DEFAULT_CHART_CONFIG: IBusterMetricChartConfig = { colors: DEFAULT_CHART_THEME, selectedChartType: ChartType.Table, - selectedView: ViewType.Table, yAxisShowAxisLabel: true, yAxisShowAxisTitle: true, yAxisAxisTitle: null, diff --git a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricChartEvaluation.tsx b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricChartEvaluation.tsx index ec6d469c7..c33f4ecbb 100644 --- a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricChartEvaluation.tsx +++ b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricChartEvaluation.tsx @@ -40,7 +40,7 @@ export const MetricChartEvaluation: React.FC<{ className={cx( styles.container, colorClass, - 'flex items-center gap-1 rounded-lg px-2 py-1' + 'flex items-center gap-1 rounded-lg px-2 py-1 hover:shadow-sm' )}> {icon} {text} diff --git a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChart.tsx b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChart.tsx index 8921e6715..0ddaf1bee 100644 --- a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChart.tsx +++ b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChart.tsx @@ -1,11 +1,12 @@ import { createStyles } from 'antd-style'; -import React from 'react'; +import React, { useMemo } from 'react'; import { MetricViewChartContent } from './MetricViewChartContent'; import { MetricViewChartHeader } from './MetricViewChartHeader'; import { useBusterMetricIndividual, useBusterMetricsContextSelector } from '@/context/Metrics'; import { useMemoizedFn } from 'ahooks'; import { inputHasText } from '@/utils/text'; import { MetricChartEvaluation } from './MetricChartEvaluation'; +import { ChartType } from '@/components/charts/interfaces/enum'; export const MetricViewChart: React.FC<{ metricId: string; @@ -14,6 +15,16 @@ export const MetricViewChart: React.FC<{ const onUpdateMetric = useBusterMetricsContextSelector((x) => x.onUpdateMetric); const { metric, metricData } = useBusterMetricIndividual({ metricId }); const { title, description, time_frame, evaluation_score, evaluation_summary } = metric; + const isTable = metric.chart_config.selectedChartType === ChartType.Table; + + const loadingData = !metricData.fetched; + const errorData = !!metricData.error; + + const cardClass = useMemo(() => { + if (loadingData || errorData) return 'h-full max-h-[600px]'; + if (isTable) return ''; + return 'h-full max-h-[600px]'; + }, [isTable, loadingData, errorData]); const onSetTitle = useMemoizedFn((title: string) => { if (inputHasText(title)) { @@ -24,8 +35,13 @@ export const MetricViewChart: React.FC<{ }); return ( -
-
+
+
({ border-radius: ${token.borderRadiusLG}px; border: 0.5px solid ${token.colorBorder}; background-color: ${token.colorBgContainer}; + overflow: hidden; `, divider: css` border-bottom: 0.5px solid ${token.colorBorder}; diff --git a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChartContent.tsx b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChartContent.tsx index 914365c3b..f5fe834db 100644 --- a/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChartContent.tsx +++ b/web/src/app/app/_controllers/MetricController/MetricViewController/MetricViewChart/MetricViewChartContent.tsx @@ -1,7 +1,8 @@ import type { DataMetadata } from '@/api/asset_interfaces'; +import { BusterChart, ChartType } from '@/components/charts'; import type { BusterMetricData, IBusterMetric } from '@/context/Metrics'; import { createStyles } from 'antd-style'; -import React from 'react'; +import React, { useMemo } from 'react'; interface MetricViewChartContentProps { className?: string; @@ -13,9 +14,27 @@ interface MetricViewChartContentProps { } export const MetricViewChartContent: React.FC = React.memo( - ({ className, chartConfig, metricData, dataMetadata, fetchedData, errorMessage }) => { + ({ className, chartConfig, metricData = null, dataMetadata, fetchedData, errorMessage }) => { const { styles, cx } = useStyles(); - return
MetricViewChartContent
; + const columnMetadata = dataMetadata?.column_metadata; + const isTable = chartConfig?.selectedChartType === ChartType.Table; + + const cardClassName = useMemo(() => { + if (isTable || !fetchedData) return ''; + return 'p-4'; + }, [isTable, fetchedData]); + + return ( +
+ +
+ ); } ); diff --git a/web/src/app/test/chartjs/chartjsundefined/page.tsx b/web/src/app/test/chartjs/chartjsundefined/page.tsx index b0b6cfa4f..7ed975d1d 100644 --- a/web/src/app/test/chartjs/chartjsundefined/page.tsx +++ b/web/src/app/test/chartjs/chartjsundefined/page.tsx @@ -6,8 +6,7 @@ import { BusterChartProps, ChartType, IColumnLabelFormat, - Trendline, - ViewType + Trendline } from '@/components/charts'; import { faker } from '@faker-js/faker'; import { Checkbox, Select } from 'antd'; @@ -150,7 +149,6 @@ export default function ChartJS() { = React.memo( onInitialAnimationEnd, editable, selectedChartType, - selectedView, columnLabelFormats = DEFAULT_CHART_CONFIG.columnLabelFormats, renderType = 'chartjs', ...props }) => { - const isTable = selectedView === ViewType.Table || selectedChartType === ChartType.Table; + const isTable = selectedChartType === ChartType.Table; const showNoData = !loading && (isEmpty(data) || data === null); const selectedAxis: ChartEncodes | undefined = useMemo(() => { @@ -73,7 +72,7 @@ export const BusterChart: React.FC = React.memo( selectedAxis, isTable }); - }, [selectedChartType, selectedView, isTable, selectedAxis]); + }, [selectedChartType, isTable, selectedAxis]); const onChartMounted = useMemoizedFn((chart?: any) => { onChartMountedProp?.(chart); @@ -152,12 +151,7 @@ export const BusterChart: React.FC = React.memo( return ( - + {SwitchComponent()} diff --git a/web/src/components/charts/BusterChartErrorWrapper.tsx b/web/src/components/charts/BusterChartErrorWrapper.tsx index eddf074f7..257d0c4e9 100644 --- a/web/src/components/charts/BusterChartErrorWrapper.tsx +++ b/web/src/components/charts/BusterChartErrorWrapper.tsx @@ -25,9 +25,7 @@ export class BusterChartErrorWrapper extends Component { render() { if (this.state.hasError) { return ( -
+
(({ children, id, className, bordered, loading, useTableSizing }) => { +}>(({ children, id, className, bordered, loading }) => { const { styles, cx } = useStyles(); const ref = useRef(null); const size = useSize(ref); @@ -22,12 +21,9 @@ export const BusterChartWrapper = React.memo<{ ref={ref} id={id} className={cx( - styles.card, className, - 'flex w-full flex-col', + 'flex h-full w-full flex-col', 'transition duration-300', - useTableSizing ? 'h-full' : 'h-full max-h-[600px] p-[18px]', - bordered ? styles.cardBorder : '', loading ? '!bg-transparent' : undefined, 'overflow-hidden' )}> @@ -40,13 +36,5 @@ export const BusterChartWrapper = React.memo<{ BusterChartWrapper.displayName = 'BusterChartWrapper'; const useStyles = createStyles(({ token }) => { - return { - card: { - borderRadius: token.borderRadius, - background: token.colorBgContainer - }, - cardBorder: { - border: `0.5px solid ${token.colorBorder}` - } - }; + return {}; }); diff --git a/web/src/components/charts/config.tsx b/web/src/components/charts/config.tsx index f35aa93a4..65598bb48 100644 --- a/web/src/components/charts/config.tsx +++ b/web/src/components/charts/config.tsx @@ -1,18 +1,5 @@ import { AppMaterialIcons } from '@/components/icons'; -import { ViewType, ChartType } from './interfaces'; - -export const viewTypeOptions = [ - { - label: 'Chart', - value: ViewType.Chart, - icon: - }, - { - label: 'Table', - value: ViewType.Table, - icon: - } -]; +import { ChartType } from './interfaces'; export const chartOptions = [ { diff --git a/web/src/components/charts/helpers.ts b/web/src/components/charts/helpers.ts index f15427a75..98613bcf6 100644 --- a/web/src/components/charts/helpers.ts +++ b/web/src/components/charts/helpers.ts @@ -1,5 +1,5 @@ -import { BarAndLineAxis, ChartEncodes } from './interfaces'; -import { ChartType, ViewType } from './interfaces/enum'; +import { ChartEncodes } from './interfaces'; +import { ChartType } from './interfaces/enum'; import isEmpty from 'lodash/isEmpty'; const defaultAxisCheck = (selectedAxis: ChartEncodes) => { diff --git a/web/src/components/charts/interfaces/chartComponentInterfaces.ts b/web/src/components/charts/interfaces/chartComponentInterfaces.ts index 99360886c..10fad87ef 100644 --- a/web/src/components/charts/interfaces/chartComponentInterfaces.ts +++ b/web/src/components/charts/interfaces/chartComponentInterfaces.ts @@ -42,7 +42,6 @@ export interface BusterChartRenderComponentProps | 'id' | 'bordered' | 'editable' - | 'selectedView' | 'groupByMethod' | 'error' | 'pieChartAxis' diff --git a/web/src/components/charts/interfaces/chartConfigProps.ts b/web/src/components/charts/interfaces/chartConfigProps.ts index e522cc845..a8636e888 100644 --- a/web/src/components/charts/interfaces/chartConfigProps.ts +++ b/web/src/components/charts/interfaces/chartConfigProps.ts @@ -1,4 +1,4 @@ -import { ChartType, ViewType } from './enum'; +import { ChartType } from './enum'; import type { BarAndLineAxis, ComboChartAxis, PieChartAxis, ScatterAxis } from './axisInterfaces'; import type { CategoryAxisStyleConfig, @@ -13,7 +13,6 @@ import type { IColumnLabelFormat } from './columnLabelInterfaces'; export type BusterChartConfigProps = { selectedChartType: ChartType; - selectedView: ViewType; //COLUMN SETTINGS columnSettings?: Record; //OPTIONAL because the defaults will be determined by the UI diff --git a/web/src/components/charts/interfaces/enum.ts b/web/src/components/charts/interfaces/enum.ts index 6d0932a37..497050700 100644 --- a/web/src/components/charts/interfaces/enum.ts +++ b/web/src/components/charts/interfaces/enum.ts @@ -14,8 +14,3 @@ export type ChartTypePlottable = | ChartType.Scatter | ChartType.Pie | ChartType.Combo; - -export enum ViewType { - Chart = 'chart', - Table = 'table' -} diff --git a/web/src/context/MetricData/BusterMetricDataProvider.tsx b/web/src/context/MetricData/BusterMetricDataProvider.tsx index 16a5edaaa..7671bfa72 100644 --- a/web/src/context/MetricData/BusterMetricDataProvider.tsx +++ b/web/src/context/MetricData/BusterMetricDataProvider.tsx @@ -98,12 +98,14 @@ const useMetricData = () => { fetching: true }); - //TODO: remove mock data - // _setMetricData(metricId, { ...MOCK_DATA, fetched: true }); - onSetMetricData({ - ...MOCK_DATA, - metricId - }); + setTimeout(() => { + //TODO: remove mock data + // _setMetricData(metricId, { ...MOCK_DATA, fetched: true }); + onSetMetricData({ + ...MOCK_DATA, + metricId + }); + }, 1800); return await busterSocket.emitAndOnce({ emitEvent: { diff --git a/web/src/context/MetricData/MOCK_DATA.ts b/web/src/context/MetricData/MOCK_DATA.ts index f38df8a89..00a4fd064 100644 --- a/web/src/context/MetricData/MOCK_DATA.ts +++ b/web/src/context/MetricData/MOCK_DATA.ts @@ -3,7 +3,7 @@ import type { BusterMetricData } from '../Metrics'; import { faker } from '@faker-js/faker'; const mockData = (): Record[] => { - return Array.from({ length: 10 }, (x, index) => ({ + return Array.from({ length: 100 }, (x, index) => ({ sales: faker.number.int({ min: index * 50, max: (index + 1) * 100 }), date: faker.date.past({ years: index + 1 }).toISOString(), product: faker.commerce.productName() @@ -41,12 +41,19 @@ const dataMetadata: DataMetadata = { row_count: 10 }; +const data = mockData(); +data.push({ + sales: 1, + date: '2024-01-01', + product: 'NATE RULEZ' +}); + export const MOCK_DATA: Required = { fetched: true, fetching: false, error: null, fetchedAt: Date.now(), - data: mockData(), + data: data, data_metadata: dataMetadata, dataFromRerun: null, code: `SELECT diff --git a/web/src/context/Metrics/BusterMetricsProvider.tsx b/web/src/context/Metrics/BusterMetricsProvider.tsx index 7405432db..7e3d09b2b 100644 --- a/web/src/context/Metrics/BusterMetricsProvider.tsx +++ b/web/src/context/Metrics/BusterMetricsProvider.tsx @@ -505,10 +505,12 @@ export const useBusterMetrics = () => { } //TODO: remove this - _onGetMetricState({ - ...MOCK_METRIC, - id: metricId - }); + setTimeout(() => { + _onGetMetricState({ + ...MOCK_METRIC, + id: metricId + }); + }, 500); return await busterSocket.emitAndOnce({ emitEvent: { diff --git a/web/src/context/Metrics/MOCK_METRIC.ts b/web/src/context/Metrics/MOCK_METRIC.ts index 258843d69..4dc842487 100644 --- a/web/src/context/Metrics/MOCK_METRIC.ts +++ b/web/src/context/Metrics/MOCK_METRIC.ts @@ -1,6 +1,55 @@ -import { DEFAULT_CHART_CONFIG, ShareRole, VerificationStatus } from '@/api/asset_interfaces'; +import { + DataMetadata, + DEFAULT_CHART_CONFIG, + IBusterMetricChartConfig, + ShareRole, + VerificationStatus +} from '@/api/asset_interfaces'; import { IBusterMetric } from './interfaces'; import { faker } from '@faker-js/faker'; +import { ChartType } from '@/components/charts'; + +const MOCK_CHART_CONFIG: IBusterMetricChartConfig = { + ...DEFAULT_CHART_CONFIG, + selectedChartType: ChartType.Bar, + barAndLineAxis: { + x: ['date'], + y: ['sales'], + category: [] + // category: ['product'] + } +}; + +const dataMetadata: DataMetadata = { + column_count: 3, + column_metadata: [ + { + name: 'sales', + min_value: 0, + max_value: 1000, + unique_values: 10, + simple_type: 'number', + type: 'integer' + }, + { + name: 'date', + min_value: '2024-01-01', + max_value: '2024-01-31', + unique_values: 31, + simple_type: 'date', + type: 'date' + }, + { + name: 'product', + min_value: 'Product A', + max_value: 'Product Z', + unique_values: 26, + simple_type: 'text', + type: 'text' + } + ], + row_count: 10 +}; export const MOCK_METRIC: IBusterMetric = { id: '123', @@ -8,14 +57,14 @@ export const MOCK_METRIC: IBusterMetric = { description: 'This is a mock metric', time_frame: '1d', type: 'metric', - chart_config: DEFAULT_CHART_CONFIG, + chart_config: MOCK_CHART_CONFIG, fetched: true, fetching: false, fetchedAt: 0, dataset_id: '123', dataset_name: 'Mock Dataset', error: null, - data_metadata: null, + data_metadata: dataMetadata, status: VerificationStatus.notRequested, evaluation_score: 'Moderate', evaluation_summary: faker.lorem.sentence(33), diff --git a/web/src/context/Metrics/helpers/messageAutoChartHandler/messageAutoChartHandler.spec.ts b/web/src/context/Metrics/helpers/messageAutoChartHandler/messageAutoChartHandler.spec.ts index f8250849a..291e69a7e 100644 --- a/web/src/context/Metrics/helpers/messageAutoChartHandler/messageAutoChartHandler.spec.ts +++ b/web/src/context/Metrics/helpers/messageAutoChartHandler/messageAutoChartHandler.spec.ts @@ -25,7 +25,6 @@ describe('createDefaultChartConfig', () => { '#E83562' ], selectedChartType: 'table', - selectedView: 'table', yAxisShowAxisLabel: true, yAxisShowAxisTitle: true, yAxisAxisTitle: null, diff --git a/web/src/utils/imageGeneration.tsx b/web/src/utils/imageGeneration.tsx index c7c5c81b4..9f7242698 100644 --- a/web/src/utils/imageGeneration.tsx +++ b/web/src/utils/imageGeneration.tsx @@ -158,7 +158,6 @@ const ChartPreviewImage = ({ messageData: BusterMetricData; }) => { const data = messageData?.data || []; - const selectedChartView = message?.chart_config?.selectedView; const chartOptions = message?.chart_config; const chart = ( @@ -187,10 +186,6 @@ export const PreviewImageReactComponent: React.FC<{ }> = ({ isDark = false, message, messageData }) => { const data = messageData?.data || []; - if (!message?.chart_config?.selectedView) { - return
; - } - const BusterLogo = (