From 24668137f808fb55c6aa55136be2ff9de8b04a42 Mon Sep 17 00:00:00 2001 From: Juan Cabanas Date: Mon, 8 May 2023 16:51:42 -0300 Subject: [PATCH] NavBar: app chrome state wrongly overwritten when ds modal is opened (#67952) --- .../src/components/Modal/ModalsContext.tsx | 2 +- .../DashboardsListModalButton.tsx | 7 ++- .../dashboard/components/DashNav/DashNav.tsx | 44 ++----------------- .../components/DashNav/ShareButton.tsx | 42 ++++++++++++++++++ .../components/ShareModal/ShareModal.tsx | 8 +++- .../SharePublicDashboardUtils.ts | 4 ++ .../dashboard/containers/DashboardPage.tsx | 2 - .../components/picker/DataSourceModal.tsx | 1 + .../PublicDashboardListTable.tsx | 7 ++- 9 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 public/app/features/dashboard/components/DashNav/ShareButton.tsx diff --git a/packages/grafana-ui/src/components/Modal/ModalsContext.tsx b/packages/grafana-ui/src/components/Modal/ModalsContext.tsx index 5bf3fb2b159..274d10b85f2 100644 --- a/packages/grafana-ui/src/components/Modal/ModalsContext.tsx +++ b/packages/grafana-ui/src/components/Modal/ModalsContext.tsx @@ -1,6 +1,6 @@ import React, { Component } from 'react'; -interface ModalsContextState { +export interface ModalsContextState { component: React.ComponentType | null; props: any; showModal: (component: React.ComponentType, props: T) => void; diff --git a/public/app/features/admin/UserListPublicDashboardPage/DashboardsListModalButton.tsx b/public/app/features/admin/UserListPublicDashboardPage/DashboardsListModalButton.tsx index f18c05d711c..ae1c4ada569 100644 --- a/public/app/features/admin/UserListPublicDashboardPage/DashboardsListModalButton.tsx +++ b/public/app/features/admin/UserListPublicDashboardPage/DashboardsListModalButton.tsx @@ -4,7 +4,10 @@ import React from 'react'; import { GrafanaTheme2 } from '@grafana/data/src'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors/src'; import { Button, LoadingPlaceholder, Modal, ModalsController, useStyles2 } from '@grafana/ui/src'; -import { generatePublicDashboardUrl } from 'app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils'; +import { + generatePublicDashboardConfigUrl, + generatePublicDashboardUrl, +} from 'app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils'; import { useGetActiveUserDashboardsQuery } from '../../dashboard/api/publicDashboardApi'; @@ -37,7 +40,7 @@ export const DashboardsListModal = ({ email, onDismiss }: { email: string; onDis Public dashboard settings diff --git a/public/app/features/dashboard/components/DashNav/DashNav.tsx b/public/app/features/dashboard/components/DashNav/DashNav.tsx index 3da75e21590..bc5f5734fc7 100644 --- a/public/app/features/dashboard/components/DashNav/DashNav.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNav.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import React, { FC, ReactNode, useContext, useEffect } from 'react'; +import React, { FC, ReactNode } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { useLocation } from 'react-router-dom'; @@ -13,7 +13,6 @@ import { useForceUpdate, Tag, ToolbarButtonRow, - ModalsContext, ConfirmModal, } from '@grafana/ui'; import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; @@ -26,7 +25,6 @@ import { t, Trans } from 'app/core/internationalization'; import { setStarred } from 'app/core/reducers/navBarTree'; import { AddPanelButton } from 'app/features/dashboard/components/AddPanelButton/AddPanelButton'; import { SaveDashboardDrawer } from 'app/features/dashboard/components/SaveDashboard/SaveDashboardDrawer'; -import { ShareModal } from 'app/features/dashboard/components/ShareModal'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; import { DashboardModel } from 'app/features/dashboard/state'; import { playlistSrv } from 'app/features/playlist/PlaylistSrv'; @@ -36,6 +34,7 @@ import { DashboardMetaChangedEvent, ShowModalReactEvent } from 'app/types/events import { DashNavButton } from './DashNavButton'; import { DashNavTimeControls } from './DashNavTimeControls'; +import { ShareButton } from './ShareButton'; const mapDispatchToProps = { setStarred, @@ -53,7 +52,6 @@ export interface OwnProps { hideTimePicker: boolean; folderTitle?: string; title: string; - shareModalActiveTab?: string; onAddPanel: () => void; } @@ -80,7 +78,6 @@ export const DashNav = React.memo((props) => { // this ensures the component rerenders when the location changes useLocation(); const forceUpdate = useForceUpdate(); - const { showModal, hideModal } = useContext(ModalsContext); // We don't really care about the event payload here only that it triggeres a re-render of this component useBusEvent(props.dashboard.events, DashboardMetaChangedEvent); @@ -164,25 +161,6 @@ export const DashNav = React.memo((props) => { return playlistSrv.isPlaying; }; - // Open/Close - useEffect(() => { - const dashboard = props.dashboard; - const shareModalActiveTab = props.shareModalActiveTab; - const { canShare } = dashboard.meta; - - if (canShare && shareModalActiveTab) { - // automagically open modal - showModal(ShareModal, { - dashboard, - onDismiss: hideModal, - activeTab: shareModalActiveTab, - }); - } - return () => { - hideModal(); - }; - }, [showModal, hideModal, props.dashboard, props.shareModalActiveTab]); - const renderLeftActions = () => { const { dashboard, kioskMode } = props; const { canStar, canShare, isStarred } = dashboard.meta; @@ -209,23 +187,7 @@ export const DashNav = React.memo((props) => { } if (canShare) { - buttons.push( - - {({ showModal, hideModal }) => ( - { - showModal(ShareModal, { - dashboard, - onDismiss: hideModal, - }); - }} - /> - )} - - ); + buttons.push(); } if (dashboard.meta.publicDashboardEnabled) { diff --git a/public/app/features/dashboard/components/DashNav/ShareButton.tsx b/public/app/features/dashboard/components/DashNav/ShareButton.tsx new file mode 100644 index 00000000000..f762b1de090 --- /dev/null +++ b/public/app/features/dashboard/components/DashNav/ShareButton.tsx @@ -0,0 +1,42 @@ +import React, { useContext, useEffect } from 'react'; + +import { ModalsContext } from '@grafana/ui'; +import { useQueryParams } from 'app/core/hooks/useQueryParams'; +import { t } from 'app/core/internationalization'; +import { DashboardModel } from 'app/features/dashboard/state'; + +import { ShareModal } from '../ShareModal'; + +import { DashNavButton } from './DashNavButton'; + +export const ShareButton = ({ dashboard }: { dashboard: DashboardModel }) => { + const [queryParams] = useQueryParams(); + const { showModal, hideModal } = useContext(ModalsContext); + + useEffect(() => { + if (!!queryParams.shareView) { + showModal(ShareModal, { + dashboard, + onDismiss: hideModal, + activeTab: String(queryParams.shareView), + }); + } + return () => { + hideModal(); + }; + }, [showModal, hideModal, dashboard, queryParams.shareView]); + + return ( + { + showModal(ShareModal, { + dashboard, + onDismiss: hideModal, + }); + }} + /> + ); +}; diff --git a/public/app/features/dashboard/components/ShareModal/ShareModal.tsx b/public/app/features/dashboard/components/ShareModal/ShareModal.tsx index d661526076d..2d12915278f 100644 --- a/public/app/features/dashboard/components/ShareModal/ShareModal.tsx +++ b/public/app/features/dashboard/components/ShareModal/ShareModal.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { reportInteraction } from '@grafana/runtime/src'; +import { locationService, reportInteraction } from '@grafana/runtime/src'; import { Modal, ModalTabsHeader, TabContent } from '@grafana/ui'; import { config } from 'app/core/config'; import { contextSrv } from 'app/core/core'; @@ -52,7 +52,7 @@ function getTabs(panel?: PanelModel, activeTab?: string) { } if (Boolean(config.featureToggles['publicDashboards'])) { - tabs.push({ label: 'Public dashboard', value: 'share', component: SharePublicDashboard }); + tabs.push({ label: 'Public dashboard', value: 'public-dashboard', component: SharePublicDashboard }); } const at = tabs.find((t) => t.value === activeTab); @@ -95,6 +95,10 @@ export class ShareModal extends React.Component { reportInteraction('grafana_dashboards_share_modal_viewed'); } + componentWillUnmount() { + locationService.partial({ shareView: null }); + } + onSelectTab: React.ComponentProps['onChangeTab'] = (t) => { this.setState((prevState) => ({ ...prevState, activeTab: t.value })); }; diff --git a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils.ts b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils.ts index 3d65f5af25a..213ef06d2b6 100644 --- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils.ts +++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils.ts @@ -76,4 +76,8 @@ export const generatePublicDashboardUrl = (accessToken: string): string => { return `${getConfig().appUrl}public-dashboards/${accessToken}`; }; +export const generatePublicDashboardConfigUrl = (dashboardUid: string): string => { + return `/d/${dashboardUid}?shareView=public-dashboard`; +}; + export const validEmailRegex = /^[A-Z\d._%+-]+@[A-Z\d.-]+\.[A-Z]{2,}$/i; diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index cab687ff7e5..5e5c7d09306 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -64,7 +64,6 @@ export type DashboardPageRouteSearchParams = { editPanel?: string; viewPanel?: string; editview?: string; - shareView?: string; panelType?: string; inspect?: string; from?: string; @@ -454,7 +453,6 @@ export class UnthemedDashboardPage extends PureComponent { onAddPanel={this.onAddPanel} kioskMode={kioskMode} hideTimePicker={dashboard.timepicker.hidden} - shareModalActiveTab={this.props.queryParams.shareView} /> )} diff --git a/public/app/features/datasources/components/picker/DataSourceModal.tsx b/public/app/features/datasources/components/picker/DataSourceModal.tsx index 663e1f2a7db..e1494321b58 100644 --- a/public/app/features/datasources/components/picker/DataSourceModal.tsx +++ b/public/app/features/datasources/components/picker/DataSourceModal.tsx @@ -155,6 +155,7 @@ export function DataSourceModal({ item: INTERACTION_ITEM.CONFIG_NEW_DS, src: analyticsInteractionSrc, }); + onDismiss(); }} /> diff --git a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx index 71f7aded088..598727fab5b 100644 --- a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx +++ b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx @@ -8,7 +8,10 @@ import { Link, ButtonGroup, LinkButton, Icon, Tag, useStyles2, Tooltip, useTheme import { Page } from 'app/core/components/Page/Page'; import { contextSrv } from 'app/core/services/context_srv'; import { useListPublicDashboardsQuery } from 'app/features/dashboard/api/publicDashboardApi'; -import { generatePublicDashboardUrl } from 'app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils'; +import { + generatePublicDashboardConfigUrl, + generatePublicDashboardUrl, +} from 'app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboardUtils'; import { isOrgAdmin } from 'app/features/plugins/admin/permissions'; import { AccessControlAction } from 'app/types'; @@ -82,7 +85,7 @@ export const PublicDashboardListTable = () => {