From 4d08f446675c1cce135663494b390eb5cce74e57 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Fri, 11 Oct 2024 15:07:01 +0100 Subject: [PATCH] SingleTopNav: Revert to using `AppChromeUpdate` so banners are correct (#94540) * revert to using AppChromeUpdate * fix dashboard settings in old arch * remove empty interface * fix AlertRuleForm --- .../core/components/AppChrome/AppChrome.tsx | 22 +- .../components/AppChrome/AppChromeUpdate.tsx | 2 +- .../AppChrome/MegaMenu/MegaMenu.tsx | 75 ++++--- .../AppChrome/TopBar/SingleTopBarActions.tsx | 33 +++ public/app/core/components/Page/Page.tsx | 120 ++++------- .../components/Page/PageToolbarActions.tsx | 47 ---- public/app/core/components/Page/types.ts | 2 - public/app/core/context/GrafanaContext.ts | 25 ++- .../alerting/unified/CloneRuleEditor.test.tsx | 5 +- .../components/receivers/TemplateForm.tsx | 44 ++-- .../alert-rule-form/AlertRuleForm.tsx | 202 ++++++++---------- .../alert-rule-form/ModifyExportRuleForm.tsx | 46 ++-- .../scene/DashboardSceneRenderer.tsx | 17 +- .../settings/AnnotationsEditView.tsx | 24 +-- .../settings/DashboardLinksEditView.tsx | 23 +- .../settings/GeneralSettingsEditView.tsx | 12 +- .../settings/JsonModelEditView.tsx | 13 +- .../settings/PermissionsEditView.tsx | 13 +- .../settings/VariablesEditView.tsx | 23 +- .../settings/VersionsEditView.tsx | 13 +- .../dashboard/components/DashNav/DashNav.tsx | 16 +- .../AccessControlDashboardPermissions.tsx | 4 +- .../DashboardSettings/AnnotationsSettings.tsx | 4 +- .../DashboardSettings/DashboardSettings.tsx | 18 +- .../DashboardSettings/GeneralSettings.tsx | 3 +- .../DashboardSettings/JsonEditorSettings.tsx | 4 +- .../DashboardSettings/LinksSettings.tsx | 4 +- .../DashboardSettings/VersionsSettings.tsx | 4 +- .../components/DashboardSettings/types.ts | 3 +- .../components/PanelEditor/PanelEditor.tsx | 16 +- .../dashboard/containers/DashboardPage.tsx | 24 +-- .../editor/VariableEditorContainer.tsx | 4 +- 32 files changed, 338 insertions(+), 527 deletions(-) create mode 100644 public/app/core/components/AppChrome/TopBar/SingleTopBarActions.tsx delete mode 100644 public/app/core/components/Page/PageToolbarActions.tsx diff --git a/public/app/core/components/AppChrome/AppChrome.tsx b/public/app/core/components/AppChrome/AppChrome.tsx index 9c323a7ba42..e85295e4660 100644 --- a/public/app/core/components/AppChrome/AppChrome.tsx +++ b/public/app/core/components/AppChrome/AppChrome.tsx @@ -18,6 +18,7 @@ import { MegaMenu, MENU_WIDTH } from './MegaMenu/MegaMenu'; import { NavToolbar } from './NavToolbar/NavToolbar'; import { ReturnToPrevious } from './ReturnToPrevious/ReturnToPrevious'; import { SingleTopBar } from './TopBar/SingleTopBar'; +import { SingleTopBarActions } from './TopBar/SingleTopBarActions'; import { TopSearchBar } from './TopBar/TopSearchBar'; import { TOP_BAR_LEVEL_HEIGHT } from './types'; @@ -28,7 +29,7 @@ export function AppChrome({ children }: Props) { const state = chrome.useState(); const searchBarHidden = state.searchBarHidden || state.kioskMode === KioskMode.TV; const theme = useTheme2(); - const styles = useStyles2(getStyles, searchBarHidden); + const styles = useStyles2(getStyles, searchBarHidden, Boolean(state.actions)); const dockedMenuBreakpoint = theme.breakpoints.values.xl; const dockedMenuLocalStorageState = store.getBool(DOCKED_LOCAL_STORAGE_KEY, true); @@ -99,12 +100,15 @@ export function AppChrome({ children }: Props) { )}
{isSingleTopNav ? ( - + <> + + {state.actions && {state.actions}} + ) : ( <> {!searchBarHidden && } @@ -156,13 +160,13 @@ export function AppChrome({ children }: Props) { ); } -const getStyles = (theme: GrafanaTheme2, searchBarHidden: boolean) => { +const getStyles = (theme: GrafanaTheme2, searchBarHidden: boolean, hasActions: boolean) => { const isSingleTopNav = config.featureToggles.singleTopNav; return { content: css({ display: 'flex', flexDirection: 'column', - paddingTop: isSingleTopNav ? TOP_BAR_LEVEL_HEIGHT : TOP_BAR_LEVEL_HEIGHT * 2, + paddingTop: !isSingleTopNav || hasActions ? TOP_BAR_LEVEL_HEIGHT * 2 : TOP_BAR_LEVEL_HEIGHT, flexGrow: 1, height: 'auto', }), diff --git a/public/app/core/components/AppChrome/AppChromeUpdate.tsx b/public/app/core/components/AppChrome/AppChromeUpdate.tsx index ef92b81903a..8daf42dba22 100644 --- a/public/app/core/components/AppChrome/AppChromeUpdate.tsx +++ b/public/app/core/components/AppChrome/AppChromeUpdate.tsx @@ -7,7 +7,7 @@ export interface AppChromeUpdateProps { actions?: React.ReactNode; } /** - * @deprecated This component is deprecated and will be removed in a future release. + * This is the way core pages add actions to the second chrome toolbar */ export const AppChromeUpdate = React.memo(({ actions }: AppChromeUpdateProps) => { const { chrome } = useGrafana(); diff --git a/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx b/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx index b2be2f189b1..23aa2b0b65a 100644 --- a/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx +++ b/public/app/core/components/AppChrome/MegaMenu/MegaMenu.tsx @@ -13,6 +13,8 @@ import { setBookmark } from 'app/core/reducers/navBarTree'; import { usePatchUserPreferencesMutation } from 'app/features/preferences/api/index'; import { useDispatch, useSelector } from 'app/types'; +import { TOP_BAR_LEVEL_HEIGHT } from '../types'; + import { MegaMenuHeader } from './MegaMenuHeader'; import { MegaMenuItem } from './MegaMenuItem'; import { usePinnedItems } from './hooks'; @@ -166,41 +168,44 @@ export const MegaMenu = memo( MegaMenu.displayName = 'MegaMenu'; -const getStyles = (theme: GrafanaTheme2) => ({ - content: css({ - display: 'flex', - flexDirection: 'column', - height: '100%', - minHeight: 0, - position: 'relative', - }), - mobileHeader: css({ - display: 'flex', - justifyContent: 'space-between', - padding: theme.spacing(1, 1, 1, 2), - borderBottom: `1px solid ${theme.colors.border.weak}`, +const getStyles = (theme: GrafanaTheme2) => { + const isSingleTopNav = config.featureToggles.singleTopNav; + return { + content: css({ + display: 'flex', + flexDirection: 'column', + height: isSingleTopNav ? `calc(100% - ${TOP_BAR_LEVEL_HEIGHT}px)` : '100%', + minHeight: 0, + position: 'relative', + }), + mobileHeader: css({ + display: 'flex', + justifyContent: 'space-between', + padding: theme.spacing(1, 1, 1, 2), + borderBottom: `1px solid ${theme.colors.border.weak}`, - [theme.breakpoints.up('md')]: { + [theme.breakpoints.up('md')]: { + display: 'none', + }, + }), + itemList: css({ + boxSizing: 'border-box', + display: 'flex', + flexDirection: 'column', + listStyleType: 'none', + padding: theme.spacing(1, 1, 2, 1), + [theme.breakpoints.up('md')]: { + width: MENU_WIDTH, + }, + }), + dockMenuButton: css({ display: 'none', - }, - }), - itemList: css({ - boxSizing: 'border-box', - display: 'flex', - flexDirection: 'column', - listStyleType: 'none', - padding: theme.spacing(1, 1, 2, 1), - [theme.breakpoints.up('md')]: { - width: MENU_WIDTH, - }, - }), - dockMenuButton: css({ - display: 'none', - position: 'relative', - top: theme.spacing(1), + position: 'relative', + top: theme.spacing(1), - [theme.breakpoints.up('xl')]: { - display: 'inline-flex', - }, - }), -}); + [theme.breakpoints.up('xl')]: { + display: 'inline-flex', + }, + }), + }; +}; diff --git a/public/app/core/components/AppChrome/TopBar/SingleTopBarActions.tsx b/public/app/core/components/AppChrome/TopBar/SingleTopBarActions.tsx new file mode 100644 index 00000000000..51e5bfad6d3 --- /dev/null +++ b/public/app/core/components/AppChrome/TopBar/SingleTopBarActions.tsx @@ -0,0 +1,33 @@ +import { css } from '@emotion/css'; +import { PropsWithChildren } from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Components } from '@grafana/e2e-selectors'; +import { Stack, useStyles2 } from '@grafana/ui'; + +import { TOP_BAR_LEVEL_HEIGHT } from '../types'; + +export function SingleTopBarActions({ children }: PropsWithChildren) { + const styles = useStyles2(getStyles); + + return ( +
+ + {children} + +
+ ); +} + +const getStyles = (theme: GrafanaTheme2) => { + return { + actionsBar: css({ + alignItems: 'center', + backgroundColor: theme.colors.background.primary, + borderBottom: `1px solid ${theme.colors.border.weak}`, + display: 'flex', + height: TOP_BAR_LEVEL_HEIGHT, + padding: theme.spacing(0, 1, 0, 2), + }), + }; +}; diff --git a/public/app/core/components/Page/Page.tsx b/public/app/core/components/Page/Page.tsx index 0c5aab757b5..cd29bbfb556 100644 --- a/public/app/core/components/Page/Page.tsx +++ b/public/app/core/components/Page/Page.tsx @@ -1,58 +1,19 @@ import { css, cx } from '@emotion/css'; -import { - createContext, - Dispatch, - ReactNode, - SetStateAction, - useContext, - useEffect, - useLayoutEffect, - useState, -} from 'react'; +import { useLayoutEffect } from 'react'; import { GrafanaTheme2, PageLayoutType } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { useStyles2 } from '@grafana/ui'; import { useGrafana } from 'app/core/context/GrafanaContext'; -import { TOP_BAR_LEVEL_HEIGHT } from '../AppChrome/types'; import NativeScrollbar from '../NativeScrollbar'; import { PageContents } from './PageContents'; import { PageHeader } from './PageHeader'; import { PageTabs } from './PageTabs'; -import { PageToolbarActions } from './PageToolbarActions'; import { PageType } from './types'; import { usePageNav } from './usePageNav'; import { usePageTitle } from './usePageTitle'; -export interface PageContextType { - setToolbar: Dispatch>; -} - -export const PageContext = createContext(undefined); - -function usePageContext(): PageContextType { - const context = useContext(PageContext); - if (!context) { - throw new Error('No PageContext found'); - } - return context; -} - -/** - * Hook to dynamically set the toolbar of a Page from a child component. - * Prefer setting the toolbar directly as a prop to Page. - * @param toolbar a ReactNode that will be rendered in a second toolbar - */ -export function usePageToolbar(toolbar?: ReactNode) { - const { setToolbar } = usePageContext(); - useEffect(() => { - setToolbar(toolbar); - return () => setToolbar(undefined); - }, [setToolbar, toolbar]); -} - export const Page: PageType = ({ navId, navModel: oldNavProp, @@ -63,15 +24,12 @@ export const Page: PageType = ({ subTitle, children, className, - toolbar: toolbarProp, info, layout = PageLayoutType.Standard, onSetScrollRef, ...otherProps }) => { - const isSingleTopNav = config.featureToggles.singleTopNav; - const [toolbar, setToolbar] = useState(toolbarProp); - const styles = useStyles2(getStyles, Boolean(isSingleTopNav && toolbar)); + const styles = useStyles2(getStyles); const navModel = usePageNav(navId, oldNavProp); const { chrome } = useGrafana(); @@ -92,58 +50,54 @@ export const Page: PageType = ({ }, [navModel, pageNav, chrome, layout]); return ( - -
- {isSingleTopNav && toolbar && {toolbar}} - {layout === PageLayoutType.Standard && ( - -
- {pageHeaderNav && ( - - )} - {pageNav && pageNav.children && } -
{children}
-
- - )} +
+ {layout === PageLayoutType.Standard && ( + +
+ {pageHeaderNav && ( + + )} + {pageNav && pageNav.children && } +
{children}
+
+ + )} - {layout === PageLayoutType.Canvas && ( - -
{children}
- - )} + {layout === PageLayoutType.Canvas && ( + +
{children}
+ + )} - {layout === PageLayoutType.Custom && children} -
- + {layout === PageLayoutType.Custom && children} +
); }; Page.Contents = PageContents; -const getStyles = (theme: GrafanaTheme2, hasToolbar: boolean) => { +const getStyles = (theme: GrafanaTheme2) => { return { wrapper: css({ label: 'page-wrapper', display: 'flex', flex: '1 1 0', flexDirection: 'column', - marginTop: hasToolbar ? TOP_BAR_LEVEL_HEIGHT : 0, position: 'relative', }), pageContent: css({ diff --git a/public/app/core/components/Page/PageToolbarActions.tsx b/public/app/core/components/Page/PageToolbarActions.tsx deleted file mode 100644 index 20c5589333f..00000000000 --- a/public/app/core/components/Page/PageToolbarActions.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import { css } from '@emotion/css'; -import { PropsWithChildren } from 'react'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { Components } from '@grafana/e2e-selectors'; -import { useChromeHeaderHeight } from '@grafana/runtime'; -import { Stack, useStyles2 } from '@grafana/ui'; -import { useGrafana } from 'app/core/context/GrafanaContext'; - -import { MENU_WIDTH } from '../AppChrome/MegaMenu/MegaMenu'; -import { TOP_BAR_LEVEL_HEIGHT } from '../AppChrome/types'; - -export interface Props {} - -export function PageToolbarActions({ children }: PropsWithChildren) { - const chromeHeaderHeight = useChromeHeaderHeight(); - const { chrome } = useGrafana(); - const state = chrome.useState(); - const menuDockedAndOpen = !state.chromeless && state.megaMenuDocked && state.megaMenuOpen; - const styles = useStyles2(getStyles, chromeHeaderHeight ?? 0, menuDockedAndOpen); - - return ( -
- - {children} - -
- ); -} - -const getStyles = (theme: GrafanaTheme2, chromeHeaderHeight: number, menuDockedAndOpen: boolean) => { - return { - pageToolbar: css({ - alignItems: 'center', - backgroundColor: theme.colors.background.primary, - borderBottom: `1px solid ${theme.colors.border.weak}`, - display: 'flex', - height: TOP_BAR_LEVEL_HEIGHT, - left: menuDockedAndOpen ? MENU_WIDTH : 0, - padding: theme.spacing(0, 1, 0, 2), - position: 'fixed', - top: chromeHeaderHeight, - right: 0, - zIndex: theme.zIndex.navbarFixed, - }), - }; -}; diff --git a/public/app/core/components/Page/types.ts b/public/app/core/components/Page/types.ts index eb6bd7dd4d3..f83f00e77f3 100644 --- a/public/app/core/components/Page/types.ts +++ b/public/app/core/components/Page/types.ts @@ -25,8 +25,6 @@ export interface PageProps extends HTMLAttributes { layout?: PageLayoutType; /** Can be used to get the scroll container element to access scroll position */ onSetScrollRef?: (ref: ScrollRefElement) => void; - /** Set a page-level toolbar */ - toolbar?: React.ReactNode; } export interface PageInfoItem { diff --git a/public/app/core/context/GrafanaContext.ts b/public/app/core/context/GrafanaContext.ts index 424bd71ee68..cf3ed6f1f1c 100644 --- a/public/app/core/context/GrafanaContext.ts +++ b/public/app/core/context/GrafanaContext.ts @@ -4,6 +4,7 @@ import { GrafanaConfig } from '@grafana/data'; import { LocationService, locationService, BackendSrv, config } from '@grafana/runtime'; import { AppChromeService } from '../components/AppChrome/AppChromeService'; +import { TOP_BAR_LEVEL_HEIGHT } from '../components/AppChrome/types'; import { NewFrontendAssetsChecker } from '../services/NewFrontendAssetsChecker'; import { KeybindingSrv } from '../services/keybindingSrv'; @@ -42,17 +43,25 @@ export function useReturnToPreviousInternal() { ); } -const SINGLE_HEADER_BAR_HEIGHT = 40; - export function useChromeHeaderHeight() { const { chrome } = useGrafana(); - const { kioskMode, searchBarHidden, chromeless } = chrome.useState(); + const { actions, kioskMode, searchBarHidden, chromeless } = chrome.useState(); - if (kioskMode || chromeless) { - return 0; - } else if (searchBarHidden || config.featureToggles.singleTopNav) { - return SINGLE_HEADER_BAR_HEIGHT; + if (config.featureToggles.singleTopNav) { + if (kioskMode || chromeless) { + return 0; + } else if (actions) { + return TOP_BAR_LEVEL_HEIGHT * 2; + } else { + return TOP_BAR_LEVEL_HEIGHT; + } } else { - return SINGLE_HEADER_BAR_HEIGHT * 2; + if (kioskMode || chromeless) { + return 0; + } else if (searchBarHidden) { + return TOP_BAR_LEVEL_HEIGHT; + } else { + return TOP_BAR_LEVEL_HEIGHT * 2; + } } } diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx index fec861fa12c..38155aa917a 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx @@ -5,7 +5,6 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { selectors } from '@grafana/e2e-selectors/src'; import { setDataSourceSrv } from '@grafana/runtime'; -import { PageContext } from 'app/core/components/Page/Page'; import { DashboardSearchItem, DashboardSearchItemType } from 'app/features/search/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; @@ -73,9 +72,7 @@ function Wrapper({ children }: React.PropsWithChildren<{}>) { const formApi = useForm({ defaultValues: getDefaultFormValues() }); return ( - - {children} - + {children} ); } diff --git a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx index c306701172b..eeb6890e744 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx @@ -1,13 +1,13 @@ import { css, cx } from '@emotion/css'; import { addMinutes, subDays, subHours } from 'date-fns'; import { Location } from 'history'; -import { useMemo, useRef, useState } from 'react'; +import { useRef, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useToggle } from 'react-use'; import AutoSizer from 'react-virtualized-auto-sizer'; import { GrafanaTheme2 } from '@grafana/data'; -import { config as runtimeConfig, isFetchError, locationService } from '@grafana/runtime'; +import { isFetchError, locationService } from '@grafana/runtime'; import { Alert, Button, @@ -21,7 +21,6 @@ import { InlineField, Box, } from '@grafana/ui'; -import { usePageToolbar } from 'app/core/components/Page/Page'; import { useAppNotification } from 'app/core/copy/appNotification'; import { useCleanup } from 'app/core/hooks/useCleanup'; import { ActiveTab as ContactPointsActiveTabs } from 'app/features/alerting/unified/components/contact-points/ContactPoints'; @@ -158,33 +157,28 @@ export const TemplateForm = ({ originalTemplate, prefill, alertmanager }: Props) } }; - const actionButtons = useMemo( - () => ( - - - - Cancel - - - ), - [alertmanager, isSubmitting] + const actionButtons = ( + + + + Cancel + + ); - usePageToolbar(actionButtons); - return ( <> - {!runtimeConfig.featureToggles.singleTopNav && } +
{/* error message */} {error && ( diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 046e1387429..6a72bdcab21 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { FormProvider, SubmitErrorHandler, useForm, UseFormWatch } from 'react-hook-form'; import { useParams } from 'react-router-dom-v5-compat'; @@ -7,7 +7,6 @@ import { GrafanaTheme2 } from '@grafana/data'; import { config, locationService } from '@grafana/runtime'; import { Button, ConfirmModal, CustomScrollbar, Spinner, Stack, useStyles2 } from '@grafana/ui'; import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; -import { usePageToolbar } from 'app/core/components/Page/Page'; import { useAppNotification } from 'app/core/copy/appNotification'; import { contextSrv } from 'app/core/core'; import InfoPausedRule from 'app/features/alerting/unified/components/InfoPausedRule'; @@ -136,66 +135,52 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { }; // @todo why is error not propagated to form? - const submit = useCallback( - async (values: RuleFormValues, exitOnSave: boolean) => { - if (conditionErrorMsg !== '') { - notifyApp.error(conditionErrorMsg); - return; - } + const submit = async (values: RuleFormValues, exitOnSave: boolean) => { + if (conditionErrorMsg !== '') { + notifyApp.error(conditionErrorMsg); + return; + } - trackAlertRuleFormSaved({ formAction: existing ? 'update' : 'create', ruleType: values.type }); + trackAlertRuleFormSaved({ formAction: existing ? 'update' : 'create', ruleType: values.type }); - const ruleDefinition = grafanaTypeRule - ? formValuesToRulerGrafanaRuleDTO(values) - : formValuesToRulerRuleDTO(values); + const ruleDefinition = grafanaTypeRule ? formValuesToRulerGrafanaRuleDTO(values) : formValuesToRulerRuleDTO(values); - const ruleGroupIdentifier = existing - ? getRuleGroupLocationFromRuleWithLocation(existing) - : getRuleGroupLocationFromFormValues(values); + const ruleGroupIdentifier = existing + ? getRuleGroupLocationFromRuleWithLocation(existing) + : getRuleGroupLocationFromFormValues(values); - // @TODO move this to a hook too to make sure the logic here is tested for regressions? - if (!existing) { - // when creating a new rule, we save the manual routing setting , and editorSettings.simplifiedQueryEditor to the local storage - storeInLocalStorageValues(values); - await addRuleToRuleGroup.execute(ruleGroupIdentifier, ruleDefinition, evaluateEvery); - } else { - const ruleIdentifier = fromRulerRuleAndRuleGroupIdentifier(ruleGroupIdentifier, existing.rule); - const targetRuleGroupIdentifier = getRuleGroupLocationFromFormValues(values); - await updateRuleInRuleGroup.execute( - ruleGroupIdentifier, - ruleIdentifier, - ruleDefinition, - targetRuleGroupIdentifier, - evaluateEvery - ); - } + // @TODO move this to a hook too to make sure the logic here is tested for regressions? + if (!existing) { + // when creating a new rule, we save the manual routing setting , and editorSettings.simplifiedQueryEditor to the local storage + storeInLocalStorageValues(values); + await addRuleToRuleGroup.execute(ruleGroupIdentifier, ruleDefinition, evaluateEvery); + } else { + const ruleIdentifier = fromRulerRuleAndRuleGroupIdentifier(ruleGroupIdentifier, existing.rule); + const targetRuleGroupIdentifier = getRuleGroupLocationFromFormValues(values); + await updateRuleInRuleGroup.execute( + ruleGroupIdentifier, + ruleIdentifier, + ruleDefinition, + targetRuleGroupIdentifier, + evaluateEvery + ); + } - const { dataSourceName, namespaceName, groupName } = ruleGroupIdentifier; - if (exitOnSave) { - const returnTo = queryParams.get('returnTo') || getReturnToUrl(ruleGroupIdentifier, ruleDefinition); + const { dataSourceName, namespaceName, groupName } = ruleGroupIdentifier; + if (exitOnSave) { + const returnTo = queryParams.get('returnTo') || getReturnToUrl(ruleGroupIdentifier, ruleDefinition); - locationService.push(returnTo); - return; - } + locationService.push(returnTo); + return; + } - // Cloud Ruler rules identifier changes on update due to containing rule name and hash components - // After successful update we need to update the URL to avoid displaying 404 errors - if (isCloudRulerRule(ruleDefinition)) { - const updatedRuleIdentifier = fromRulerRule(dataSourceName, namespaceName, groupName, ruleDefinition); - locationService.replace(`/alerting/${encodeURIComponent(stringifyIdentifier(updatedRuleIdentifier))}/edit`); - } - }, - [ - addRuleToRuleGroup, - conditionErrorMsg, - evaluateEvery, - existing, - grafanaTypeRule, - notifyApp, - queryParams, - updateRuleInRuleGroup, - ] - ); + // Cloud Ruler rules identifier changes on update due to containing rule name and hash components + // After successful update we need to update the URL to avoid displaying 404 errors + if (isCloudRulerRule(ruleDefinition)) { + const updatedRuleIdentifier = fromRulerRule(dataSourceName, namespaceName, groupName, ruleDefinition); + locationService.replace(`/alerting/${encodeURIComponent(stringifyIdentifier(updatedRuleIdentifier))}/edit`); + } + }; const deleteRule = async () => { if (existing) { @@ -208,80 +193,73 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { } }; - const onInvalid: SubmitErrorHandler = useCallback( - (errors): void => { - trackAlertRuleFormError({ - grafana_version: config.buildInfo.version, - org_id: contextSrv.user.orgId, - user_id: contextSrv.user.id, - error: Object.keys(errors).toString(), - formAction: existing ? 'update' : 'create', - }); - notifyApp.error('There are errors in the form. Please correct them and try again!'); - }, - [existing, notifyApp] - ); + const onInvalid: SubmitErrorHandler = (errors): void => { + trackAlertRuleFormError({ + grafana_version: config.buildInfo.version, + org_id: contextSrv.user.orgId, + user_id: contextSrv.user.id, + error: Object.keys(errors).toString(), + formAction: existing ? 'update' : 'create', + }); + notifyApp.error('There are errors in the form. Please correct them and try again!'); + }; - const cancelRuleCreation = useCallback(() => { + const cancelRuleCreation = () => { logInfo(LogMessages.cancelSavingAlertRule); trackAlertRuleFormCancelled({ formAction: existing ? 'update' : 'create' }); locationService.getHistory().goBack(); - }, [existing]); + }; const evaluateEveryInForm = watch('evaluateEvery'); useEffect(() => setEvaluateEvery(evaluateEveryInForm), [evaluateEveryInForm]); - const actionButtons = useMemo( - () => ( - - {existing && ( - - )} + const actionButtons = ( + + {existing && ( - + + {existing ? ( + - {existing ? ( - - ) : null} - {existing && isCortexLokiOrRecordingRule(watch) && ( - - )} - - ), - [cancelRuleCreation, existing, handleSubmit, isSubmitting, onInvalid, styles.buttonSpinner, submit, watch] + ) : null} + {existing && isCortexLokiOrRecordingRule(watch) && ( + + )} + ); - usePageToolbar(actionButtons); const isPaused = existing && isGrafanaRulerRule(existing.rule) && isGrafanaRulerRulePaused(existing.rule); if (!type) { @@ -289,7 +267,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { } return ( - {!config.featureToggles.singleTopNav && } + e.preventDefault()} className={styles.form}>
{isPaused && } diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx index ee890b033bc..79eef183128 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx @@ -2,9 +2,7 @@ import { memo, useCallback, useEffect, useMemo, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useAsync } from 'react-use'; -import { config } from '@grafana/runtime'; import { Button, CustomScrollbar, LinkButton, LoadingPlaceholder, Stack } from '@grafana/ui'; -import { usePageToolbar } from 'app/core/components/Page/Page'; import { useAppNotification } from 'app/core/copy/appNotification'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; @@ -52,47 +50,39 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor const [conditionErrorMsg, setConditionErrorMsg] = useState(''); const [evaluateEvery, setEvaluateEvery] = useState(ruleForm?.evaluateEvery ?? DEFAULT_GROUP_EVALUATION_INTERVAL); - const onInvalid = useCallback((): void => { + const onInvalid = (): void => { notifyApp.error('There are errors in the form. Please correct them and try again!'); - }, [notifyApp]); + }; const checkAlertCondition = (msg = '') => { setConditionErrorMsg(msg); }; - const submit = useCallback( - (exportData: RuleFormValues | undefined) => { - if (conditionErrorMsg !== '') { - notifyApp.error(conditionErrorMsg); - return; - } - setExportData(exportData); - }, - [conditionErrorMsg, notifyApp] - ); + const submit = (exportData: RuleFormValues | undefined) => { + if (conditionErrorMsg !== '') { + notifyApp.error(conditionErrorMsg); + return; + } + setExportData(exportData); + }; const onClose = useCallback(() => { setExportData(undefined); }, [setExportData]); - const actionButtons = useMemo( - () => [ - submit(undefined)}> - Cancel - , - , - ], - [formAPI, onInvalid, returnTo, submit] - ); - - usePageToolbar(actionButtons); + const actionButtons = [ + submit(undefined)}> + Cancel + , + , + ]; return ( <> - {!config.featureToggles.singleTopNav && } + e.preventDefault()}>
diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx index d338a64be03..88f2bdc68ed 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx @@ -3,10 +3,9 @@ import { useEffect, useMemo } from 'react'; import { useLocation } from 'react-router-dom-v5-compat'; import { GrafanaTheme2, PageLayoutType } from '@grafana/data'; -import { config, useChromeHeaderHeight } from '@grafana/runtime'; +import { useChromeHeaderHeight } from '@grafana/runtime'; import { SceneComponentProps } from '@grafana/scenes'; import { useStyles2 } from '@grafana/ui'; -import { TOP_BAR_LEVEL_HEIGHT } from 'app/core/components/AppChrome/types'; import NativeScrollbar from 'app/core/components/NativeScrollbar'; import { Page } from 'app/core/components/Page/Page'; import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; @@ -15,7 +14,7 @@ import DashboardEmpty from 'app/features/dashboard/dashgrid/DashboardEmpty'; import { useSelector } from 'app/types'; import { DashboardScene } from './DashboardScene'; -import { NavToolbarActions, ToolbarActions } from './NavToolbarActions'; +import { NavToolbarActions } from './NavToolbarActions'; import { PanelSearchLayout } from './PanelSearchLayout'; import { DashboardAngularDeprecationBanner } from './angular/DashboardAngularDeprecationBanner'; @@ -31,7 +30,6 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps { @@ -81,17 +79,12 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps : undefined} - > + {editPanel && } {!editPanel && (
- {!isSingleTopNav && } + {controls && (
@@ -140,7 +133,7 @@ function getStyles(theme: GrafanaTheme2, headerHeight: number) { position: 'sticky', zIndex: theme.zIndex.activePanel, background: theme.colors.background.canvas, - top: config.featureToggles.singleTopNav ? headerHeight + TOP_BAR_LEVEL_HEIGHT : headerHeight, + top: headerHeight, }, }), canvasContent: css({ diff --git a/public/app/features/dashboard-scene/settings/AnnotationsEditView.tsx b/public/app/features/dashboard-scene/settings/AnnotationsEditView.tsx index 0e0b0976a2a..72510de5130 100644 --- a/public/app/features/dashboard-scene/settings/AnnotationsEditView.tsx +++ b/public/app/features/dashboard-scene/settings/AnnotationsEditView.tsx @@ -1,11 +1,11 @@ import { AnnotationQuery, getDataSourceRef, NavModel, NavModelItem, PageLayoutType } from '@grafana/data'; -import { config, getDataSourceSrv } from '@grafana/runtime'; +import { getDataSourceSrv } from '@grafana/runtime'; import { SceneComponentProps, SceneObjectBase, VizPanel, dataLayers } from '@grafana/scenes'; import { Page } from 'app/core/components/Page/Page'; import { DashboardAnnotationsDataLayer } from '../scene/DashboardAnnotationsDataLayer'; import { DashboardScene } from '../scene/DashboardScene'; -import { NavToolbarActions, ToolbarActions } from '../scene/NavToolbarActions'; +import { NavToolbarActions } from '../scene/NavToolbarActions'; import { dataLayersToAnnotations } from '../serialization/dataLayersToAnnotations'; import { dashboardSceneGraph } from '../utils/dashboardSceneGraph'; import { getDashboardSceneFor } from '../utils/utils'; @@ -133,7 +133,6 @@ function AnnotationsSettingsView({ model }: SceneComponentProps : undefined} - > - {!isSingleTopNav && } + + : undefined} - > - {!isSingleTopNav && } + + : undefined} - > - {!isSingleTopNav && } + + : undefined} - > - {!isSingleTopNav && } + + ); diff --git a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx index 1540c2fbbd0..73b1d8cdceb 100644 --- a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx +++ b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx @@ -25,7 +25,7 @@ import { GenAIDashTitleButton } from 'app/features/dashboard/components/GenAI/Ge import { updateNavModel } from '../pages/utils'; import { DashboardScene } from '../scene/DashboardScene'; -import { NavToolbarActions, ToolbarActions } from '../scene/NavToolbarActions'; +import { NavToolbarActions } from '../scene/NavToolbarActions'; import { dashboardSceneGraph } from '../utils/dashboardSceneGraph'; import { getDashboardSceneFor } from '../utils/utils'; @@ -177,16 +177,10 @@ export class GeneralSettingsEditView const { intervals } = model.getRefreshPicker().useState(); const { hideTimeControls } = model.getDashboardControls().useState(); const { enabled: liveNow } = model.getLiveNowTimer().useState(); - const isSingleTopNav = config.featureToggles.singleTopNav; return ( - : undefined} - > - {!isSingleTopNav && } + +
i const { navModel, pageNav } = useDashboardEditPageNav(dashboard, model.getUrlKey()); const canSave = dashboard.useState().meta.canSave; const { jsonText } = model.useState(); - const isSingleTopNav = config.featureToggles.singleTopNav; const onSave = async (overwrite: boolean) => { const result = await onSaveDashboard(dashboard, JSON.parse(model.state.jsonText), { @@ -176,13 +174,8 @@ export class JsonModelEditView extends SceneObjectBase i ); } return ( - : undefined} - > - {!isSingleTopNav && } + +
The JSON model below is the data structure that defines the dashboard. This includes dashboard settings, diff --git a/public/app/features/dashboard-scene/settings/PermissionsEditView.tsx b/public/app/features/dashboard-scene/settings/PermissionsEditView.tsx index 727122e9ecb..5dfa1a49659 100644 --- a/public/app/features/dashboard-scene/settings/PermissionsEditView.tsx +++ b/public/app/features/dashboard-scene/settings/PermissionsEditView.tsx @@ -1,5 +1,4 @@ import { PageLayoutType } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { SceneComponentProps, SceneObjectBase } from '@grafana/scenes'; import { Permissions } from 'app/core/components/AccessControl'; import { Page } from 'app/core/components/Page/Page'; @@ -7,7 +6,7 @@ import { contextSrv } from 'app/core/core'; import { AccessControlAction } from 'app/types'; import { DashboardScene } from '../scene/DashboardScene'; -import { NavToolbarActions, ToolbarActions } from '../scene/NavToolbarActions'; +import { NavToolbarActions } from '../scene/NavToolbarActions'; import { getDashboardSceneFor } from '../utils/utils'; import { DashboardEditView, DashboardEditViewState, useDashboardEditPageNav } from './utils'; @@ -35,16 +34,10 @@ function PermissionsEditorSettings({ model }: SceneComponentProps : undefined} - > - {!isSingleTopNav && } + + ); diff --git a/public/app/features/dashboard-scene/settings/VariablesEditView.tsx b/public/app/features/dashboard-scene/settings/VariablesEditView.tsx index 993bcd0fb10..ffdfe4e7856 100644 --- a/public/app/features/dashboard-scene/settings/VariablesEditView.tsx +++ b/public/app/features/dashboard-scene/settings/VariablesEditView.tsx @@ -1,10 +1,9 @@ import { NavModel, NavModelItem, PageLayoutType } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { SceneComponentProps, SceneObjectBase, SceneVariable, SceneVariables, sceneGraph } from '@grafana/scenes'; import { Page } from 'app/core/components/Page/Page'; import { DashboardScene } from '../scene/DashboardScene'; -import { NavToolbarActions, ToolbarActions } from '../scene/NavToolbarActions'; +import { NavToolbarActions } from '../scene/NavToolbarActions'; import { getDashboardSceneFor } from '../utils/utils'; import { EditListViewSceneUrlSync } from './EditListViewSceneUrlSync'; @@ -207,7 +206,6 @@ function VariableEditorSettingsListView({ model }: SceneComponentProps : undefined} - > - {!isSingleTopNav && } + + : undefined} - > - {!isSingleTopNav && } + + 1; const hasMore = model.versions.length >= model.limit; const isLastPage = model.versions.find((rev) => rev.version === 1); - const isSingleTopNav = config.featureToggles.singleTopNav; const viewModeCompare = ( <> @@ -239,13 +237,8 @@ function VersionsEditorSettingsListView({ model }: SceneComponentProps : undefined} - > - {!isSingleTopNav && } + + {viewMode === 'compare' ? viewModeCompare : viewModeList} ); diff --git a/public/app/features/dashboard/components/DashNav/DashNav.tsx b/public/app/features/dashboard/components/DashNav/DashNav.tsx index 3d92e3731c3..86a3986be31 100644 --- a/public/app/features/dashboard/components/DashNav/DashNav.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNav.tsx @@ -16,6 +16,7 @@ import { Badge, } from '@grafana/ui'; import { updateNavIndex } from 'app/core/actions'; +import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; import { NavToolbarSeparator } from 'app/core/components/AppChrome/NavToolbar/NavToolbarSeparator'; import config from 'app/core/config'; import { useAppNotification } from 'app/core/copy/appNotification'; @@ -82,7 +83,6 @@ export const DashNav = memo((props) => { // this ensures the component rerenders when the location changes useLocation(); const forceUpdate = useForceUpdate(); - const isSingleTopNav = config.featureToggles.singleTopNav; // 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); @@ -357,11 +357,15 @@ export const DashNav = memo((props) => { }; return ( - <> - {renderLeftActions()} - {!isSingleTopNav && } - {renderRightActions()} - + + {renderLeftActions()} + + {renderRightActions()} + + } + /> ); }); diff --git a/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx b/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx index 303cae3ac2b..6b31090b447 100644 --- a/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx +++ b/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx @@ -5,12 +5,12 @@ import { AccessControlAction } from 'app/types'; import { SettingsPageProps } from '../DashboardSettings/types'; -export const AccessControlDashboardPermissions = ({ dashboard, sectionNav, toolbar }: SettingsPageProps) => { +export const AccessControlDashboardPermissions = ({ dashboard, sectionNav }: SettingsPageProps) => { const canSetPermissions = contextSrv.hasPermission(AccessControlAction.DashboardsPermissionsWrite); const pageNav = sectionNav.node.parentItem; return ( - + ); diff --git a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx index e345cf1533d..158d80b9dcc 100644 --- a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx @@ -7,7 +7,7 @@ import { AnnotationSettingsEdit, AnnotationSettingsList, newAnnotationName } fro import { SettingsPageProps } from './types'; -export function AnnotationsSettings({ dashboard, editIndex, sectionNav, toolbar }: SettingsPageProps) { +export function AnnotationsSettings({ dashboard, editIndex, sectionNav }: SettingsPageProps) { const onNew = () => { const newAnnotation: AnnotationQuery = { name: newAnnotationName, @@ -27,7 +27,7 @@ export function AnnotationsSettings({ dashboard, editIndex, sectionNav, toolbar const isEditing = editIndex != null && editIndex < dashboard.annotations.list.length; return ( - + {!isEditing && } {isEditing && } diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx index ab2fd4d6a21..adb6af01c37 100644 --- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx @@ -4,7 +4,7 @@ import { useLocation } from 'react-router-dom-v5-compat'; import { locationUtil, NavModel, NavModelItem } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { config, locationService } from '@grafana/runtime'; +import { locationService } from '@grafana/runtime'; import { Button, Stack, Text, ToolbarButtonRow } from '@grafana/ui'; import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; import { Page } from 'app/core/components/Page/Page'; @@ -36,7 +36,6 @@ const onClose = () => locationService.partial({ editview: null, editIndex: null export function DashboardSettings({ dashboard, editview, pageNav, sectionNav }: Props) { const [updateId, setUpdateId] = useState(0); - const isSingleTopNav = config.featureToggles.singleTopNav; useEffect(() => { dashboard.events.subscribe(DashboardMetaChangedEvent, () => setUpdateId((v) => v + 1)); }, [dashboard]); @@ -82,15 +81,8 @@ export function DashboardSettings({ dashboard, editview, pageNav, sectionNav }: return ( <> - {!isSingleTopNav && ( - {actions}} /> - )} - {actions} : undefined} - sectionNav={subSectionNav} - dashboard={dashboard} - editIndex={editIndex} - /> + {actions}} /> + ); } @@ -217,9 +209,9 @@ function getSectionNav( }; } -function MakeEditable({ dashboard, sectionNav, toolbar }: SettingsPageProps) { +function MakeEditable({ dashboard, sectionNav }: SettingsPageProps) { return ( - + Dashboard not editable