geido commented on code in PR #33398: URL: https://github.com/apache/superset/pull/33398#discussion_r2116163137
########## superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx: ########## @@ -0,0 +1,1147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { FC, useEffect, useMemo, useState, useRef, useCallback } from 'react'; +import { t, styled, css, SLOW_DEBOUNCE, useTheme } from '@superset-ui/core'; +import { debounce } from 'lodash'; +import { Form, FormItem } from 'src/components/Form'; +import { Input, TextArea } from 'src/components/Input'; +import { Radio } from 'src/components/Radio'; +import Select from 'src/components/Select/Select'; +import Collapse from 'src/components/Collapse'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import Loading from 'src/components/Loading'; +import { getChartDataRequest } from 'src/components/Chart/chartAction'; +import { CollapsibleControl } from '../FiltersConfigModal/FiltersConfigForm/CollapsibleControl'; +import { ColumnSelect } from '../FiltersConfigModal/FiltersConfigForm/ColumnSelect'; +import DatasetSelect from '../FiltersConfigModal/FiltersConfigForm/DatasetSelect'; +import DefaultValue from '../FiltersConfigModal/FiltersConfigForm/DefaultValue'; +import { ChartCustomizationItem } from './types'; +import { getFormData } from '../utils'; + +const StyledForm = styled(Form)` + .ant-form-item { + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + } +`; + +const StyledContainer = styled.div` + ${({ theme }) => ` + display: flex; + flex-direction: row; + gap: ${theme.gridUnit * 4}px; + padding: ${theme.gridUnit * 2}px; + `} +`; + +const FORM_ITEM_WIDTH = 300; + +const StyledFormItem = styled(FormItem)` + width: ${FORM_ITEM_WIDTH}px; + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + + .ant-form-item-label > label { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + font-weight: ${({ theme }) => theme.typography.weights.normal}; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + } Review Comment: Not going to comment on these next but the same logic applies everywhere. Can we remove as many style customizations as possible and go vanilla? ########## superset-frontend/src/dashboard/actions/dashboardInfo.ts: ########## @@ -150,32 +232,136 @@ export function saveFilterBarOrientation(orientation: FilterBarOrientation) { } export function saveCrossFiltersSetting(crossFiltersEnabled: boolean) { - return async (dispatch: Dispatch, getState: () => RootState) => { + return async function saveCrossFiltersSettingThunk( + dispatch: Dispatch, + getState: () => RootState, + ) { const { id, metadata } = getState().dashboardInfo; + dispatch(setCrossFiltersEnabled(crossFiltersEnabled)); const updateDashboard = makeApi< Partial<DashboardInfo>, - { result: Partial<DashboardInfo>; last_modified_time: number } + { result: DashboardInfo } >({ method: 'PUT', endpoint: `/api/v1/dashboard/${id}`, }); + try { const response = await updateDashboard({ json_metadata: JSON.stringify({ ...metadata, cross_filters_enabled: crossFiltersEnabled, }), }); + dispatch( + dashboardInfoChanged({ + metadata: JSON.parse(response.result.json_metadata), + }), + ); + return response; + } catch (err) { + dispatch(addDangerToast(t('Failed to save cross-filters setting'))); + throw err; + } + }; +} + +export function saveChartCustomization( + chartCustomizationItems: ChartCustomizationSavePayload[], +): ThunkAction<Promise<any>, RootState, null, AnyAction> { + return async function ( + dispatch: ThunkDispatch<RootState, null, AnyAction>, + getState: () => RootState, + ) { + const { id, metadata, json_metadata } = getState().dashboardInfo; + const simpleItems = chartCustomizationItems + .filter(item => !item.removed) + .map(item => ({ + id: item.id, + title: item.title || '[untitled]', + customization: { + name: item.customization?.name || '', + dataset: item.customization?.dataset || null, + datasetInfo: item.customization?.datasetInfo, + column: item.customization?.column || null, + description: item.customization?.description, + sortFilter: !!item.customization?.sortFilter, + sortAscending: item.customization?.sortAscending !== false, + sortMetric: item.customization?.sortMetric || undefined, + hasDefaultValue: !!item.customization?.hasDefaultValue, + defaultValue: item.customization?.defaultValue, + isRequired: !!item.customization?.isRequired, + selectFirst: !!item.customization?.selectFirst, + defaultDataMask: item.customization?.defaultDataMask, + defaultValueQueriesData: item.customization?.defaultValueQueriesData, + }, + })); + + const updateDashboard = makeApi< + Partial<DashboardInfo>, + { result: Partial<DashboardInfo>; last_modified_time: number } + >({ + method: 'PUT', + endpoint: `/api/v1/dashboard/${id}`, + }); + + try { + let parsedMetadata: any = {}; + try { + parsedMetadata = json_metadata ? JSON.parse(json_metadata) : metadata; + } catch (e) { + console.error('Error parsing json_metadata:', e); + parsedMetadata = metadata || {}; + } + + const updatedMetadata = { + ...parsedMetadata, + native_filter_configuration: ( + parsedMetadata.native_filter_configuration || [] + ).filter( + (item: any) => Review Comment: I am marking all these `any` as I think these are dangerous. Can we get rid of them? ########## superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx: ########## @@ -153,7 +152,10 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => { return; } const scopes = nativeFilterScopes.map(filterScope => { - if (filterScope.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX)) { + if ( + filterScope.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX) || + (filterScope as any).type === 'CHART_CUSTOMIZATION' Review Comment: Can we use a type for the `CHART_CUSTOMIZATION`? ########## superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx: ########## @@ -0,0 +1,1147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { FC, useEffect, useMemo, useState, useRef, useCallback } from 'react'; +import { t, styled, css, SLOW_DEBOUNCE, useTheme } from '@superset-ui/core'; +import { debounce } from 'lodash'; +import { Form, FormItem } from 'src/components/Form'; +import { Input, TextArea } from 'src/components/Input'; +import { Radio } from 'src/components/Radio'; +import Select from 'src/components/Select/Select'; +import Collapse from 'src/components/Collapse'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import Loading from 'src/components/Loading'; +import { getChartDataRequest } from 'src/components/Chart/chartAction'; +import { CollapsibleControl } from '../FiltersConfigModal/FiltersConfigForm/CollapsibleControl'; +import { ColumnSelect } from '../FiltersConfigModal/FiltersConfigForm/ColumnSelect'; +import DatasetSelect from '../FiltersConfigModal/FiltersConfigForm/DatasetSelect'; +import DefaultValue from '../FiltersConfigModal/FiltersConfigForm/DefaultValue'; +import { ChartCustomizationItem } from './types'; +import { getFormData } from '../utils'; + +const StyledForm = styled(Form)` + .ant-form-item { + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + } +`; + +const StyledContainer = styled.div` + ${({ theme }) => ` + display: flex; + flex-direction: row; + gap: ${theme.gridUnit * 4}px; + padding: ${theme.gridUnit * 2}px; + `} +`; + +const FORM_ITEM_WIDTH = 300; + +const StyledFormItem = styled(FormItem)` + width: ${FORM_ITEM_WIDTH}px; + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + + .ant-form-item-label > label { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + font-weight: ${({ theme }) => theme.typography.weights.normal}; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + } +`; + +const CheckboxLabel = styled.span` + font-size: ${({ theme }) => theme.typography.sizes.m}px; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + font-weight: ${({ theme }) => theme.typography.weights.normal}; +`; + +const StyledTextArea = styled(TextArea)` + min-height: ${({ theme }) => theme.gridUnit * 24}px; + resize: vertical; +`; + +const StyledRadioGroup = styled(Radio.Group)` + .ant-radio-wrapper { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + } +`; + +const StyledMarginTop = styled.div` + margin-top: ${({ theme }) => theme.gridUnit * 2}px; +`; + +interface Props { + form: any; + item: ChartCustomizationItem; + onUpdate: (updatedItem: ChartCustomizationItem) => void; +} + +const ChartCustomizationForm: FC<Props> = ({ form, item, onUpdate }) => { + const theme = useTheme(); + const customization = useMemo( + () => item.customization || {}, + [item.customization], + ); + + const [metrics, setMetrics] = useState<any[]>([]); + const [isDefaultValueLoading, setIsDefaultValueLoading] = useState(false); + const [error, setError] = useState<any>(null); + const [datasetDetails, setDatasetDetails] = useState<{ + id: number; + table_name: string; + schema?: string; + database?: { database_name: string }; + } | null>(null); + + const fetchedRef = useRef({ + dataset: null, + column: null, + hasDefaultValue: false, + }); + + const datasetValue = useMemo(() => { + if (!customization.dataset) return undefined; + + let datasetId: number; + + if ( + typeof customization.dataset === 'object' && + 'value' in customization.dataset + ) { + datasetId = Number((customization.dataset as any).value); + } else { + datasetId = Number(customization.dataset); + } + + if (Number.isNaN(datasetId)) return undefined; + + if (datasetDetails && datasetDetails.id === datasetId) { + return { + value: datasetId, + label: + datasetDetails.table_name + + (datasetDetails.schema ? ` (${datasetDetails.schema})` : '') + + (datasetDetails.database?.database_name + ? ` [${datasetDetails.database.database_name}]` + : ''), + }; + } + + if (customization.datasetInfo) { + if ('label' in customization.datasetInfo) { + return { + value: datasetId, + label: (customization.datasetInfo as { label: string }).label, + }; + } + if ('table_name' in customization.datasetInfo) { + return { + value: datasetId, + label: (customization.datasetInfo as { table_name: string }) + .table_name, + }; + } + } + + return { + value: datasetId, + label: `Dataset ${datasetId}`, + }; + }, [customization.dataset, customization.datasetInfo, datasetDetails]); + + const formChanged = useCallback(() => { + form.setFields([{ name: 'changed', value: true }]); + + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + onUpdate({ + ...item, + customization: { + ...customization, + ...formValues, + }, + }); + }, [form, item, customization, onUpdate]); + + const debouncedFormChanged = useMemo( + () => debounce(formChanged, SLOW_DEBOUNCE), + [formChanged], + ); + + const ensureFilterSlot = useCallback(() => { + const currentFilters = form.getFieldValue('filters') || {}; + if (!currentFilters[item.id]) { + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: {}, + }, + }); + } + }, [form, item.id]); + + const fetchDatasetInfo = useCallback(async () => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + + if (!dataset) { + setMetrics([]); + return; + } + + try { + let datasetId: number; + if ( + typeof dataset === 'object' && + dataset !== null && + 'value' in dataset + ) { + datasetId = Number((dataset as any).value); + } else { + datasetId = Number(dataset); + } + if (Number.isNaN(datasetId)) return; + + const response = await fetch(`/api/v1/dataset/${datasetId}`); + const data = await response.json(); + + if (data?.result) { + const datasetDetails = { + id: data.result.id, + table_name: data.result.table_name, + schema: data.result.schema, + database: data.result.database, + }; + + setDatasetDetails(datasetDetails); + + const currentFilters = form.getFieldValue('filters') || {}; + const currentItemValues = currentFilters[item.id] || {}; + + if ( + currentItemValues.dataset && + typeof currentItemValues.dataset === 'string' + ) { + const enhancedDataset = { + value: Number(currentItemValues.dataset), + label: data.result.table_name, + table_name: data.result.table_name, + schema: data.result.schema, + }; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...currentItemValues, + dataset: currentItemValues.dataset, + datasetInfo: enhancedDataset, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + dataset: currentItemValues.dataset, + datasetInfo: enhancedDataset, + }, + }); + } + + if (data.result.metrics && data.result.metrics.length > 0) { + setMetrics(data.result.metrics); + } else { + setMetrics([]); + } + } + } catch (error) { + console.error('Error fetching dataset info:', error); + setMetrics([]); + } + }, [form, item.id, customization.dataset]); + + const fetchDefaultValueData = useCallback(async () => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + const column = formValues.column || customization.column; + const hasDefaultValue = + formValues.hasDefaultValue ?? customization.hasDefaultValue; + const isRequired = formValues.isRequired ?? customization.isRequired; + + if (hasDefaultValue && !isRequired) { + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + hasDefaultValue, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + hasDefaultValue, + }, + }); + } + + if (!dataset || !column) { + setIsDefaultValueLoading(false); + return; + } + + setIsDefaultValueLoading(true); + try { + const datasetId = Number(dataset.value || dataset); + if (Number.isNaN(datasetId)) { + throw new Error('Invalid dataset ID'); + } + + const formData = getFormData({ + datasetId, + dashboardId: 0, + groupby: column, + filterType: 'filter_select', + controlValues: { + sortAscending: + formValues.sortAscending ?? customization.sortAscending, + sortMetric: formValues.sortMetric ?? customization.sortMetric, + }, + }); + + const { json } = await getChartDataRequest({ formData }); + + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + defaultValueQueriesData: json.result, + filterType: 'filter_select', + hasDefaultValue: true, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + defaultValueQueriesData: json.result, + hasDefaultValue: + formValues.hasDefaultValue ?? customization.hasDefaultValue, + }, + }); + + setError(null); + } catch (error) { + console.error('Error fetching default value data:', error); + setError(error); + + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + defaultValueQueriesData: null, + hasDefaultValue: + currentFilters[item.id]?.hasDefaultValue ?? + customization.hasDefaultValue ?? + false, + }, + }, + }); + } finally { + setIsDefaultValueLoading(false); + } + }, [customization, ensureFilterSlot, form, item, onUpdate]); + + useEffect(() => { + ensureFilterSlot(); + + const initialValues = { + filters: { + [item.id]: { + name: customization.name || '', + description: customization.description || '', + dataset: customization.dataset + ? String( + typeof customization.dataset === 'object' && + customization.dataset !== null + ? (customization.dataset as any).value || + customization.dataset + : customization.dataset, + ) + : null, + column: customization.column || null, + filterType: 'filter_select', + sortFilter: customization.sortFilter || false, + sortAscending: customization.sortAscending !== false, + sortMetric: customization.sortMetric || null, + hasDefaultValue: customization.hasDefaultValue || false, + isRequired: customization.isRequired || false, + selectFirst: customization.selectFirst || false, + defaultValue: customization.defaultValue, + defaultDataMask: customization.defaultDataMask, + defaultValueQueriesData: customization.defaultValueQueriesData, + }, + }, + }; + + form.setFieldsValue(initialValues); + + if (customization.dataset) { + fetchDatasetInfo(); + } + + if (customization.isRequired) { + setTimeout(() => { + form + .validateFields([['filters', item.id, 'isRequired']]) + .catch(() => {}); + }, 0); + } + }, [item.id, fetchDatasetInfo]); + + useEffect(() => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const hasDataset = !!formValues.dataset; + const hasColumn = !!formValues.column; + const hasDefaultValue = !!formValues.hasDefaultValue; + const isRequired = !!formValues.isRequired; + + if (hasDataset && fetchedRef.current.dataset !== formValues.dataset) { + fetchDatasetInfo(); + } + + if (isRequired && (!hasDataset || !hasColumn)) { + form.validateFields([['filters', item.id, 'isRequired']]).catch(() => {}); + } + if ( + (hasDefaultValue || isRequired) && + hasDataset && + hasColumn && + (fetchedRef.current.dataset !== formValues.dataset || + fetchedRef.current.column !== formValues.column || + fetchedRef.current.hasDefaultValue !== hasDefaultValue) + ) { + fetchedRef.current = { + dataset: formValues.dataset, + column: formValues.column, + hasDefaultValue, + }; + + fetchDefaultValueData(); + } + }, [form, item.id, fetchDefaultValueData, fetchDatasetInfo]); + + const setDataMask = useCallback( + (dataMask: any) => { + if (!dataMask.filterState) return; + + ensureFilterSlot(); + const filtersValue = form.getFieldValue('filters') || {}; + + form.setFieldsValue({ + filters: { + ...filtersValue, + [item.id]: { + ...(filtersValue[item.id] || {}), + defaultDataMask: dataMask, + defaultValue: dataMask.filterState?.value, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + defaultDataMask: dataMask, + defaultValue: dataMask.filterState?.value, + }, + }); + }, + [ensureFilterSlot, form, item, onUpdate, customization], + ); + + const generatedFormData = useMemo(() => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + const column = formValues.column || customization.column; + + const datasetId = dataset ? Number(dataset.value || dataset) : undefined; + + return getFormData({ + datasetId, + groupby: column || '', + dashboardId: 0, + filterType: 'filter_select', + controlValues: { + sortAscending: formValues.sortAscending ?? customization.sortAscending, + sortMetric: formValues.sortMetric ?? customization.sortMetric, + }, + }); + }, [form, item.id, customization]); + + return ( + <StyledForm + layout="vertical" + form={form} + onValuesChange={() => formChanged()} + > + <StyledContainer> + <StyledFormItem + name={['filters', item.id, 'name']} + label={t('Name')} + initialValue={customization.name || ''} + rules={[{ required: true, message: t('Please enter a name') }]} + > + <Input + placeholder={t('Enter a name for this customization')} + onChange={debouncedFormChanged} + /> + </StyledFormItem> + + <StyledFormItem + name={['filters', item.id, 'dataset']} + label={ + <> + {t('Dataset')} + <InfoTooltipWithTrigger + tooltip={t('Select the dataset this group by will use')} + placement="right" + /> + </> + } + initialValue={datasetValue} + rules={[{ required: true, message: t('Please select a dataset') }]} + > + <DatasetSelect + onChange={(dataset: { label: string; value: number }) => { + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + + const datasetWithInfo = { + ...dataset, + table_name: dataset.label, + }; + + const datasetId = dataset.value.toString(); + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + dataset: datasetId, + datasetInfo: datasetWithInfo, + column: null, + defaultValueQueriesData: null, + defaultValue: undefined, + defaultDataMask: undefined, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + dataset: datasetId, + datasetInfo: datasetWithInfo, + column: null, + defaultValueQueriesData: null, + defaultValue: undefined, + defaultDataMask: undefined, + }, + }); + + fetchDatasetInfo(); + formChanged(); + }} + /> + </StyledFormItem> + </StyledContainer> + + <StyledContainer> + <StyledFormItem + name={['filters', item.id, 'column']} + label={ + <> + <CheckboxLabel>{t('Group by column')}</CheckboxLabel> + <InfoTooltipWithTrigger + tooltip={t('Choose the column to group by')} + placement="right" + /> + </> + } + initialValue={customization.column} + rules={[{ required: true, message: t('Please select a column') }]} + css={css` + width: 100%; + `} + > + <ColumnSelect + allowClear + form={form} + formField="column" + filterId={item.id} + filterValues={(column: any) => column.filterable !== false} + datasetId={(() => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + if (dataset) { + if ( + typeof dataset === 'object' && + dataset !== null && + 'value' in dataset + ) { + return Number((dataset as any).value); + } + return Number(dataset); + } + return undefined; + })()} + onChange={(column: string) => { + setError(null); + const currentFilters = form.getFieldValue('filters') || {}; + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + column, + defaultValueQueriesData: null, + defaultValue: undefined, + defaultDataMask: null, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + column, + defaultValueQueriesData: null, + defaultValue: undefined, + defaultDataMask: null, + }, + }); + + formChanged(); + }} + /> + </StyledFormItem> + </StyledContainer> + + <Collapse defaultActiveKey={['settings']} expandIconPosition="right"> + <Collapse.Panel header={t('Customization settings')} key="settings"> + <StyledFormItem + name={['filters', item.id, 'description']} + label={t('Description')} + initialValue={customization.description || ''} + css={css` + width: ${FORM_ITEM_WIDTH * 1.5}px; + `} + > + <StyledTextArea + placeholder={t( + 'Add description that will be displayed when hovering over the label...', + )} + onChange={debouncedFormChanged} + autoSize={{ minRows: 4 }} + /> + </StyledFormItem> + + <StyledFormItem name={['filters', item.id, 'sortFilter']}> + <CollapsibleControl + checked={ + form.getFieldValue('filters')?.[item.id]?.sortFilter ?? + customization.sortFilter ?? + false + } + initialValue={customization.sortFilter ?? false} + title={<CheckboxLabel>{t('Sort filter values')}</CheckboxLabel>} + onChange={checked => { + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + const currentHasDefaultValue = + currentFilters[item.id]?.hasDefaultValue; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + sortFilter: checked, + ...(checked + ? {} + : { + sortAscending: undefined, + sortMetric: undefined, + }), + hasDefaultValue: currentHasDefaultValue, + }, + }, + }); + onUpdate({ + ...item, + customization: { + ...customization, + sortFilter: checked, + ...(checked === false + ? { + sortAscending: undefined, + sortMetric: undefined, + } + : {}), + }, + }); + + formChanged(); + }} + > + <StyledFormItem + name={['filters', item.id, 'sortAscending']} + label={<CheckboxLabel>{t('Sort type')}</CheckboxLabel>} + initialValue={customization.sortAscending !== false} + > + <StyledRadioGroup + options={[ + { label: t('Sort ascending'), value: true }, + { label: t('Sort descending'), value: false }, + ]} + onChange={value => { + const currentFilters = form.getFieldValue('filters') || {}; + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + sortAscending: value.target.value, + }, + }, + }); + formChanged(); + }} + /> + </StyledFormItem> + + {customization.sortFilter && metrics.length > 0 && ( + <StyledFormItem + name={['filters', item.id, 'sortMetric']} + label={ + <> + <CheckboxLabel>{t('Sort Metric')}</CheckboxLabel> + <InfoTooltipWithTrigger + placement="top" + tooltip={t( + 'If a metric is specified, sorting will be done based on the metric value', + )} + /> + </> + } + initialValue={customization.sortMetric} + > + <Select + allowClear + ariaLabel={t('Sort metric')} + value={ + form.getFieldValue('filters')?.[item.id]?.sortMetric ?? + customization.sortMetric + } + options={metrics.map((metric: any) => ({ + value: metric.metric_name, + label: metric.verbose_name ?? metric.metric_name, + }))} + onChange={value => { + ensureFilterSlot(); + const currentFilters = + form.getFieldValue('filters') || {}; + const currentHasDefaultValue = + currentFilters[item.id]?.hasDefaultValue; + + const stringValue = + value !== null && value !== undefined + ? String(value) + : undefined; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + sortMetric: stringValue, + hasDefaultValue: currentHasDefaultValue, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + sortMetric: stringValue, + }, + }); + + formChanged(); + }} + /> + </StyledFormItem> + )} + </CollapsibleControl> + </StyledFormItem> + + <StyledFormItem + name={['filters', item.id, 'defaultValueQueriesData']} + hidden + initialValue={customization.defaultValueQueriesData || null} + > + <Input /> + </StyledFormItem> + + <StyledFormItem name={['filters', item.id, 'defaultValue']}> + <CollapsibleControl + checked={ + form.getFieldValue('filters')?.[item.id]?.hasDefaultValue ?? + customization.hasDefaultValue ?? + false + } + initialValue={customization.hasDefaultValue ?? false} + disabled={customization.isRequired || customization.selectFirst} + title={ + <CheckboxLabel> + {t('Dynamic group by has a default value')} + </CheckboxLabel> + } + tooltip={ + customization.isRequired + ? t('Cannot set default value when filter value is required') + : customization.selectFirst + ? t( + 'Cannot set default value when "Select first filter value by default" is enabled', + ) + : t('Set a default value for this filter') + } + onChange={checked => { + ensureFilterSlot(); + const currentFilters = form.getFieldValue('filters') || {}; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...(currentFilters[item.id] || {}), + hasDefaultValue: checked, + ...(checked ? { selectFirst: false } : {}), + ...(checked === false + ? { + defaultDataMask: null, + defaultValue: undefined, + defaultValueQueriesData: null, + } + : {}), + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + hasDefaultValue: checked, + ...(checked === false + ? { + defaultDataMask: null, + defaultValue: undefined, + defaultValueQueriesData: null, + } + : {}), + }, + }); + + if (checked) { + fetchDefaultValueData(); + } + + formChanged(); + }} + > + <StyledFormItem + name={['filters', item.id, 'defaultDataMask']} + initialValue={customization.defaultDataMask || null} + label={<CheckboxLabel>{t('Default Value')}</CheckboxLabel>} + required={customization.hasDefaultValue} + rules={[ + { + validator: async (_, value) => { + const current = + form.getFieldValue(['filters', item.id]) || {}; + if (!current.hasDefaultValue && !current.isRequired) { + return Promise.resolve(); + } + + const val = + value?.filterState?.value ?? current.defaultValue; + if (!val) { + return Promise.reject( + new Error(t('Please choose a valid value')), + ); + } + return Promise.resolve(); + }, + }, + ]} + > + <StyledMarginTop> + {(() => { + const currentFilters = form.getFieldValue('filters') || {}; + const currentItemValues = currentFilters[item.id] || {}; + + const hasDatasetAndColumn = + !!currentItemValues.dataset && !!currentItemValues.column; + const showDefaultValue = + !hasDatasetAndColumn || + (!isDefaultValueLoading && + hasDatasetAndColumn && + currentItemValues.defaultValueQueriesData); + + if (error) { + return ( + <div style={{ color: theme.colors.error.base }}> Review Comment: We have an Alert component. Can we use that instead? ########## superset-frontend/src/dashboard/actions/dashboardInfo.ts: ########## @@ -113,7 +143,59 @@ export function setCrossFiltersEnabled(crossFiltersEnabled: boolean) { return { type: SET_CROSS_FILTERS_ENABLED, crossFiltersEnabled }; } -export function saveFilterBarOrientation(orientation: FilterBarOrientation) { +export const SAVE_CHART_CUSTOMIZATION_COMPLETE = + 'SAVE_CHART_CUSTOMIZATION_COMPLETE'; + +function getAffectedChartIdsFromCustomization( + chartCustomization: ChartCustomizationItem[], + state: any, +): number[] { + const targetDatasets = chartCustomization + .map(item => item.customization?.dataset) + .filter(dataset => dataset !== null && dataset !== undefined) as string[]; + + console.log('Target datasets for customization:', targetDatasets); + + const charts = state.charts || {}; + if (targetDatasets.length === 0) { + return []; + } + + return Object.keys(charts) + .filter(id => { + const chart = charts[id]; + if ( + !chart?.latestQueryFormData || + Object.keys(chart.latestQueryFormData).length === 0 + ) { + return false; + } + + const chartDataset = chart.latestQueryFormData.datasource; + if (!chartDataset) { + return false; + } + + const chartDatasetParts = String(chartDataset).split('__'); + const chartDatasetId = chartDatasetParts[0]; + + return targetDatasets.some(targetDataset => { + const targetParts = String(targetDataset).split('__'); + const targetDatasetId = targetParts[0]; + + return chartDatasetId === targetDatasetId; + }); + }) + .map(id => parseInt(id, 10)); +} + +export function setChartCustomization( + chartCustomization: ChartCustomizationItem[], +) { + return { type: SAVE_CHART_CUSTOMIZATION_COMPLETE, chartCustomization }; +} + +export function saveFilterBarOrientation(orientation: any) { Review Comment: We have lost the orientation type here which should be `FilterBarOrientation` ########## superset-frontend/src/dashboard/actions/dashboardInfo.ts: ########## @@ -113,7 +143,59 @@ export function setCrossFiltersEnabled(crossFiltersEnabled: boolean) { return { type: SET_CROSS_FILTERS_ENABLED, crossFiltersEnabled }; } -export function saveFilterBarOrientation(orientation: FilterBarOrientation) { +export const SAVE_CHART_CUSTOMIZATION_COMPLETE = + 'SAVE_CHART_CUSTOMIZATION_COMPLETE'; + +function getAffectedChartIdsFromCustomization( + chartCustomization: ChartCustomizationItem[], + state: any, Review Comment: Do we have a type representation of the state here that we can use or implement? ########## superset-frontend/src/dashboard/components/GroupByBadge/index.tsx: ########## @@ -0,0 +1,159 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { memo, useMemo, useState, useRef } from 'react'; +import { useSelector } from 'react-redux'; +import { styled, t } from '@superset-ui/core'; +import { Icons } from 'src/components/Icons'; +import Badge from 'src/components/Badge'; +import { Tooltip } from 'src/components/Tooltip'; +import { ChartCustomizationItem } from '../nativeFilters/ChartCustomization/types'; +import { RootState } from '../../types'; + +export interface GroupByBadgeProps { + chartId: number; +} + +const StyledGroupByCount = styled.div` Review Comment: Is there an Ant Design component here that can do the job without all the heavy style customizations we need here? ########## superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx: ########## @@ -0,0 +1,1147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { FC, useEffect, useMemo, useState, useRef, useCallback } from 'react'; +import { t, styled, css, SLOW_DEBOUNCE, useTheme } from '@superset-ui/core'; +import { debounce } from 'lodash'; +import { Form, FormItem } from 'src/components/Form'; +import { Input, TextArea } from 'src/components/Input'; +import { Radio } from 'src/components/Radio'; +import Select from 'src/components/Select/Select'; +import Collapse from 'src/components/Collapse'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import Loading from 'src/components/Loading'; +import { getChartDataRequest } from 'src/components/Chart/chartAction'; +import { CollapsibleControl } from '../FiltersConfigModal/FiltersConfigForm/CollapsibleControl'; +import { ColumnSelect } from '../FiltersConfigModal/FiltersConfigForm/ColumnSelect'; +import DatasetSelect from '../FiltersConfigModal/FiltersConfigForm/DatasetSelect'; +import DefaultValue from '../FiltersConfigModal/FiltersConfigForm/DefaultValue'; +import { ChartCustomizationItem } from './types'; +import { getFormData } from '../utils'; + +const StyledForm = styled(Form)` + .ant-form-item { + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + } +`; + +const StyledContainer = styled.div` + ${({ theme }) => ` + display: flex; + flex-direction: row; + gap: ${theme.gridUnit * 4}px; + padding: ${theme.gridUnit * 2}px; + `} +`; + +const FORM_ITEM_WIDTH = 300; + +const StyledFormItem = styled(FormItem)` + width: ${FORM_ITEM_WIDTH}px; + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + + .ant-form-item-label > label { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + font-weight: ${({ theme }) => theme.typography.weights.normal}; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + } +`; + +const CheckboxLabel = styled.span` + font-size: ${({ theme }) => theme.typography.sizes.m}px; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + font-weight: ${({ theme }) => theme.typography.weights.normal}; +`; + +const StyledTextArea = styled(TextArea)` + min-height: ${({ theme }) => theme.gridUnit * 24}px; + resize: vertical; +`; + +const StyledRadioGroup = styled(Radio.Group)` + .ant-radio-wrapper { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + } +`; + +const StyledMarginTop = styled.div` + margin-top: ${({ theme }) => theme.gridUnit * 2}px; +`; + +interface Props { + form: any; + item: ChartCustomizationItem; + onUpdate: (updatedItem: ChartCustomizationItem) => void; +} + +const ChartCustomizationForm: FC<Props> = ({ form, item, onUpdate }) => { + const theme = useTheme(); + const customization = useMemo( + () => item.customization || {}, + [item.customization], + ); + + const [metrics, setMetrics] = useState<any[]>([]); + const [isDefaultValueLoading, setIsDefaultValueLoading] = useState(false); + const [error, setError] = useState<any>(null); + const [datasetDetails, setDatasetDetails] = useState<{ + id: number; + table_name: string; + schema?: string; + database?: { database_name: string }; + } | null>(null); + + const fetchedRef = useRef({ + dataset: null, + column: null, + hasDefaultValue: false, + }); + + const datasetValue = useMemo(() => { + if (!customization.dataset) return undefined; + + let datasetId: number; + + if ( + typeof customization.dataset === 'object' && + 'value' in customization.dataset + ) { + datasetId = Number((customization.dataset as any).value); + } else { + datasetId = Number(customization.dataset); + } + + if (Number.isNaN(datasetId)) return undefined; + + if (datasetDetails && datasetDetails.id === datasetId) { + return { + value: datasetId, + label: + datasetDetails.table_name + + (datasetDetails.schema ? ` (${datasetDetails.schema})` : '') + + (datasetDetails.database?.database_name + ? ` [${datasetDetails.database.database_name}]` + : ''), + }; + } + + if (customization.datasetInfo) { + if ('label' in customization.datasetInfo) { + return { + value: datasetId, + label: (customization.datasetInfo as { label: string }).label, + }; + } + if ('table_name' in customization.datasetInfo) { + return { + value: datasetId, + label: (customization.datasetInfo as { table_name: string }) + .table_name, + }; + } + } + + return { + value: datasetId, + label: `Dataset ${datasetId}`, + }; + }, [customization.dataset, customization.datasetInfo, datasetDetails]); + + const formChanged = useCallback(() => { + form.setFields([{ name: 'changed', value: true }]); + + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + onUpdate({ + ...item, + customization: { + ...customization, + ...formValues, + }, + }); + }, [form, item, customization, onUpdate]); + + const debouncedFormChanged = useMemo( + () => debounce(formChanged, SLOW_DEBOUNCE), + [formChanged], + ); + + const ensureFilterSlot = useCallback(() => { + const currentFilters = form.getFieldValue('filters') || {}; + if (!currentFilters[item.id]) { + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: {}, + }, + }); + } + }, [form, item.id]); + + const fetchDatasetInfo = useCallback(async () => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + + if (!dataset) { + setMetrics([]); + return; + } + + try { + let datasetId: number; + if ( + typeof dataset === 'object' && + dataset !== null && + 'value' in dataset + ) { + datasetId = Number((dataset as any).value); + } else { + datasetId = Number(dataset); + } + if (Number.isNaN(datasetId)) return; + + const response = await fetch(`/api/v1/dataset/${datasetId}`); + const data = await response.json(); + + if (data?.result) { + const datasetDetails = { + id: data.result.id, + table_name: data.result.table_name, + schema: data.result.schema, + database: data.result.database, + }; + + setDatasetDetails(datasetDetails); + + const currentFilters = form.getFieldValue('filters') || {}; + const currentItemValues = currentFilters[item.id] || {}; + + if ( + currentItemValues.dataset && + typeof currentItemValues.dataset === 'string' + ) { + const enhancedDataset = { + value: Number(currentItemValues.dataset), + label: data.result.table_name, + table_name: data.result.table_name, + schema: data.result.schema, + }; + + form.setFieldsValue({ + filters: { + ...currentFilters, + [item.id]: { + ...currentItemValues, + dataset: currentItemValues.dataset, + datasetInfo: enhancedDataset, + }, + }, + }); + + onUpdate({ + ...item, + customization: { + ...customization, + dataset: currentItemValues.dataset, + datasetInfo: enhancedDataset, + }, + }); + } + + if (data.result.metrics && data.result.metrics.length > 0) { + setMetrics(data.result.metrics); + } else { + setMetrics([]); + } + } + } catch (error) { + console.error('Error fetching dataset info:', error); + setMetrics([]); + } + }, [form, item.id, customization.dataset]); + + const fetchDefaultValueData = useCallback(async () => { + const formValues = form.getFieldValue('filters')?.[item.id] || {}; + const dataset = formValues.dataset || customization.dataset; + const column = formValues.column || customization.column; + const hasDefaultValue = + formValues.hasDefaultValue ?? customization.hasDefaultValue; + const isRequired = formValues.isRequired ?? customization.isRequired; + + if (hasDefaultValue && !isRequired) { + ensureFilterSlot(); Review Comment: It seems we are updating fields values multiple times on this function and just 2 lines later. Is that intended? Can we improve the code structure to DRY this up a little bit? ########## superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationModal.tsx: ########## @@ -0,0 +1,394 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { useState, useEffect, useMemo, useCallback, memo } from 'react'; Review Comment: Do we really need to rebuild a Modal for this feature? Can't we re-use the existing Modal for native filters? I believe it should be doing pretty much the same thing. We can maybe split that up a little bit more and DRY this up? ########## superset-frontend/src/dashboard/components/GroupByBadge/index.tsx: ########## @@ -0,0 +1,159 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { memo, useMemo, useState, useRef } from 'react'; +import { useSelector } from 'react-redux'; +import { styled, t } from '@superset-ui/core'; +import { Icons } from 'src/components/Icons'; +import Badge from 'src/components/Badge'; +import { Tooltip } from 'src/components/Tooltip'; +import { ChartCustomizationItem } from '../nativeFilters/ChartCustomization/types'; +import { RootState } from '../../types'; + +export interface GroupByBadgeProps { + chartId: number; +} + +const StyledGroupByCount = styled.div` + ${({ theme }) => ` + display: flex; + justify-items: center; + align-items: center; + cursor: pointer; + margin-right: ${theme.gridUnit}px; + padding-left: ${theme.gridUnit * 2}px; + padding-right: ${theme.gridUnit * 2}px; + background: ${theme.colors.grayscale.light4}; + border-radius: 4px; + height: 100%; + .anticon { + vertical-align: middle; + color: ${theme.colors.primary.base}; + &:hover { + color: ${theme.colors.primary.dark1}; + } + } + &:focus-visible { + outline: 2px solid ${theme.colors.primary.dark2}; + } + `} +`; + +const StyledBadge = styled(Badge)` + ${({ theme }) => ` + margin-left: ${theme.gridUnit * 2}px; + &>sup.antd5-badge-count { + padding: 0 ${theme.gridUnit}px; + min-width: ${theme.gridUnit * 4}px; + height: ${theme.gridUnit * 4}px; + line-height: 1.5; + font-weight: ${theme.typography.weights.medium}; + font-size: ${theme.typography.sizes.s - 1}px; + box-shadow: none; + padding: 0 ${theme.gridUnit}px; + background-color: ${theme.colors.primary.base}; Review Comment: How different is it going to look if we use vanilla Ant Design here? We are trying to keep style customizations to the minimum ########## superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationForm.tsx: ########## @@ -0,0 +1,1147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 { FC, useEffect, useMemo, useState, useRef, useCallback } from 'react'; +import { t, styled, css, SLOW_DEBOUNCE, useTheme } from '@superset-ui/core'; +import { debounce } from 'lodash'; +import { Form, FormItem } from 'src/components/Form'; +import { Input, TextArea } from 'src/components/Input'; +import { Radio } from 'src/components/Radio'; +import Select from 'src/components/Select/Select'; +import Collapse from 'src/components/Collapse'; +import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import Loading from 'src/components/Loading'; +import { getChartDataRequest } from 'src/components/Chart/chartAction'; +import { CollapsibleControl } from '../FiltersConfigModal/FiltersConfigForm/CollapsibleControl'; +import { ColumnSelect } from '../FiltersConfigModal/FiltersConfigForm/ColumnSelect'; +import DatasetSelect from '../FiltersConfigModal/FiltersConfigForm/DatasetSelect'; +import DefaultValue from '../FiltersConfigModal/FiltersConfigForm/DefaultValue'; +import { ChartCustomizationItem } from './types'; +import { getFormData } from '../utils'; + +const StyledForm = styled(Form)` + .ant-form-item { + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + } +`; + +const StyledContainer = styled.div` + ${({ theme }) => ` + display: flex; + flex-direction: row; + gap: ${theme.gridUnit * 4}px; + padding: ${theme.gridUnit * 2}px; + `} +`; + +const FORM_ITEM_WIDTH = 300; + +const StyledFormItem = styled(FormItem)` + width: ${FORM_ITEM_WIDTH}px; + margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; + + .ant-form-item-label > label { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + font-weight: ${({ theme }) => theme.typography.weights.normal}; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + } +`; + +const CheckboxLabel = styled.span` + font-size: ${({ theme }) => theme.typography.sizes.m}px; + color: ${({ theme }) => theme.colors.grayscale.dark1}; + font-weight: ${({ theme }) => theme.typography.weights.normal}; +`; + +const StyledTextArea = styled(TextArea)` + min-height: ${({ theme }) => theme.gridUnit * 24}px; + resize: vertical; +`; + +const StyledRadioGroup = styled(Radio.Group)` + .ant-radio-wrapper { + font-size: ${({ theme }) => theme.typography.sizes.m}px; + } +`; + +const StyledMarginTop = styled.div` + margin-top: ${({ theme }) => theme.gridUnit * 2}px; +`; + +interface Props { + form: any; + item: ChartCustomizationItem; + onUpdate: (updatedItem: ChartCustomizationItem) => void; +} + +const ChartCustomizationForm: FC<Props> = ({ form, item, onUpdate }) => { + const theme = useTheme(); + const customization = useMemo( + () => item.customization || {}, + [item.customization], + ); + + const [metrics, setMetrics] = useState<any[]>([]); Review Comment: Not going to comment on `any` as well next but all of these should be replaced with proper types -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
