From 671a537dbce4537c94861341279f92bdc8272de6 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Mon, 30 Sep 2024 17:22:04 +0100 Subject: [PATCH] Chore: Remove `topnav` references (#5092) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does - removes references to `topnav` - `topnav` was default enabled in grafana v9.5 - we intend to remove the toggle soon™️ ## Which issue(s) this PR closes Related to [issue link here] ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- dev/helm-local.yml | 2 +- grafana-plugin/jest.setup.ts | 4 +- grafana-plugin/src/PluginPage.tsx | 7 +- .../src/containers/Alerts/Alerts.tsx | 11 +-- .../DefaultPageLayout/DefaultPageLayout.tsx | 44 ++--------- .../MobileAppConnection.test.tsx | 4 - .../RotationForm/RotationForm.helpers.ts | 27 +++---- grafana-plugin/src/navbar/Header/Header.tsx | 3 +- .../src/navbar/LegacyNavHeading.tsx | 4 +- grafana-plugin/src/pages/pages.tsx | 9 +-- .../src/pages/settings/SettingsPage.tsx | 79 +++++++++---------- .../tabs/MainSettings/MainSettings.tsx | 10 --- .../plugin/GrafanaPluginRootPage.helpers.tsx | 5 -- .../src/plugin/GrafanaPluginRootPage.tsx | 14 +--- 14 files changed, 64 insertions(+), 159 deletions(-) diff --git a/dev/helm-local.yml b/dev/helm-local.yml index b5fd8049..33a28790 100644 --- a/dev/helm-local.yml +++ b/dev/helm-local.yml @@ -70,7 +70,7 @@ grafana: - name: DATABASE_PASSWORD value: oncallpassword env: - GF_FEATURE_TOGGLES_ENABLE: topnav,externalServiceAccounts + GF_FEATURE_TOGGLES_ENABLE: externalServiceAccounts GF_SECURITY_ADMIN_PASSWORD: oncall GF_SECURITY_ADMIN_USER: oncall GF_PLUGINS_ALLOW_LOADING_UNSIGNED_PLUGINS: grafana-oncall-app diff --git a/grafana-plugin/jest.setup.ts b/grafana-plugin/jest.setup.ts index 0ef64969..da0cee32 100644 --- a/grafana-plugin/jest.setup.ts +++ b/grafana-plugin/jest.setup.ts @@ -11,9 +11,7 @@ import { TextEncoder, TextDecoder } from 'util'; jest.mock('@grafana/runtime', () => ({ __esModule: true, config: { - featureToggles: { - topNav: false, - }, + featureToggles: {}, bootData: { user: { timezone: 'UTC', diff --git a/grafana-plugin/src/PluginPage.tsx b/grafana-plugin/src/PluginPage.tsx index 70020c93..eff870c7 100644 --- a/grafana-plugin/src/PluginPage.tsx +++ b/grafana-plugin/src/PluginPage.tsx @@ -6,13 +6,12 @@ import { Header } from 'navbar/Header/Header'; import { RenderConditionally } from 'components/RenderConditionally/RenderConditionally'; import { pages } from 'pages/pages'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; interface AppPluginPageProps extends PluginPageProps { page?: string; } -export const PluginPage = (isTopNavbar() ? RealPlugin : PluginPageFallback) as React.ComponentType; +export const PluginPage = RealPlugin as React.ComponentType; function RealPlugin(props: AppPluginPageProps): React.ReactNode { const { page } = props; @@ -33,7 +32,3 @@ function RealPlugin(props: AppPluginPageProps): React.ReactNode { ); } - -export function PluginPageFallback(props: PluginPageProps): React.ReactNode { - return props.children; -} diff --git a/grafana-plugin/src/containers/Alerts/Alerts.tsx b/grafana-plugin/src/containers/Alerts/Alerts.tsx index d08b1df3..e22bf04a 100644 --- a/grafana-plugin/src/containers/Alerts/Alerts.tsx +++ b/grafana-plugin/src/containers/Alerts/Alerts.tsx @@ -15,7 +15,6 @@ import { getSlackMessage } from 'containers/DefaultPageLayout/DefaultPageLayout. import { SlackError } from 'containers/DefaultPageLayout/DefaultPageLayout.types'; import { getIfChatOpsConnected } from 'containers/DefaultPageLayout/helper'; import { UserHelper } from 'models/user/user.helpers'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { AppFeature } from 'state/features'; import { useStore } from 'state/useStore'; @@ -72,7 +71,7 @@ export const Alerts = observer(() => { return null; } return ( -
+
{showSlackInstallAlert && ( { instructionsLink: css` color: ${theme.colors.primary.text}; `, - - alertsContainerLegacy: css` - paddingtop: '10px'; - - @media (max-width: 768px) { - padding-top: 50px; - } - `, }; }; diff --git a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx index aa1b1f7a..98c91469 100644 --- a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx +++ b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx @@ -1,13 +1,11 @@ -import React, { FC, ReactElement } from 'react'; +import React, { FC } from 'react'; -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import { AppRootProps, NavModelItem } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import { PluginPage } from 'PluginPage'; import { observer } from 'mobx-react'; -import { Alerts } from 'containers/Alerts/Alerts'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; interface DefaultPageLayoutProps extends AppRootProps { children?: any; @@ -19,39 +17,11 @@ export const DefaultPageLayout: FC = observer((props) => const { children, page, pageNav } = props; const styles = useStyles2(getStyles); - if (isTopNavbar()) { - return renderTopNavbar(); - } - - return renderLegacyNavbar(); - - function renderTopNavbar(): ReactElement { - return ( - -
{children}
-
- ); - } - - function renderLegacyNavbar(): ReactElement { - return ( - -
-
- - {children} -
-
-
- ); - } + return ( + +
{children}
+
+ ); }); const getStyles = () => { diff --git a/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.test.tsx b/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.test.tsx index 124fde2a..d6136ce5 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.test.tsx +++ b/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.test.tsx @@ -9,10 +9,6 @@ import { rootStore } from 'state/rootStore'; import { MobileAppConnection } from './MobileAppConnection'; -jest.mock('plugin/GrafanaPluginRootPage.helpers', () => ({ - isTopNavbar: () => false, -})); - jest.mock('helpers/authorization/authorization', () => ({ ...jest.requireActual('helpers/authorization/authorization'), isUserActionAllowed: jest.fn().mockReturnValue(true), diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts index 7043f614..68066574 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts @@ -1,9 +1,7 @@ import { Dayjs, ManipulateType } from 'dayjs'; -import { GRAFANA_HEADER_HEIGHT, GRAFANA_LEGACY_SIDEBAR_WIDTH } from 'helpers/consts'; +import { GRAFANA_HEADER_HEIGHT } from 'helpers/consts'; import { DraggableData } from 'react-draggable'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; - import { RepeatEveryPeriod } from './RotationForm.types'; export const getRepeatShiftsEveryOptions = (repeatEveryPeriod: number) => { @@ -195,20 +193,13 @@ export function getDraggableModalCoordinatesOnInit( const baseReferenceElRect = body.getBoundingClientRect(); const { innerHeight } = window; - const { right, bottom } = baseReferenceElRect; + const { right } = baseReferenceElRect; - return isTopNavbar() - ? { - // values are adjusted by any padding/margin differences - left: -data.node.offsetLeft + 12, - right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, - top: -offsetTop + GRAFANA_HEADER_HEIGHT + 12, - bottom: innerHeight - data.node.offsetHeight - offsetTop - 12, - } - : { - left: -data.node.offsetLeft + 12 + GRAFANA_LEGACY_SIDEBAR_WIDTH, - right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, - top: -offsetTop + 12, - bottom: bottom - data.node.offsetHeight - offsetTop - 12, - }; + return { + // values are adjusted by any padding/margin differences + left: -data.node.offsetLeft + 12, + right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, + top: -offsetTop + GRAFANA_HEADER_HEIGHT + 12, + bottom: innerHeight - data.node.offsetHeight - offsetTop - 12, + }; } diff --git a/grafana-plugin/src/navbar/Header/Header.tsx b/grafana-plugin/src/navbar/Header/Header.tsx index c5485d9d..e1c99af0 100644 --- a/grafana-plugin/src/navbar/Header/Header.tsx +++ b/grafana-plugin/src/navbar/Header/Header.tsx @@ -8,7 +8,6 @@ import { observer } from 'mobx-react'; import gitHubStarSVG from 'assets/img/github_star.svg'; import logo from 'assets/img/logo.svg'; import { Alerts } from 'containers/Alerts/Alerts'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { getHeaderStyles } from './Header.styles'; @@ -18,7 +17,7 @@ export const Header = observer(() => { return ( <>
-
+
Grafana OnCall diff --git a/grafana-plugin/src/navbar/LegacyNavHeading.tsx b/grafana-plugin/src/navbar/LegacyNavHeading.tsx index 184e453a..b125bc99 100644 --- a/grafana-plugin/src/navbar/LegacyNavHeading.tsx +++ b/grafana-plugin/src/navbar/LegacyNavHeading.tsx @@ -1,11 +1,9 @@ -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; - interface LegacyNavHeadingProps { children: JSX.Element; show?: boolean; } export const LegacyNavHeading = function (props: LegacyNavHeadingProps): JSX.Element { - const { show = !isTopNavbar(), children } = props; + const { show = false, children } = props; return show ? children : null; }; diff --git a/grafana-plugin/src/pages/pages.tsx b/grafana-plugin/src/pages/pages.tsx index 8379ba9b..eea14bdf 100644 --- a/grafana-plugin/src/pages/pages.tsx +++ b/grafana-plugin/src/pages/pages.tsx @@ -3,7 +3,6 @@ import { UserActions, UserAction, isUserActionAllowed } from 'helpers/authorizat import { PLUGIN_ROOT } from 'helpers/consts'; import { matchPath } from 'react-router-dom-v5-compat'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { AppFeature } from 'state/features'; import { RootBaseStore } from 'state/rootBaseStore/RootBaseStore'; @@ -109,7 +108,7 @@ export const pages: { [id: string]: PageDefinition } = [ text: 'ChatOps', path: getPath('chat-ops'), hideFromBreadcrumbs: true, - hideFromTabs: isTopNavbar(), + hideFromTabs: true, action: UserActions.ChatOpsRead, }, { @@ -126,7 +125,7 @@ export const pages: { [id: string]: PageDefinition } = [ text: 'Env Variables', hideFromTabsFn: (store: RootBaseStore) => { const hasLiveSettings = store.hasFeature(AppFeature.LiveSettings); - return isTopNavbar() || !hasLiveSettings; + return !hasLiveSettings; }, path: getPath('live-settings'), action: UserActions.OtherSettingsRead, @@ -137,7 +136,7 @@ export const pages: { [id: string]: PageDefinition } = [ text: 'Cloud', hideFromTabsFn: (store: RootBaseStore) => { const hasCloudFeature = store.hasFeature(AppFeature.CloudConnection); - return isTopNavbar() || !hasCloudFeature; + return !hasCloudFeature; }, path: getPath('cloud'), action: UserActions.OtherSettingsWrite, @@ -161,7 +160,7 @@ export const pages: { [id: string]: PageDefinition } = [ ...current, getPageNav: (pageTitle: string) => ({ - text: isTopNavbar() ? '' : current.text, + text: '', parentItem: current.getParentItem ? current.getParentItem(pageTitle) : undefined, hideFromBreadcrumbs: current.hideFromBreadcrumbs, hideFromTabs: current.hideFromTabs, diff --git a/grafana-plugin/src/pages/settings/SettingsPage.tsx b/grafana-plugin/src/pages/settings/SettingsPage.tsx index f9fed768..3f52bb78 100644 --- a/grafana-plugin/src/pages/settings/SettingsPage.tsx +++ b/grafana-plugin/src/pages/settings/SettingsPage.tsx @@ -9,7 +9,6 @@ import { observer } from 'mobx-react'; import { ChatOpsPage } from 'pages/settings/tabs/ChatOps/ChatOps'; import { MainSettings } from 'pages/settings/tabs/MainSettings/MainSettings'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { AppFeature } from 'state/features'; import { WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; @@ -54,52 +53,48 @@ class Settings extends React.Component { const showCloudPage = hasCloudPage && isUserActionAllowed(UserActions.OtherSettingsWrite); const showLiveSettings = hasLiveSettings && isUserActionAllowed(UserActions.OtherSettingsRead); - if (isTopNavbar()) { - return ( - <> - + return ( + <> + + onTabChange(SettingsPageTab.MainSettings.key)} + active={activeTab === SettingsPageTab.MainSettings.key} + label={SettingsPageTab.MainSettings.value} + /> + onTabChange(SettingsPageTab.ChatOps.key)} + active={activeTab === SettingsPageTab.ChatOps.key} + label={SettingsPageTab.ChatOps.value} + /> + onTabChange(SettingsPageTab.TeamsSettings.key)} + active={activeTab === SettingsPageTab.TeamsSettings.key} + label={SettingsPageTab.TeamsSettings.value} + /> + {showLiveSettings && ( onTabChange(SettingsPageTab.MainSettings.key)} - active={activeTab === SettingsPageTab.MainSettings.key} - label={SettingsPageTab.MainSettings.value} + key={SettingsPageTab.EnvVariables.key} + onChangeTab={() => onTabChange(SettingsPageTab.EnvVariables.key)} + active={activeTab === SettingsPageTab.EnvVariables.key} + label={SettingsPageTab.EnvVariables.value} /> + )} + {showCloudPage && ( onTabChange(SettingsPageTab.ChatOps.key)} - active={activeTab === SettingsPageTab.ChatOps.key} - label={SettingsPageTab.ChatOps.value} + key={SettingsPageTab.Cloud.key} + onChangeTab={() => onTabChange(SettingsPageTab.Cloud.key)} + active={activeTab === SettingsPageTab.Cloud.key} + label={SettingsPageTab.Cloud.value} /> - onTabChange(SettingsPageTab.TeamsSettings.key)} - active={activeTab === SettingsPageTab.TeamsSettings.key} - label={SettingsPageTab.TeamsSettings.value} - /> - {showLiveSettings && ( - onTabChange(SettingsPageTab.EnvVariables.key)} - active={activeTab === SettingsPageTab.EnvVariables.key} - label={SettingsPageTab.EnvVariables.value} - /> - )} - {showCloudPage && ( - onTabChange(SettingsPageTab.Cloud.key)} - active={activeTab === SettingsPageTab.Cloud.key} - label={SettingsPageTab.Cloud.value} - /> - )} - + )} + - - - ); - } - - return ; + + + ); } getMatchingPageNav() { diff --git a/grafana-plugin/src/pages/settings/tabs/MainSettings/MainSettings.tsx b/grafana-plugin/src/pages/settings/tabs/MainSettings/MainSettings.tsx index 278d3600..66793e63 100644 --- a/grafana-plugin/src/pages/settings/tabs/MainSettings/MainSettings.tsx +++ b/grafana-plugin/src/pages/settings/tabs/MainSettings/MainSettings.tsx @@ -9,8 +9,6 @@ import { LegacyNavHeading } from 'navbar/LegacyNavHeading'; import { Text } from 'components/Text/Text'; import { ApiTokenSettings } from 'containers/ApiTokenSettings/ApiTokenSettings'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; -import { TeamsSettings } from 'pages/settings/tabs/TeamsSettings/TeamsSettings'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { useStore } from 'state/useStore'; export const MainSettings = observer(() => { @@ -50,14 +48,6 @@ export const MainSettings = observer(() => {
- {!isTopNavbar() && ( -
- - Teams and Access Settings - - -
- )} API URL diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.helpers.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.helpers.tsx index 63b461be..478523c3 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.helpers.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.helpers.tsx @@ -1,8 +1,3 @@ -import { config } from '@grafana/runtime'; - -export function isTopNavbar(): boolean { - return !!config.featureToggles.topnav; -} export function getQueryParams(): any { const searchParams = new URLSearchParams(window.location.search); diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index a0040f83..921b8095 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -8,8 +8,6 @@ import { DEFAULT_PAGE, getOnCallApiUrl } from 'helpers/consts'; import { FaroHelper } from 'helpers/faro'; import { useOnMount } from 'helpers/hooks'; import { observer, Provider } from 'mobx-react'; -import { Header } from 'navbar/Header/Header'; -import { LegacyNavTabsBar } from 'navbar/LegacyNavTabsBar'; import { Navigate, Route, Routes, useLocation } from 'react-router-dom-v5-compat'; import { RenderConditionally } from 'components/RenderConditionally/RenderConditionally'; @@ -36,7 +34,7 @@ import { rootStore } from 'state/rootStore'; import { useStore } from 'state/useStore'; import 'assets/style/global.css'; -import { getQueryParams, isTopNavbar } from './GrafanaPluginRootPage.helpers'; +import { getQueryParams } from './GrafanaPluginRootPage.helpers'; import grafanaGlobalStyle from '!raw-loader!assets/style/grafanaGlobalStyles.css'; @@ -112,22 +110,12 @@ export const Root = observer((props: AppRootProps) => { return ( - {!isTopNavbar() && ( - <> -
- - - )}