From 59c90f6acd36d98edc5439868d1b0bc2216a974c Mon Sep 17 00:00:00 2001 From: Uladzimir Havenchyk <125459446+uladzimirdev@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:18:33 +0300 Subject: [PATCH] Drop dead code of the dashboard filters stickiness calculation (#35358) --- .../components/Dashboard/Dashboard.jsx | 156 +++++++----------- .../components/Dashboard/stickyParameters.js | 2 +- .../DashboardHeader/DashboardHeader.jsx | 3 +- .../containers/DashboardApp/DashboardApp.jsx | 3 - 4 files changed, 67 insertions(+), 97 deletions(-) diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx index 537408c3b53..bf18b1050c2 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx +++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx @@ -1,10 +1,8 @@ // TODO: merge with metabase/dashboard/containers/Dashboard.jsx -import { createRef, Component } from "react"; +import { Component } from "react"; import PropTypes from "prop-types"; import _ from "underscore"; -import { getMainElement } from "metabase/lib/dom"; - import { DashboardHeader } from "metabase/dashboard/components/DashboardHeader"; import SyncedParametersList from "metabase/parameters/components/SyncedParametersList/SyncedParametersList"; import { FilterApplyButton } from "metabase/parameters/components/FilterApplyButton"; @@ -31,8 +29,6 @@ import { } from "./DashboardEmptyState/DashboardEmptyState"; import { updateParametersWidgetStickiness } from "./stickyParameters"; -const SCROLL_THROTTLE_INTERVAL = 1000 / 24; - // NOTE: move DashboardControls HoC to container class Dashboard extends Component { @@ -42,76 +38,12 @@ class Dashboard extends Component { parametersListLength: 0, }; - static propTypes = { - loadDashboardParams: PropTypes.func, - location: PropTypes.object, - - isAdmin: PropTypes.bool, - isFullscreen: PropTypes.bool, - isNightMode: PropTypes.bool, - isSharing: PropTypes.bool, - isEditing: PropTypes.oneOfType([PropTypes.bool, PropTypes.object]) - .isRequired, - isEditingParameter: PropTypes.bool.isRequired, - isNavbarOpen: PropTypes.bool.isRequired, - isHeaderVisible: PropTypes.bool, - isAdditionalInfoVisible: PropTypes.bool, - isNavigatingBackToDashboard: PropTypes.bool, - - dashboard: PropTypes.object, - dashboardId: PropTypes.number, - dashcardData: PropTypes.object, - selectedTabId: PropTypes.number, - parameters: PropTypes.array, - parameterValues: PropTypes.object, - draftParameterValues: PropTypes.object, - editingParameter: PropTypes.object, - - editingOnLoad: PropTypes.bool, - addCardOnLoad: PropTypes.number, - addCardToDashboard: PropTypes.func.isRequired, - addParameter: PropTypes.func, - archiveDashboard: PropTypes.func.isRequired, - cancelFetchDashboardCardData: PropTypes.func.isRequired, - fetchDashboard: PropTypes.func.isRequired, - fetchDashboardCardData: PropTypes.func.isRequired, - fetchDashboardCardMetadata: PropTypes.func.isRequired, - initialize: PropTypes.func.isRequired, - onRefreshPeriodChange: PropTypes.func, - updateDashboardAndCards: PropTypes.func.isRequired, - setDashboardAttributes: PropTypes.func.isRequired, - setEditingDashboard: PropTypes.func.isRequired, - setErrorPage: PropTypes.func, - setSharing: PropTypes.func.isRequired, - setParameterValue: PropTypes.func.isRequired, - setEditingParameter: PropTypes.func.isRequired, - setParameterIndex: PropTypes.func.isRequired, - - onUpdateDashCardVisualizationSettings: PropTypes.func.isRequired, - onUpdateDashCardColumnSettings: PropTypes.func.isRequired, - onReplaceAllDashCardVisualizationSettings: PropTypes.func.isRequired, - - onChangeLocation: PropTypes.func.isRequired, - onSharingClick: PropTypes.func, - onEmbeddingClick: PropTypes.any, - sidebar: PropTypes.shape({ - name: PropTypes.string, - props: PropTypes.object, - }).isRequired, - toggleSidebar: PropTypes.func.isRequired, - closeSidebar: PropTypes.func.isRequired, - closeNavbar: PropTypes.func.isRequired, - embedOptions: PropTypes.object, - isAutoApplyFilters: PropTypes.bool, - }; - static defaultProps = { isSharing: false, }; constructor(props) { super(props); - this.parametersWidgetRef = createRef(); } static getDerivedStateFromProps({ parameters }, { parametersListLength }) { @@ -121,28 +53,15 @@ class Dashboard extends Component { : null; } - throttleParameterWidgetStickiness = _.throttle( - () => updateParametersWidgetStickiness(this), - SCROLL_THROTTLE_INTERVAL, - ); - - // NOTE: all of these lifecycle methods should be replaced with DashboardData HoC in container async componentDidMount() { await this.loadDashboard(this.props.dashboardId); - - const main = getMainElement(); - main.addEventListener("scroll", this.throttleParameterWidgetStickiness, { - passive: true, - }); - main.addEventListener("resize", this.throttleParameterWidgetStickiness, { - passive: true, - }); } async componentDidUpdate(prevProps) { + updateParametersWidgetStickiness(this); + if (prevProps.dashboardId !== this.props.dashboardId) { await this.loadDashboard(this.props.dashboardId); - this.throttleParameterWidgetStickiness(); return; } @@ -162,9 +81,6 @@ class Dashboard extends Component { componentWillUnmount() { this.props.cancelFetchDashboardCardData(); - const main = getMainElement(); - main.removeEventListener("scroll", this.throttleParameterWidgetStickiness); - main.removeEventListener("resize", this.throttleParameterWidgetStickiness); } async loadDashboard(dashboardId) { @@ -296,13 +212,11 @@ class Dashboard extends Component { parameters, parameterValues, draftParameterValues, - isNavbarOpen, editingParameter, setParameterValue, setParameterIndex, setEditingParameter, isHeaderVisible, - embedOptions, isAutoApplyFilters, } = this.props; @@ -384,10 +298,7 @@ class Dashboard extends Component { {shouldRenderParametersWidgetInViewMode && ( <ParametersWidgetContainer data-testid="dashboard-parameters-widget-container" - ref={this.parametersWidgetRef} - isNavbarOpen={isNavbarOpen} isSticky={isParametersWidgetSticky} - topNav={embedOptions?.top_nav} > {parametersWidget} <FilterApplyButton /> @@ -415,4 +326,65 @@ class Dashboard extends Component { } } +Dashboard.propTypes = { + loadDashboardParams: PropTypes.func, + location: PropTypes.object, + + isAdmin: PropTypes.bool, + isFullscreen: PropTypes.bool, + isNightMode: PropTypes.bool, + isSharing: PropTypes.bool, + isEditing: PropTypes.oneOfType([PropTypes.bool, PropTypes.object]).isRequired, + isEditingParameter: PropTypes.bool.isRequired, + isNavbarOpen: PropTypes.bool.isRequired, + isHeaderVisible: PropTypes.bool, + isAdditionalInfoVisible: PropTypes.bool, + isNavigatingBackToDashboard: PropTypes.bool, + + dashboard: PropTypes.object, + dashboardId: PropTypes.number, + dashcardData: PropTypes.object, + selectedTabId: PropTypes.number, + parameters: PropTypes.array, + parameterValues: PropTypes.object, + draftParameterValues: PropTypes.object, + editingParameter: PropTypes.object, + + editingOnLoad: PropTypes.bool, + addCardOnLoad: PropTypes.number, + addCardToDashboard: PropTypes.func.isRequired, + addParameter: PropTypes.func, + archiveDashboard: PropTypes.func.isRequired, + cancelFetchDashboardCardData: PropTypes.func.isRequired, + fetchDashboard: PropTypes.func.isRequired, + fetchDashboardCardData: PropTypes.func.isRequired, + fetchDashboardCardMetadata: PropTypes.func.isRequired, + initialize: PropTypes.func.isRequired, + onRefreshPeriodChange: PropTypes.func, + updateDashboardAndCards: PropTypes.func.isRequired, + setDashboardAttributes: PropTypes.func.isRequired, + setEditingDashboard: PropTypes.func.isRequired, + setErrorPage: PropTypes.func, + setSharing: PropTypes.func.isRequired, + setParameterValue: PropTypes.func.isRequired, + setEditingParameter: PropTypes.func.isRequired, + setParameterIndex: PropTypes.func.isRequired, + + onUpdateDashCardVisualizationSettings: PropTypes.func.isRequired, + onUpdateDashCardColumnSettings: PropTypes.func.isRequired, + onReplaceAllDashCardVisualizationSettings: PropTypes.func.isRequired, + + onChangeLocation: PropTypes.func.isRequired, + onSharingClick: PropTypes.func, + onEmbeddingClick: PropTypes.any, + sidebar: PropTypes.shape({ + name: PropTypes.string, + props: PropTypes.object, + }).isRequired, + toggleSidebar: PropTypes.func.isRequired, + closeSidebar: PropTypes.func.isRequired, + closeNavbar: PropTypes.func.isRequired, + isAutoApplyFilters: PropTypes.bool, +}; + export default DashboardControls(Dashboard); diff --git a/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js index deffdf06e8c..e5977897052 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js +++ b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js @@ -2,7 +2,7 @@ import { isSmallScreen } from "metabase/lib/dom"; export const MAXIMUM_PARAMETERS_FOR_STICKINESS = 5; -// Dashboard Filters should always be sticky, except the case with small screen: +// Dashboard filters container should always be sticky, except the case with small screen: // if more than MAXIMUM_PARAMETERS_FOR_STICKINESS parameters exist, we do not stick them to avoid // taking to much space on the screen export const updateParametersWidgetStickiness = dashboard => { diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.jsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.jsx index eadd52daef4..e46a0c39737 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.jsx @@ -554,6 +554,7 @@ class DashboardHeaderContainer extends Component { collection, isEditing, isFullscreen, + isNavBarOpen, isAdditionalInfoVisible, setDashboardAttribute, setSidebar, @@ -577,7 +578,7 @@ class DashboardHeaderContainer extends Component { } isLastEditInfoVisible={hasLastEditInfo && isAdditionalInfoVisible} isEditingInfo={isEditing} - isNavBarOpen={this.props.isNavBarOpen} + isNavBarOpen={isNavBarOpen} headerButtons={this.getHeaderButtons()} editWarning={this.getEditWarning(dashboard)} editingTitle={t`You're editing this dashboard.`.concat( diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx index 59b64e8deda..a1c4b4ddf1b 100644 --- a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx +++ b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx @@ -25,8 +25,6 @@ import { getUserIsAdmin, } from "metabase/selectors/user"; -import { getEmbedOptions } from "metabase/selectors/embed"; - import { parseHashOptions } from "metabase/lib/browser"; import * as Urls from "metabase/lib/urls"; @@ -103,7 +101,6 @@ const mapStateToProps = state => { isLoadingComplete: getIsLoadingComplete(state), isHeaderVisible: getIsHeaderVisible(state), isAdditionalInfoVisible: getIsAdditionalInfoVisible(state), - embedOptions: getEmbedOptions(state), selectedTabId: getSelectedTabId(state), isAutoApplyFilters: getIsAutoApplyFilters(state), isNavigatingBackToDashboard: getIsNavigatingBackToDashboard(state), -- GitLab