diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 5bdce6d72ecfa..48e70c8fd468a 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -26,11 +26,7 @@ import getBootstrapData from 'src/utils/getBootstrapData'; import getChartIdsFromLayout from '../util/getChartIdsFromLayout'; import getLayoutComponentFromChartId from '../util/getLayoutComponentFromChartId'; -import { - slicePropShape, - dashboardInfoPropShape, - dashboardStatePropShape, -} from '../util/propShapes'; +import { slicePropShape } from '../util/propShapes'; import { LOG_ACTIONS_HIDE_BROWSER_TAB, LOG_ACTIONS_MOUNT_DASHBOARD, @@ -51,8 +47,10 @@ const propTypes = { logEvent: PropTypes.func.isRequired, clearDataMaskState: PropTypes.func.isRequired, }).isRequired, - dashboardInfo: dashboardInfoPropShape.isRequired, - dashboardState: dashboardStatePropShape.isRequired, + dashboardId: PropTypes.number.isRequired, + editMode: PropTypes.bool, + isPublished: PropTypes.bool, + hasUnsavedChanges: PropTypes.bool, slices: PropTypes.objectOf(slicePropShape).isRequired, activeFilters: PropTypes.object.isRequired, chartConfiguration: PropTypes.object, @@ -96,13 +94,13 @@ class Dashboard extends PureComponent { componentDidMount() { const bootstrapData = getBootstrapData(); - const { dashboardState, layout } = this.props; + const { editMode, isPublished, layout } = this.props; const eventData = { is_soft_navigation: Logger.timeOriginOffset > 0, - is_edit_mode: dashboardState.editMode, + is_edit_mode: editMode, mount_duration: Logger.getTimestamp(), is_empty: isDashboardEmpty(layout), - is_published: dashboardState.isPublished, + is_published: isPublished, bootstrap_data_length: bootstrapData.length, }; const directLinkComponentId = getLocationHash(); @@ -130,7 +128,7 @@ class Dashboard extends PureComponent { const currentChartIds = getChartIdsFromLayout(this.props.layout); const nextChartIds = getChartIdsFromLayout(nextProps.layout); - if (this.props.dashboardInfo.id !== nextProps.dashboardInfo.id) { + if (this.props.dashboardId !== nextProps.dashboardId) { // single-page-app navigation check return; } @@ -157,10 +155,14 @@ class Dashboard extends PureComponent { } applyCharts() { - const { hasUnsavedChanges, editMode } = this.props.dashboardState; - + const { + activeFilters, + ownDataCharts, + chartConfiguration, + hasUnsavedChanges, + editMode, + } = this.props; const { appliedFilters, appliedOwnDataCharts } = this; - const { activeFilters, ownDataCharts, chartConfiguration } = this.props; if ( isFeatureEnabled(FeatureFlag.DashboardCrossFilters) && !chartConfiguration diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 7da2064f96305..43e207d8c8821 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -208,7 +208,9 @@ describe('DashboardBuilder', () => { }); it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { - const { queryAllByTestId } = setup({ dashboardState: { editMode: true } }); + const { queryAllByTestId } = setup({ + dashboardState: { ...mockState.dashboardState, editMode: true }, + }); const builderComponents = queryAllByTestId('mock-builder-component-pane'); expect(builderComponents.length).toBeGreaterThanOrEqual(1); }); @@ -241,7 +243,7 @@ describe('DashboardBuilder', () => { it('should display a loading spinner when saving is in progress', async () => { const { findByAltText } = setup({ - dashboardState: { dashboardIsSaving: true }, + dashboardState: { ...mockState.dashboardState, dashboardIsSaving: true }, }); expect(await findByAltText('Loading...')).toBeVisible(); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 347858f039eaf..30c61f0af6553 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -370,6 +370,10 @@ const StyledDashboardContent = styled.div<{ `} `; +const ELEMENT_ON_SCREEN_OPTIONS = { + threshold: [1], +}; + const DashboardBuilder: FC = () => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); @@ -469,9 +473,9 @@ const DashboardBuilder: FC = () => { nativeFiltersEnabled, } = useNativeFilters(); - const [containerRef, isSticky] = useElementOnScreen({ - threshold: [1], - }); + const [containerRef, isSticky] = useElementOnScreen( + ELEMENT_ON_SCREEN_OPTIONS, + ); const showFilterBar = (crossFiltersEnabled || nativeFiltersEnabled) && !editMode; @@ -581,6 +585,43 @@ const DashboardBuilder: FC = () => { ? 0 : theme.gridUnit * 8; + const renderChild = useCallback( + adjustedWidth => { + const filterBarWidth = dashboardFiltersOpen + ? adjustedWidth + : CLOSED_FILTER_BAR_WIDTH; + return ( + + ); + }, + [ + dashboardFiltersOpen, + toggleDashboardFiltersOpen, + filterBarHeight, + filterBarOffset, + isReport, + ], + ); + return ( {showFilterBar && @@ -593,33 +634,7 @@ const DashboardBuilder: FC = () => { maxWidth={OPEN_FILTER_BAR_MAX_WIDTH} initialWidth={OPEN_FILTER_BAR_WIDTH} > - {adjustedWidth => { - const filterBarWidth = dashboardFiltersOpen - ? adjustedWidth - : CLOSED_FILTER_BAR_WIDTH; - return ( - - ); - }} + {renderChild} )} diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index f36520ba317f5..c333e5318ae29 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -18,8 +18,17 @@ */ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening -import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { + FC, + memo, + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; import { Filter, Filters, @@ -43,6 +52,7 @@ import { import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId'; import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import { applyDashboardLabelsColorOnLoad, updateDashboardLabelsColor, @@ -59,6 +69,21 @@ type DashboardContainerProps = { topLevelTabs?: LayoutItem; }; +export const renderedChartIdsSelector = createSelector( + [(state: RootState) => state.charts], + charts => + Object.values(charts) + .filter(chart => chart.chartStatus === 'rendered') + .map(chart => chart.id), +); + +const useRenderedChartIds = () => { + const renderedChartIds = useSelector( + renderedChartIdsSelector, + ); + return useMemo(() => renderedChartIds, [JSON.stringify(renderedChartIds)]); +}; + const useNativeFilterScopes = () => { const nativeFilters = useSelector( state => state.nativeFilters?.filters, @@ -70,10 +95,12 @@ const useNativeFilterScopes = () => { pick(filter, ['id', 'scope', 'type']), ) : [], - [JSON.stringify(nativeFilters)], + [nativeFilters], ); }; +const TOP_OF_PAGE_RANGE = 220; + const DashboardContainer: FC = ({ topLevelTabs }) => { const nativeFilterScopes = useNativeFilterScopes(); const dispatch = useDispatch(); @@ -87,14 +114,10 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); - const chartIds = useSelector(state => - Object.values(state.charts).map(chart => chart.id), - ); - const renderedChartIds = useSelector(state => - Object.values(state.charts) - .filter(chart => chart.chartStatus === 'rendered') - .map(chart => chart.id), - ); + const chartIds = useChartIds(); + + const renderedChartIds = useRenderedChartIds(); + const [dashboardLabelsColorInitiated, setDashboardLabelsColorInitiated] = useState(false); const prevRenderedChartIds = useRef([]); @@ -136,11 +159,13 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { chartsInScope: [], }; } + const chartsInScope: number[] = getChartIdsInFilterScope( filterScope.scope, chartIds, dashboardLayout, ); + const tabsInScope = findTabsWithChartsInScope( dashboardLayout, chartsInScope, @@ -152,14 +177,14 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }); dispatch(setInScopeStatusOfFilters(scopes)); - }, [nativeFilterScopes, dashboardLayout, dispatch]); + }, [chartIds, JSON.stringify(nativeFilterScopes), dashboardLayout, dispatch]); - const childIds: string[] = topLevelTabs - ? topLevelTabs.children - : [DASHBOARD_GRID_ID]; + const childIds: string[] = useMemo( + () => (topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID]), + [topLevelTabs], + ); const min = Math.min(tabIndex, childIds.length - 1); const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString(); - const TOP_OF_PAGE_RANGE = 220; useEffect(() => { if (shouldForceFreshSharedLabelsColors) { @@ -229,57 +254,63 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }, [onBeforeUnload]); + const renderTabBar = useCallback(() => <>, []); + const handleFocus = useCallback(e => { + if ( + // prevent scrolling when tabbing to the tab pane + e.target.classList.contains('ant-tabs-tabpane') && + window.scrollY < TOP_OF_PAGE_RANGE + ) { + // prevent window from jumping down when tabbing + // if already at the top of the page + // to help with accessibility when using keyboard navigation + window.scrollTo(window.scrollX, 0); + } + }, []); + + const renderParentSizeChildren = useCallback( + ({ width }) => ( + /* + We use a TabContainer irrespective of whether top-level tabs exist to maintain + a consistent React component tree. This avoids expensive mounts/unmounts of + the entire dashboard upon adding/removing top-level tabs, which would otherwise + happen because of React's diffing algorithm + */ + + {childIds.map((id, index) => ( + // Matching the key of the first TabPane irrespective of topLevelTabs + // lets us keep the same React component tree when !!topLevelTabs changes. + // This avoids expensive mounts/unmounts of the entire dashboard. + + + + ))} + + ), + [activeKey, childIds, dashboardLayout, handleFocus, renderTabBar, tabIndex], + ); + return (
- - {({ width }) => ( - /* - We use a TabContainer irrespective of whether top-level tabs exist to maintain - a consistent React component tree. This avoids expensive mounts/unmounts of - the entire dashboard upon adding/removing top-level tabs, which would otherwise - happen because of React's diffing algorithm - */ - <>} - fullWidth={false} - animated={false} - allowOverflow - onFocus={e => { - if ( - // prevent scrolling when tabbing to the tab pane - e.target.classList.contains('ant-tabs-tabpane') && - window.scrollY < TOP_OF_PAGE_RANGE - ) { - // prevent window from jumping down when tabbing - // if already at the top of the page - // to help with accessibility when using keyboard navigation - window.scrollTo(window.scrollX, 0); - } - }} - > - {childIds.map((id, index) => ( - // Matching the key of the first TabPane irrespective of topLevelTabs - // lets us keep the same React component tree when !!topLevelTabs changes. - // This avoids expensive mounts/unmounts of the entire dashboard. - - - - ))} - - )} - + {renderParentSizeChildren}
); }; -export default DashboardContainer; +export default memo(DashboardContainer); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts index b7e6c266de537..ec1cc0bc1f0f8 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts @@ -17,7 +17,7 @@ * under the License. */ import { useSelector } from 'react-redux'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { RootState } from 'src/dashboard/types'; @@ -34,7 +34,7 @@ export const useNativeFilters = () => { ); const filters = useFilters(); - const filterValues = Object.values(filters); + const filterValues = useMemo(() => Object.values(filters), [filters]); const expandFilters = getUrlParam(URL_PARAMS.expandFilters); const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState( expandFilters ?? !!filterValues.length, @@ -43,24 +43,28 @@ export const useNativeFilters = () => { const nativeFiltersEnabled = canEdit || (!canEdit && filterValues.length !== 0); - const requiredFirstFilter = filterValues.filter( - filter => filter.requiredFirst, + const requiredFirstFilter = useMemo( + () => filterValues.filter(filter => filter.requiredFirst), + [filterValues], ); const dataMask = useNativeFiltersDataMask(); - const missingInitialFilters = requiredFirstFilter - .filter(({ id }) => dataMask[id]?.filterState?.value === undefined) - .map(({ name }) => name); + const missingInitialFilters = useMemo( + () => + requiredFirstFilter + .filter(({ id }) => dataMask[id]?.filterState?.value === undefined) + .map(({ name }) => name), + [requiredFirstFilter, dataMask], + ); + const showDashboard = isInitialized || !nativeFiltersEnabled || missingInitialFilters.length === 0; - const toggleDashboardFiltersOpen = useCallback( - (visible?: boolean) => { - setDashboardFiltersOpen(visible ?? !dashboardFiltersOpen); - }, - [dashboardFiltersOpen], - ); + + const toggleDashboardFiltersOpen = useCallback((visible?: boolean) => { + setDashboardFiltersOpen(prevState => visible ?? !prevState); + }, []); useEffect(() => { if ( diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index ab1f7c46fa40a..a59767c004a2e 100644 --- a/superset-frontend/src/dashboard/containers/Dashboard.ts +++ b/superset-frontend/src/dashboard/containers/Dashboard.ts @@ -43,8 +43,10 @@ function mapStateToProps(state: RootState) { return { timeout: dashboardInfo.common?.conf?.SUPERSET_WEBSERVER_TIMEOUT, userId: dashboardInfo.userId, - dashboardInfo, - dashboardState, + dashboardId: dashboardInfo.id, + editMode: dashboardState.editMode, + isPublished: dashboardState.isPublished, + hasUnsavedChanges: dashboardState.hasUnsavedChanges, datasources, chartConfiguration: dashboardInfo.metadata?.chart_configuration, slices: sliceEntities.slices, diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index b811f398abb5a..2bbc51915d046 100644 --- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx @@ -16,16 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import { useCallback, memo, useMemo } from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; +import { useSelector, useDispatch } from 'react-redux'; import { logEvent } from 'src/logger/actions'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { componentLookup } from 'src/dashboard/components/gridComponents'; import getDetailedComponentWidth from 'src/dashboard/util/getDetailedComponentWidth'; import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; -import { componentShape } from 'src/dashboard/util/propShapes'; import { COLUMN_TYPE, ROW_TYPE } from 'src/dashboard/util/componentTypes'; import { createComponent, @@ -47,86 +46,93 @@ const propTypes = { renderHoverMenu: PropTypes.bool, renderTabContent: PropTypes.bool, onChangeTab: PropTypes.func, - component: componentShape.isRequired, - parentComponent: componentShape.isRequired, - createComponent: PropTypes.func.isRequired, - deleteComponent: PropTypes.func.isRequired, - updateComponents: PropTypes.func.isRequired, - handleComponentDrop: PropTypes.func.isRequired, - logEvent: PropTypes.func.isRequired, directPathToChild: PropTypes.arrayOf(PropTypes.string), directPathLastUpdated: PropTypes.number, - dashboardId: PropTypes.number.isRequired, isComponentVisible: PropTypes.bool, }; -const defaultProps = { - isComponentVisible: true, -}; +const DashboardComponent = props => { + const dispatch = useDispatch(); + const dashboardLayout = useSelector(state => state.dashboardLayout.present); + const dashboardInfo = useSelector(state => state.dashboardInfo); + const editMode = useSelector(state => state.dashboardState.editMode); + const fullSizeChartId = useSelector( + state => state.dashboardState.fullSizeChartId, + ); + const dashboardId = dashboardInfo.id; + const component = dashboardLayout[props.id]; + const parentComponent = dashboardLayout[props.parentId]; + const getComponentById = useCallback( + id => dashboardLayout[id], + [dashboardLayout], + ); + const { isComponentVisible = true } = props; + const filters = getActiveFilters(); + const embeddedMode = !dashboardInfo.userId; -function mapStateToProps( - { dashboardLayout: undoableLayout, dashboardState, dashboardInfo }, - ownProps, -) { - const dashboardLayout = undoableLayout.present; - const { id, parentId } = ownProps; - const component = dashboardLayout[id]; - const props = { - component, - getComponentById: id => dashboardLayout[id], - parentComponent: dashboardLayout[parentId], - editMode: dashboardState.editMode, - filters: getActiveFilters(), - dashboardId: dashboardInfo.id, - dashboardInfo, - fullSizeChartId: dashboardState.fullSizeChartId, - embeddedMode: !dashboardInfo?.userId, - }; + const boundActionCreators = useMemo( + () => + bindActionCreators( + { + addDangerToast, + createComponent, + deleteComponent, + updateComponents, + handleComponentDrop, + setDirectPathToChild, + setFullSizeChartId, + setActiveTab, + logEvent, + }, + dispatch, + ), + [dispatch], + ); // rows and columns need more data about their child dimensions // doing this allows us to not pass the entire component lookup to all Components - if (component) { - const componentType = component.type; - if (componentType === ROW_TYPE || componentType === COLUMN_TYPE) { - const { occupiedWidth, minimumWidth } = getDetailedComponentWidth({ - id, - components: dashboardLayout, - }); + const { occupiedColumnCount, minColumnWidth } = useMemo(() => { + if (component) { + const componentType = component.type; + if (componentType === ROW_TYPE || componentType === COLUMN_TYPE) { + const { occupiedWidth, minimumWidth } = getDetailedComponentWidth({ + id: props.id, + components: dashboardLayout, + }); - if (componentType === ROW_TYPE) props.occupiedColumnCount = occupiedWidth; - if (componentType === COLUMN_TYPE) props.minColumnWidth = minimumWidth; + if (componentType === ROW_TYPE) { + return { occupiedColumnCount: occupiedWidth }; + } + if (componentType === COLUMN_TYPE) { + return { minColumnWidth: minimumWidth }; + } + } + return {}; } - } + return {}; + }, [component, dashboardLayout, props.id]); - return props; -} - -function mapDispatchToProps(dispatch) { - return bindActionCreators( - { - addDangerToast, - createComponent, - deleteComponent, - updateComponents, - handleComponentDrop, - setDirectPathToChild, - setFullSizeChartId, - setActiveTab, - logEvent, - }, - dispatch, - ); -} - -class DashboardComponent extends PureComponent { - render() { - const { component } = this.props; - const Component = component ? componentLookup[component.type] : null; - return Component ? : null; - } -} + const Component = component ? componentLookup[component.type] : null; + return Component ? ( + + ) : null; +}; DashboardComponent.propTypes = propTypes; -DashboardComponent.defaultProps = defaultProps; -export default connect(mapStateToProps, mapDispatchToProps)(DashboardComponent); +export default memo(DashboardComponent);