From eab888c63a3a6a68c1ea7ec24d12bdf55ab0751b Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 2 Dec 2024 15:04:29 +0100 Subject: [PATCH] perf: Optimize dashboard chart-related components (#31241) --- .../components/SliceHeader/index.tsx | 342 +++---- .../components/URLShortLinkButton/index.tsx | 13 +- .../components/gridComponents/Chart.jsx | 879 +++++++++--------- .../components/gridComponents/Chart.test.jsx | 112 ++- .../components/gridComponents/ChartHolder.tsx | 225 +++-- .../src/dashboard/containers/Chart.jsx | 135 --- 6 files changed, 836 insertions(+), 870 deletions(-) delete mode 100644 superset-frontend/src/dashboard/containers/Chart.jsx diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index b281ff320..fb84bf398 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -16,7 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import { FC, ReactNode, useContext, useEffect, useRef, useState } from 'react'; +import { + forwardRef, + ReactNode, + useContext, + useEffect, + useRef, + useState, +} from 'react'; import { css, getExtensionsRegistry, styled, t } from '@superset-ui/core'; import { useUiConfig } from 'src/components/UiConfigContext'; import { Tooltip } from 'src/components/Tooltip'; @@ -34,7 +41,6 @@ import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; const extensionsRegistry = getExtensionsRegistry(); type SliceHeaderProps = SliceHeaderControlsProps & { - innerRef?: string; updateSliceName?: (arg0: string) => void; editMode?: boolean; annotationQuery?: object; @@ -122,176 +128,182 @@ const ChartHeaderStyles = styled.div` `} `; -const SliceHeader: FC = ({ - innerRef = null, - forceRefresh = () => ({}), - updateSliceName = () => ({}), - toggleExpandSlice = () => ({}), - logExploreChart = () => ({}), - logEvent, - exportCSV = () => ({}), - exportXLSX = () => ({}), - editMode = false, - annotationQuery = {}, - annotationError = {}, - cachedDttm = null, - updatedDttm = null, - isCached = [], - isExpanded = false, - sliceName = '', - supersetCanExplore = false, - supersetCanShare = false, - supersetCanCSV = false, - exportPivotCSV, - exportFullCSV, - exportFullXLSX, - slice, - componentId, - dashboardId, - addSuccessToast, - addDangerToast, - handleToggleFullSize, - isFullSize, - chartStatus, - formData, - width, - height, -}) => { - const SliceHeaderExtension = extensionsRegistry.get('dashboard.slice.header'); - const uiConfig = useUiConfig(); - const dashboardPageId = useContext(DashboardPageIdContext); - const [headerTooltip, setHeaderTooltip] = useState(null); - const headerRef = useRef(null); - // TODO: change to indicator field after it will be implemented - const crossFilterValue = useSelector( - state => state.dataMask[slice?.slice_id]?.filterState?.value, - ); - const isCrossFiltersEnabled = useSelector( - ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled, - ); +const SliceHeader = forwardRef( + ( + { + forceRefresh = () => ({}), + updateSliceName = () => ({}), + toggleExpandSlice = () => ({}), + logExploreChart = () => ({}), + logEvent, + exportCSV = () => ({}), + exportXLSX = () => ({}), + editMode = false, + annotationQuery = {}, + annotationError = {}, + cachedDttm = null, + updatedDttm = null, + isCached = [], + isExpanded = false, + sliceName = '', + supersetCanExplore = false, + supersetCanShare = false, + supersetCanCSV = false, + exportPivotCSV, + exportFullCSV, + exportFullXLSX, + slice, + componentId, + dashboardId, + addSuccessToast, + addDangerToast, + handleToggleFullSize, + isFullSize, + chartStatus, + formData, + width, + height, + }, + ref, + ) => { + const SliceHeaderExtension = extensionsRegistry.get( + 'dashboard.slice.header', + ); + const uiConfig = useUiConfig(); + const dashboardPageId = useContext(DashboardPageIdContext); + const [headerTooltip, setHeaderTooltip] = useState(null); + const headerRef = useRef(null); + // TODO: change to indicator field after it will be implemented + const crossFilterValue = useSelector( + state => state.dataMask[slice?.slice_id]?.filterState?.value, + ); + const isCrossFiltersEnabled = useSelector( + ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled, + ); - const canExplore = !editMode && supersetCanExplore; + const canExplore = !editMode && supersetCanExplore; - useEffect(() => { - const headerElement = headerRef.current; - if (canExplore) { - setHeaderTooltip(getSliceHeaderTooltip(sliceName)); - } else if ( - headerElement && - (headerElement.scrollWidth > headerElement.offsetWidth || - headerElement.scrollHeight > headerElement.offsetHeight) - ) { - setHeaderTooltip(sliceName ?? null); - } else { - setHeaderTooltip(null); - } - }, [sliceName, width, height, canExplore]); + useEffect(() => { + const headerElement = headerRef.current; + if (canExplore) { + setHeaderTooltip(getSliceHeaderTooltip(sliceName)); + } else if ( + headerElement && + (headerElement.scrollWidth > headerElement.offsetWidth || + headerElement.scrollHeight > headerElement.offsetHeight) + ) { + setHeaderTooltip(sliceName ?? null); + } else { + setHeaderTooltip(null); + } + }, [sliceName, width, height, canExplore]); - const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`; + const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`; - return ( - -
- - - - {!!Object.values(annotationQuery).length && ( - - +
+ + - )} - {!!Object.values(annotationError).length && ( - - - - )} -
-
- {!editMode && ( - <> - {SliceHeaderExtension && ( - + - )} - {crossFilterValue && ( - - - - )} - {!uiConfig.hideChartControls && ( - - )} - {!uiConfig.hideChartControls && ( - + )} + {!!Object.values(annotationError).length && ( + + - )} - - )} -
- - ); -}; + + )} +
+
+ {!editMode && ( + <> + {SliceHeaderExtension && ( + + )} + {crossFilterValue && ( + + + + )} + {!uiConfig.hideChartControls && ( + + )} + {!uiConfig.hideChartControls && ( + + )} + + )} +
+ + ); + }, +); export default SliceHeader; diff --git a/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx b/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx index 849b1cd44..f227a9e84 100644 --- a/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx +++ b/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx @@ -22,7 +22,7 @@ import Popover, { PopoverProps } from 'src/components/Popover'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { getDashboardPermalink } from 'src/utils/urlUtils'; import { useToasts } from 'src/components/MessageToasts/withToasts'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import { RootState } from 'src/dashboard/types'; export type URLShortLinkButtonProps = { @@ -42,10 +42,13 @@ export default function URLShortLinkButton({ }: URLShortLinkButtonProps) { const [shortUrl, setShortUrl] = useState(''); const { addDangerToast } = useToasts(); - const { dataMask, activeTabs } = useSelector((state: RootState) => ({ - dataMask: state.dataMask, - activeTabs: state.dashboardState.activeTabs, - })); + const { dataMask, activeTabs } = useSelector( + (state: RootState) => ({ + dataMask: state.dataMask, + activeTabs: state.dashboardState.activeTabs, + }), + shallowEqual, + ); const getCopyUrl = async () => { try { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index d125ccb62..5cc8ba6d3 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -17,11 +17,13 @@ * under the License. */ import cx from 'classnames'; -import { Component } from 'react'; +import { useCallback, useEffect, useRef, useMemo, useState, memo } from 'react'; import PropTypes from 'prop-types'; import { styled, t, logging } from '@superset-ui/core'; -import { debounce, isEqual } from 'lodash'; -import { withRouter } from 'react-router-dom'; +import { debounce } from 'lodash'; +import { useHistory } from 'react-router-dom'; +import { bindActionCreators } from 'redux'; +import { useDispatch, useSelector } from 'react-redux'; import { exportChart, mountExploreUrl } from 'src/explore/exploreUtils'; import ChartContainer from 'src/components/Chart/ChartContainer'; @@ -32,13 +34,30 @@ import { LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART, LOG_ACTIONS_FORCE_REFRESH_CHART, } from 'src/logger/LogUtils'; -import { areObjectsEqual } from 'src/reduxUtils'; import { postFormData } from 'src/explore/exploreUtils/formData'; import { URL_PARAMS } from 'src/constants'; +import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; import SliceHeader from '../SliceHeader'; import MissingChart from '../MissingChart'; -import { slicePropShape, chartPropShape } from '../../util/propShapes'; +import { + addDangerToast, + addSuccessToast, +} from '../../../components/MessageToasts/actions'; +import { + setFocusedFilterField, + toggleExpandSlice, + unsetFocusedFilterField, +} from '../../actions/dashboardState'; +import { changeFilter } from '../../actions/dashboardFilters'; +import { refreshChart } from '../../../components/Chart/chartAction'; +import { logEvent } from '../../../logger/actions'; +import { + getActiveFilters, + getAppliedFilterValues, +} from '../../util/activeDashboardFilters'; +import getFormDataWithExtraFilters from '../../util/charts/getFormDataWithExtraFilters'; +import { PLACEHOLDER_DATASOURCE } from '../../constants'; const propTypes = { id: PropTypes.number.isRequired, @@ -50,53 +69,15 @@ const propTypes = { isComponentVisible: PropTypes.bool, handleToggleFullSize: PropTypes.func.isRequired, setControlValue: PropTypes.func, - - // from redux - chart: chartPropShape.isRequired, - formData: PropTypes.object.isRequired, - labelsColor: PropTypes.object, - labelsColorMap: PropTypes.object, - datasource: PropTypes.object, - slice: slicePropShape.isRequired, sliceName: PropTypes.string.isRequired, - timeout: PropTypes.number.isRequired, - maxRows: PropTypes.number.isRequired, - // all active filter fields in dashboard - filters: PropTypes.object.isRequired, - refreshChart: PropTypes.func.isRequired, - logEvent: PropTypes.func.isRequired, - toggleExpandSlice: PropTypes.func.isRequired, - changeFilter: PropTypes.func.isRequired, - setFocusedFilterField: PropTypes.func.isRequired, - unsetFocusedFilterField: PropTypes.func.isRequired, - editMode: PropTypes.bool.isRequired, - isExpanded: PropTypes.bool.isRequired, - isCached: PropTypes.bool, - supersetCanExplore: PropTypes.bool.isRequired, - supersetCanShare: PropTypes.bool.isRequired, - supersetCanCSV: PropTypes.bool.isRequired, - addSuccessToast: PropTypes.func.isRequired, - addDangerToast: PropTypes.func.isRequired, - ownState: PropTypes.object, - filterState: PropTypes.object, - postTransformProps: PropTypes.func, - datasetsStatus: PropTypes.oneOf(['loading', 'error', 'complete']), + isFullSize: PropTypes.bool, + extraControls: PropTypes.object, isInView: PropTypes.bool, - emitCrossFilters: PropTypes.bool, -}; - -const defaultProps = { - isCached: false, - isComponentVisible: true, }; // we use state + shouldComponentUpdate() logic to prevent perf-wrecking // resizing across all slices on a dashboard on every update const RESIZE_TIMEOUT = 500; -const SHOULD_UPDATE_ON_PROP_CHANGES = Object.keys(propTypes).filter( - prop => - prop !== 'width' && prop !== 'height' && prop !== 'isComponentVisible', -); const DEFAULT_HEADER_HEIGHT = 22; const ChartWrapper = styled.div` @@ -121,429 +102,457 @@ const SliceContainer = styled.div` max-height: 100%; `; -class Chart extends Component { - constructor(props) { - super(props); - this.state = { - width: props.width, - height: props.height, - descriptionHeight: 0, - }; +const EMPTY_OBJECT = {}; - this.changeFilter = this.changeFilter.bind(this); - this.handleFilterMenuOpen = this.handleFilterMenuOpen.bind(this); - this.handleFilterMenuClose = this.handleFilterMenuClose.bind(this); - this.exportCSV = this.exportCSV.bind(this); - this.exportPivotCSV = this.exportPivotCSV.bind(this); - this.exportFullCSV = this.exportFullCSV.bind(this); - this.exportXLSX = this.exportXLSX.bind(this); - this.exportFullXLSX = this.exportFullXLSX.bind(this); - this.forceRefresh = this.forceRefresh.bind(this); - this.resize = debounce(this.resize.bind(this), RESIZE_TIMEOUT); - this.setDescriptionRef = this.setDescriptionRef.bind(this); - this.setHeaderRef = this.setHeaderRef.bind(this); - this.getChartHeight = this.getChartHeight.bind(this); - this.getDescriptionHeight = this.getDescriptionHeight.bind(this); - } +const Chart = props => { + const dispatch = useDispatch(); + const descriptionRef = useRef(null); + const headerRef = useRef(null); - shouldComponentUpdate(nextProps, nextState) { - // this logic mostly pertains to chart resizing. we keep a copy of the dimensions in - // state so that we can buffer component size updates and only update on the final call - // which improves performance significantly - if ( - nextState.width !== this.state.width || - nextState.height !== this.state.height || - nextState.descriptionHeight !== this.state.descriptionHeight || - !isEqual(nextProps.datasource, this.props.datasource) - ) { - return true; + const boundActionCreators = useMemo( + () => + bindActionCreators( + { + addSuccessToast, + addDangerToast, + toggleExpandSlice, + changeFilter, + setFocusedFilterField, + unsetFocusedFilterField, + refreshChart, + logEvent, + }, + dispatch, + ), + [dispatch], + ); + + const chart = useSelector(state => state.charts[props.id] || EMPTY_OBJECT); + const slice = useSelector( + state => state.sliceEntities.slices[props.id] || EMPTY_OBJECT, + ); + const editMode = useSelector(state => state.dashboardState.editMode); + const isExpanded = useSelector( + state => !!state.dashboardState.expandedSlices[props.id], + ); + const supersetCanExplore = useSelector( + state => !!state.dashboardInfo.superset_can_explore, + ); + const supersetCanShare = useSelector( + state => !!state.dashboardInfo.superset_can_share, + ); + const supersetCanCSV = useSelector( + state => !!state.dashboardInfo.superset_can_csv, + ); + const timeout = useSelector( + state => state.dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, + ); + const emitCrossFilters = useSelector( + state => !!state.dashboardInfo.crossFiltersEnabled, + ); + const datasource = useSelector( + state => + (chart && + chart.form_data && + state.datasources[chart.form_data.datasource]) || + PLACEHOLDER_DATASOURCE, + ); + + const [descriptionHeight, setDescriptionHeight] = useState(0); + const [height, setHeight] = useState(props.height); + const [width, setWidth] = useState(props.width); + const history = useHistory(); + const resize = useCallback( + debounce(() => { + const { width, height } = props; + setHeight(height); + setWidth(width); + }, RESIZE_TIMEOUT), + [props.width, props.height], + ); + + const ownColorScheme = chart.form_data?.color_scheme; + + const addFilter = useCallback( + (newSelectedValues = {}) => { + boundActionCreators.logEvent(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, { + id: chart.id, + columns: Object.keys(newSelectedValues).filter( + key => newSelectedValues[key] !== null, + ), + }); + boundActionCreators.changeFilter(chart.id, newSelectedValues); + }, + [boundActionCreators.logEvent, boundActionCreators.changeFilter, chart.id], + ); + + useEffect(() => { + if (isExpanded) { + const descriptionHeight = + isExpanded && descriptionRef.current + ? descriptionRef.current?.offsetHeight + : 0; + setDescriptionHeight(descriptionHeight); } + }, [isExpanded]); - // allow chart to update if the status changed and the previous status was loading. - if ( - this.props?.chart?.chartStatus !== nextProps?.chart?.chartStatus && - this.props?.chart?.chartStatus === 'loading' - ) { - return true; - } + useEffect( + () => () => { + resize.cancel(); + }, + [resize], + ); - // allow chart update/re-render only if visible: - // under selected tab or no tab layout - if (nextProps.isComponentVisible) { - if (nextProps.chart.triggerQuery) { - return true; - } + useEffect(() => { + resize(); + }, [resize, props.isFullSize]); - if (nextProps.isFullSize !== this.props.isFullSize) { - this.resize(); - return false; - } - - if ( - nextProps.width !== this.props.width || - nextProps.height !== this.props.height || - nextProps.width !== this.state.width || - nextProps.height !== this.state.height - ) { - this.resize(); - } - - for (let i = 0; i < SHOULD_UPDATE_ON_PROP_CHANGES.length; i += 1) { - const prop = SHOULD_UPDATE_ON_PROP_CHANGES[i]; - // use deep objects equality comparison to prevent - // unnecessary updates when objects references change - if (!areObjectsEqual(nextProps[prop], this.props[prop])) { - return true; - } - } - } else if ( - // chart should re-render if color scheme or label colors were changed - nextProps.formData?.color_scheme !== this.props.formData?.color_scheme || - !areObjectsEqual( - nextProps.formData?.label_colors || {}, - this.props.formData?.label_colors || {}, - ) || - !areObjectsEqual( - nextProps.formData?.map_label_colors || {}, - this.props.formData?.map_label_colors || {}, - ) || - !isEqual( - nextProps.formData?.shared_label_colors || [], - this.props.formData?.shared_label_colors || [], - ) - ) { - return true; - } - - // `cacheBusterProp` is injected by react-hot-loader - return this.props.cacheBusterProp !== nextProps.cacheBusterProp; - } - - componentDidMount() { - if (this.props.isExpanded) { - const descriptionHeight = this.getDescriptionHeight(); - this.setState({ descriptionHeight }); - } - } - - componentWillUnmount() { - this.resize.cancel(); - } - - componentDidUpdate(prevProps) { - if (this.props.isExpanded !== prevProps.isExpanded) { - const descriptionHeight = this.getDescriptionHeight(); - // eslint-disable-next-line react/no-did-update-set-state - this.setState({ descriptionHeight }); - } - } - - getDescriptionHeight() { - return this.props.isExpanded && this.descriptionRef - ? this.descriptionRef.offsetHeight - : 0; - } - - getChartHeight() { - const headerHeight = this.getHeaderHeight(); - return Math.max( - this.state.height - headerHeight - this.state.descriptionHeight, - 20, - ); - } - - getHeaderHeight() { - if (this.headerRef) { - const computedStyle = getComputedStyle(this.headerRef).getPropertyValue( - 'margin-bottom', - ); + const getHeaderHeight = useCallback(() => { + if (headerRef.current) { + const computedStyle = getComputedStyle( + headerRef.current, + ).getPropertyValue('margin-bottom'); const marginBottom = parseInt(computedStyle, 10) || 0; - return this.headerRef.offsetHeight + marginBottom; + return headerRef.current.offsetHeight + marginBottom; } return DEFAULT_HEADER_HEIGHT; - } + }, [headerRef]); - setDescriptionRef(ref) { - this.descriptionRef = ref; - } + const getChartHeight = useCallback(() => { + const headerHeight = getHeaderHeight(); + return Math.max(height - headerHeight - descriptionHeight, 20); + }, [getHeaderHeight, height, descriptionHeight]); - setHeaderRef(ref) { - this.headerRef = ref; - } + const handleFilterMenuOpen = useCallback( + (chartId, column) => { + boundActionCreators.setFocusedFilterField(chartId, column); + }, + [boundActionCreators.setFocusedFilterField], + ); - resize() { - const { width, height } = this.props; - this.setState(() => ({ width, height })); - } + const handleFilterMenuClose = useCallback( + (chartId, column) => { + boundActionCreators.unsetFocusedFilterField(chartId, column); + }, + [boundActionCreators.unsetFocusedFilterField], + ); - changeFilter(newSelectedValues = {}) { - this.props.logEvent(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, { - id: this.props.chart.id, - columns: Object.keys(newSelectedValues).filter( - key => newSelectedValues[key] !== null, - ), + const logExploreChart = useCallback(() => { + boundActionCreators.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, { + slice_id: slice.slice_id, + is_cached: props.isCached, }); - this.props.changeFilter(this.props.chart.id, newSelectedValues); - } + }, [boundActionCreators.logEvent, slice.slice_id, props.isCached]); - handleFilterMenuOpen(chartId, column) { - this.props.setFocusedFilterField(chartId, column); - } + const chartConfiguration = useSelector( + state => state.dashboardInfo.metadata?.chart_configuration, + ); + const colorScheme = useSelector(state => state.dashboardState.colorScheme); + const colorNamespace = useSelector( + state => state.dashboardState.colorNamespace, + ); + const datasetsStatus = useSelector( + state => state.dashboardState.datasetsStatus, + ); + const allSliceIds = useSelector(state => state.dashboardState.sliceIds); + const nativeFilters = useSelector(state => state.nativeFilters?.filters); + const dataMask = useSelector(state => state.dataMask); + const labelsColor = useSelector( + state => state.dashboardInfo?.metadata?.label_colors || EMPTY_OBJECT, + ); + const labelsColorMap = useSelector( + state => state.dashboardInfo?.metadata?.map_label_colors || EMPTY_OBJECT, + ); + const sharedLabelsColors = useSelector(state => + enforceSharedLabelsColorsArray( + state.dashboardInfo?.metadata?.shared_label_colors, + ), + ); - handleFilterMenuClose(chartId, column) { - this.props.unsetFocusedFilterField(chartId, column); - } - - logExploreChart = () => { - this.props.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - }; - - onExploreChart = async clickEvent => { - const isOpenInNewTab = - clickEvent.shiftKey || clickEvent.ctrlKey || clickEvent.metaKey; - try { - const lastTabId = window.localStorage.getItem('last_tab_id'); - const nextTabId = lastTabId - ? String(Number.parseInt(lastTabId, 10) + 1) - : undefined; - const key = await postFormData( - this.props.datasource.id, - this.props.datasource.type, - this.props.formData, - this.props.slice.slice_id, - nextTabId, - ); - const url = mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - [URL_PARAMS.sliceId.name]: this.props.slice.slice_id, - }); - if (isOpenInNewTab) { - window.open(url, '_blank', 'noreferrer'); - } else { - this.props.history.push(url); - } - } catch (error) { - logging.error(error); - this.props.addDangerToast(t('An error occurred while opening Explore')); - } - }; - - exportFullCSV() { - this.exportCSV(true); - } - - exportCSV(isFullCSV = false) { - this.exportTable('csv', isFullCSV); - } - - exportPivotCSV() { - this.exportTable('csv', false, true); - } - - exportXLSX() { - this.exportTable('xlsx', false); - } - - exportFullXLSX() { - this.exportTable('xlsx', true); - } - - exportTable(format, isFullCSV, isPivot = false) { - const logAction = - format === 'csv' - ? LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART - : LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART; - this.props.logEvent(logAction, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - exportChart({ - formData: isFullCSV - ? { ...this.props.formData, row_limit: this.props.maxRows } - : this.props.formData, - resultType: isPivot ? 'post_processed' : 'full', - resultFormat: format, - force: true, - ownState: this.props.ownState, - }); - } - - forceRefresh() { - this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_CHART, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - return this.props.refreshChart( - this.props.chart.id, - true, - this.props.dashboardId, - ); - } - - render() { - const { - id, - componentId, - dashboardId, + const formData = useMemo( + () => + getFormDataWithExtraFilters({ + chart, + chartConfiguration, + filters: getAppliedFilterValues(props.id), + colorScheme, + colorNamespace, + sliceId: props.id, + nativeFilters, + allSliceIds, + dataMask, + extraControls: props.extraControls, + labelsColor, + labelsColorMap, + sharedLabelsColors, + ownColorScheme, + }), + [ chart, - slice, - datasource, - isExpanded, - editMode, - filters, - formData, + chartConfiguration, + props.id, + props.extraControls, + colorScheme, + colorNamespace, + nativeFilters, + allSliceIds, + dataMask, labelsColor, labelsColorMap, - updateSliceName, - sliceName, - toggleExpandSlice, - timeout, - supersetCanExplore, - supersetCanShare, - supersetCanCSV, - addSuccessToast, - addDangerToast, - ownState, - filterState, - handleToggleFullSize, - isFullSize, - setControlValue, - postTransformProps, - datasetsStatus, - isInView, - emitCrossFilters, - logEvent, - } = this.props; + sharedLabelsColors, + ownColorScheme, + ], + ); - const { width } = this.state; - // this prevents throwing in the case that a gridComponent - // references a chart that is not associated with the dashboard - if (!chart || !slice) { - return ; - } + const onExploreChart = useCallback( + async clickEvent => { + const isOpenInNewTab = + clickEvent.shiftKey || clickEvent.ctrlKey || clickEvent.metaKey; + try { + const lastTabId = window.localStorage.getItem('last_tab_id'); + const nextTabId = lastTabId + ? String(Number.parseInt(lastTabId, 10) + 1) + : undefined; + const key = await postFormData( + datasource.id, + datasource.type, + formData, + slice.slice_id, + nextTabId, + ); + const url = mountExploreUrl(null, { + [URL_PARAMS.formDataKey.name]: key, + [URL_PARAMS.sliceId.name]: slice.slice_id, + }); + if (isOpenInNewTab) { + window.open(url, '_blank', 'noreferrer'); + } else { + history.push(url); + } + } catch (error) { + logging.error(error); + boundActionCreators.addDangerToast( + t('An error occurred while opening Explore'), + ); + } + }, + [ + datasource.id, + datasource.type, + formData, + slice.slice_id, + boundActionCreators.addDangerToast, + history, + ], + ); - const { queriesResponse, chartUpdateEndTime, chartStatus } = chart; - const isLoading = chartStatus === 'loading'; + const exportTable = useCallback( + (format, isFullCSV, isPivot = false) => { + const logAction = + format === 'csv' + ? LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART + : LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART; + boundActionCreators.logEvent(logAction, { + slice_id: slice.slice_id, + is_cached: props.isCached, + }); + exportChart({ + formData: isFullCSV + ? { ...formData, row_limit: props.maxRows } + : formData, + resultType: isPivot ? 'post_processed' : 'full', + resultFormat: format, + force: true, + ownState: props.ownState, + }); + }, + [ + slice.slice_id, + props.isCached, + formData, + props.maxRows, + props.ownState, + boundActionCreators.logEvent, + ], + ); + + const exportCSV = useCallback( + (isFullCSV = false) => { + exportTable('csv', isFullCSV); + }, + [exportTable], + ); + + const exportFullCSV = useCallback(() => { + exportCSV(true); + }, [exportCSV]); + + const exportPivotCSV = useCallback(() => { + exportTable('csv', false, true); + }, [exportTable]); + + const exportXLSX = useCallback(() => { + exportTable('xlsx', false); + }, [exportTable]); + + const exportFullXLSX = useCallback(() => { + exportTable('xlsx', true); + }, [exportTable]); + + const forceRefresh = useCallback(() => { + boundActionCreators.logEvent(LOG_ACTIONS_FORCE_REFRESH_CHART, { + slice_id: slice.slice_id, + is_cached: props.isCached, + }); + return boundActionCreators.refreshChart(chart.id, true, props.dashboardId); + }, [ + boundActionCreators.refreshChart, + chart.id, + props.dashboardId, + slice.slice_id, + props.isCached, + boundActionCreators.logEvent, + ]); + + if (chart === EMPTY_OBJECT || slice === EMPTY_OBJECT) { + return ; + } + + const { queriesResponse, chartUpdateEndTime, chartStatus, annotationQuery } = + chart; + const isLoading = chartStatus === 'loading'; + // eslint-disable-next-line camelcase + const isCached = queriesResponse?.map(({ is_cached }) => is_cached) || []; + const cachedDttm = // eslint-disable-next-line camelcase - const isCached = queriesResponse?.map(({ is_cached }) => is_cached) || []; - const cachedDttm = - // eslint-disable-next-line camelcase - queriesResponse?.map(({ cached_dttm }) => cached_dttm) || []; - const initialValues = {}; + queriesResponse?.map(({ cached_dttm }) => cached_dttm) || []; - return ( - - + return ( + + - {/* + {/* This usage of dangerouslySetInnerHTML is safe since it is being used to render markdown that is sanitized with nh3. See: https://github.com/apache/superset/pull/4390 and https://github.com/apache/superset/pull/23862 */} - {isExpanded && slice.description_markeddown && ( -
+ )} + + + {isLoading && ( + )} - - {isLoading && ( - - )} - - - - - ); - } -} + + + + ); +}; Chart.propTypes = propTypes; -Chart.defaultProps = defaultProps; -export default withRouter(Chart); +export default memo(Chart, (prevProps, nextProps) => { + if (prevProps.cacheBusterProp !== nextProps.cacheBusterProp) { + return false; + } + return ( + !nextProps.isComponentVisible || + (prevProps.isInView === nextProps.isInView && + prevProps.componentId === nextProps.componentId && + prevProps.id === nextProps.id && + prevProps.dashboardId === nextProps.dashboardId && + prevProps.extraControls === nextProps.extraControls && + prevProps.handleToggleFullSize === nextProps.handleToggleFullSize && + prevProps.isFullSize === nextProps.isFullSize && + prevProps.setControlValue === nextProps.setControlValue && + prevProps.sliceName === nextProps.sliceName && + prevProps.updateSliceName === nextProps.updateSliceName && + prevProps.width === nextProps.width && + prevProps.height === nextProps.height) + ); +}); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx index e54dac757..223b98d57 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx @@ -18,6 +18,7 @@ */ import { fireEvent, render } from 'spec/helpers/testing-library'; import { FeatureFlag, VizType } from '@superset-ui/core'; +import * as redux from 'redux'; import Chart from 'src/dashboard/components/gridComponents/Chart'; import * as exploreUtils from 'src/explore/exploreUtils'; @@ -32,18 +33,10 @@ const props = { width: 100, height: 100, updateSliceName() {}, - // from redux maxRows: 666, - chart: chartQueries[queryId], formData: chartQueries[queryId].form_data, datasource: mockDatasource[sliceEntities.slices[queryId].datasource], - slice: { - ...sliceEntities.slices[queryId], - description_markeddown: 'markdown', - owners: [], - viz_type: VizType.Table, - }, sliceName: sliceEntities.slices[queryId].slice_name, timeout: 60, filters: {}, @@ -63,20 +56,60 @@ const props = { exportFullXLSX() {}, componentId: 'test', dashboardId: 111, - editMode: false, - isExpanded: false, - supersetCanExplore: false, - supersetCanCSV: false, - supersetCanShare: false, }; -function setup(overrideProps) { - return render(, { +const defaultState = { + charts: chartQueries, + sliceEntities: { + ...sliceEntities, + slices: { + [queryId]: { + ...sliceEntities.slices[queryId], + description_markeddown: 'markdown', + owners: [], + viz_type: VizType.Table, + }, + }, + }, + datasources: mockDatasource, + dashboardState: { editMode: false, expandedSlices: {} }, + dashboardInfo: { + superset_can_explore: false, + superset_can_share: false, + superset_can_csv: false, + common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 0 } }, + }, +}; + +function setup(overrideProps, overrideState) { + return render(, { useRedux: true, useRouter: true, + initialState: { ...defaultState, ...overrideState }, }); } +const refreshChart = jest.fn(); +const logEvent = jest.fn(); +const changeFilter = jest.fn(); +const addSuccessToast = jest.fn(); +const addDangerToast = jest.fn(); +const toggleExpandSlice = jest.fn(); +const setFocusedFilterField = jest.fn(); +const unsetFocusedFilterField = jest.fn(); +beforeAll(() => { + jest.spyOn(redux, 'bindActionCreators').mockImplementation(() => ({ + refreshChart, + logEvent, + changeFilter, + addSuccessToast, + addDangerToast, + toggleExpandSlice, + setFocusedFilterField, + unsetFocusedFilterField, + })); +}); + test('should render a SliceHeader', () => { const { getByTestId, container } = setup(); expect(getByTestId('slice-header')).toBeInTheDocument(); @@ -89,23 +122,20 @@ test('should render a ChartContainer', () => { }); test('should render a description if it has one and isExpanded=true', () => { - const { container } = setup({ isExpanded: true }); - expect(container.querySelector('.slice_description')).toBeInTheDocument(); -}); - -test('should calculate the description height if it has one and isExpanded=true', () => { - const spy = jest.spyOn( - Chart.WrappedComponent.prototype, - 'getDescriptionHeight', + const { container } = setup( + {}, + { + dashboardState: { + ...defaultState.dashboardState, + expandedSlices: { [props.id]: true }, + }, + }, ); - const { container } = setup({ isExpanded: true }); expect(container.querySelector('.slice_description')).toBeInTheDocument(); - expect(spy).toHaveBeenCalled(); }); test('should call refreshChart when SliceHeader calls forceRefresh', () => { - const refreshChart = jest.fn(); - const { getByText, getByRole } = setup({ refreshChart }); + const { getByText, getByRole } = setup({}); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.click(getByText('Force refresh')); expect(refreshChart).toHaveBeenCalled(); @@ -122,7 +152,12 @@ test('should call exportChart when exportCSV is clicked', async () => { const stubbedExportCSV = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to .CSV'); @@ -145,7 +180,12 @@ test('should call exportChart with row_limit props.maxRows when exportFullCSV is const stubbedExportCSV = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to full .CSV'); @@ -167,7 +207,12 @@ test('should call exportChart when exportXLSX is clicked', async () => { const stubbedExportXLSX = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to Excel'); @@ -189,7 +234,12 @@ test('should call exportChart with row_limit props.maxRows when exportFullXLSX i const stubbedExportXLSX = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to full Excel'); diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx index d31e240e4..887f1baa7 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useMemo, useCallback, useEffect } from 'react'; +import { useState, useMemo, useCallback, useEffect, memo } from 'react'; import { ResizeCallback, ResizeStartCallback } from 're-resizable'; import cx from 'classnames'; @@ -24,7 +24,7 @@ import { useSelector } from 'react-redux'; import { css, useTheme } from '@superset-ui/core'; import { LayoutItem, RootState } from 'src/dashboard/types'; import AnchorLink from 'src/dashboard/components/AnchorLink'; -import Chart from 'src/dashboard/containers/Chart'; +import Chart from 'src/dashboard/components/gridComponents/Chart'; import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; import { Draggable } from 'src/dashboard/components/dnd/DragDroppable'; import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; @@ -70,7 +70,7 @@ interface ChartHolderProps { isInView: boolean; } -const ChartHolder: React.FC = ({ +const ChartHolder = ({ id, parentId, component, @@ -92,7 +92,7 @@ const ChartHolder: React.FC = ({ handleComponentDrop, setFullSizeChartId, isInView, -}) => { +}: ChartHolderProps) => { const theme = useTheme(); const fullSizeStyle = css` && { @@ -107,9 +107,13 @@ const ChartHolder: React.FC = ({ const isFullSize = fullSizeChartId === chartId; const focusHighlightStyles = useFilterFocusHighlightStyles(chartId); - const dashboardState = useSelector( - (state: RootState) => state.dashboardState, + const directPathToChild = useSelector( + (state: RootState) => state.dashboardState.directPathToChild, ); + const directPathLastUpdated = useSelector( + (state: RootState) => state.dashboardState.directPathLastUpdated ?? 0, + ); + const [extraControls, setExtraControls] = useState>( {}, ); @@ -118,18 +122,8 @@ const ChartHolder: React.FC = ({ const [currentDirectPathLastUpdated, setCurrentDirectPathLastUpdated] = useState(0); - const directPathToChild = useMemo( - () => dashboardState?.directPathToChild ?? [], - [dashboardState], - ); - - const directPathLastUpdated = useMemo( - () => dashboardState?.directPathLastUpdated ?? 0, - [dashboardState], - ); - const infoFromPath = useMemo( - () => getChartAndLabelComponentIdFromPath(directPathToChild) as any, + () => getChartAndLabelComponentIdFromPath(directPathToChild ?? []) as any, [directPathToChild], ); @@ -191,26 +185,26 @@ const ChartHolder: React.FC = ({ ]); const { chartWidth, chartHeight } = useMemo(() => { - let chartWidth = 0; - let chartHeight = 0; + let width = 0; + let height = 0; if (isFullSize) { - chartWidth = window.innerWidth - CHART_MARGIN; - chartHeight = window.innerHeight - CHART_MARGIN; + width = window.innerWidth - CHART_MARGIN; + height = window.innerHeight - CHART_MARGIN; } else { - chartWidth = Math.floor( + width = Math.floor( widthMultiple * columnWidth + (widthMultiple - 1) * GRID_GUTTER_SIZE - CHART_MARGIN, ); - chartHeight = Math.floor( + height = Math.floor( component.meta.height * GRID_BASE_UNIT - CHART_MARGIN, ); } return { - chartWidth, - chartHeight, + chartWidth: width, + chartHeight: height, }; }, [columnWidth, component, isFullSize, widthMultiple]); @@ -244,6 +238,111 @@ const ChartHolder: React.FC = ({ })); }, []); + const renderChild = useCallback( + ({ dragSourceRef }) => ( + +
+ {!editMode && ( + + )} + {!!outlinedComponentId && ( + + )} + + {editMode && ( + +
+ +
+ + )} +
+ + ), + [ + component.id, + component.meta.height, + component.meta.chartId, + component.meta.sliceNameOverride, + component.meta.sliceName, + parentComponent.type, + columnWidth, + widthMultiple, + availableColumnCount, + onResizeStart, + onResize, + onResizeStop, + editMode, + focusHighlightStyles, + isFullSize, + fullSizeStyle, + chartId, + outlinedComponentId, + outlinedColumnName, + dashboardId, + chartWidth, + chartHeight, + handleUpdateSliceName, + isComponentVisible, + handleToggleFullSize, + handleExtraControl, + extraControls, + isInView, + handleDeleteComponent, + ], + ); + return ( = ({ disableDragDrop={false} editMode={editMode} > - {({ dragSourceRef }) => ( - -
- {!editMode && ( - - )} - {!!outlinedComponentId && ( - - )} - - {editMode && ( - -
- -
- - )} -
- - )} + {renderChild} ); }; -export default ChartHolder; +export default memo(ChartHolder); diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx deleted file mode 100644 index 9f00bd0bf..000000000 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ /dev/null @@ -1,135 +0,0 @@ -/** - * 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 { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; -import { - toggleExpandSlice, - setFocusedFilterField, - unsetFocusedFilterField, -} from 'src/dashboard/actions/dashboardState'; -import { updateComponents } from 'src/dashboard/actions/dashboardLayout'; -import { changeFilter } from 'src/dashboard/actions/dashboardFilters'; -import { - addSuccessToast, - addDangerToast, -} from 'src/components/MessageToasts/actions'; -import { refreshChart } from 'src/components/Chart/chartAction'; -import { logEvent } from 'src/logger/actions'; -import { - getActiveFilters, - getAppliedFilterValues, -} from 'src/dashboard/util/activeDashboardFilters'; -import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; -import Chart from 'src/dashboard/components/gridComponents/Chart'; -import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants'; -import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; - -const EMPTY_OBJECT = {}; - -function mapStateToProps( - { - charts: chartQueries, - dashboardInfo, - dashboardState, - dataMask, - datasources, - sliceEntities, - nativeFilters, - common, - }, - ownProps, -) { - const { id, extraControls, setControlValue } = ownProps; - const chart = chartQueries[id] || EMPTY_OBJECT; - const datasource = - (chart && chart.form_data && datasources[chart.form_data.datasource]) || - PLACEHOLDER_DATASOURCE; - const { - colorScheme: appliedColorScheme, - colorNamespace, - datasetsStatus, - } = dashboardState; - const labelsColor = dashboardInfo?.metadata?.label_colors || {}; - const labelsColorMap = dashboardInfo?.metadata?.map_label_colors || {}; - const sharedLabelsColors = enforceSharedLabelsColorsArray( - dashboardInfo?.metadata?.shared_label_colors, - ); - const ownColorScheme = chart.form_data?.color_scheme; - // note: this method caches filters if possible to prevent render cascades - const formData = getFormDataWithExtraFilters({ - chart, - chartConfiguration: dashboardInfo.metadata?.chart_configuration, - charts: chartQueries, - filters: getAppliedFilterValues(id), - colorNamespace, - colorScheme: appliedColorScheme, - ownColorScheme, - sliceId: id, - nativeFilters: nativeFilters?.filters, - allSliceIds: dashboardState.sliceIds, - dataMask, - extraControls, - labelsColor, - labelsColorMap, - sharedLabelsColors, - }); - - formData.dashboardId = dashboardInfo.id; - - return { - chart, - datasource, - labelsColor, - labelsColorMap, - slice: sliceEntities.slices[id], - timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, - filters: getActiveFilters() || EMPTY_OBJECT, - formData, - editMode: dashboardState.editMode, - isExpanded: !!dashboardState.expandedSlices[id], - supersetCanExplore: !!dashboardInfo.superset_can_explore, - supersetCanShare: !!dashboardInfo.superset_can_share, - supersetCanCSV: !!dashboardInfo.superset_can_csv, - ownState: dataMask[id]?.ownState, - filterState: dataMask[id]?.filterState, - maxRows: common.conf.SQL_MAX_ROW, - setControlValue, - datasetsStatus, - emitCrossFilters: !!dashboardInfo.crossFiltersEnabled, - }; -} - -function mapDispatchToProps(dispatch) { - return bindActionCreators( - { - updateComponents, - addSuccessToast, - addDangerToast, - toggleExpandSlice, - changeFilter, - setFocusedFilterField, - unsetFocusedFilterField, - refreshChart, - logEvent, - }, - dispatch, - ); -} - -export default connect(mapStateToProps, mapDispatchToProps)(Chart);