diff --git a/.betterer.results b/.betterer.results index d25b88cfaaa..b7208959ef4 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3877,9 +3877,6 @@ exports[`better eslint`] = { "public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], - "public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"] - ], "public/app/features/dashboard/components/DashboardSettings/GeneralSettings.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], diff --git a/e2e/dashboards-suite/dashboard-time-zone.spec.ts b/e2e/dashboards-suite/dashboard-time-zone.spec.ts index da3924ac58d..510abeccdb7 100644 --- a/e2e/dashboards-suite/dashboard-time-zone.spec.ts +++ b/e2e/dashboards-suite/dashboard-time-zone.spec.ts @@ -61,7 +61,8 @@ e2e.scenario({ e2e.components.Select.option().should('be.visible').contains(toTimeZone).click(); // click to go back to the dashboard. - e2e.components.BackButton.backArrow().click({ force: true }).wait(2000); + e2e.components.BackButton.backArrow().click({ force: true }).wait(5000); + e2e.components.RefreshPicker.runButtonV2().click(); for (const title of panelsToCheck) { e2e.components.Panels.Panel.containerByTitle(title) diff --git a/packages/grafana-data/src/types/navModel.ts b/packages/grafana-data/src/types/navModel.ts index 453e1a1f572..50b2396e6a0 100644 --- a/packages/grafana-data/src/types/navModel.ts +++ b/packages/grafana-data/src/types/navModel.ts @@ -31,6 +31,7 @@ export interface NavModelItem extends NavLinkDTO { highlightId?: string; tabSuffix?: ComponentType<{ className?: string }>; showIconInNavbar?: boolean; + hideFromBreadcrumbs?: boolean; } export enum NavSection { diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index a81bc793b36..ff2aa956c4b 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -72,7 +72,7 @@ export const Pages = { * @deprecated use components.TimeZonePicker.containerV2 from Grafana 8.3 instead */ timezone: 'Time zone picker select container', - title: 'Dashboard settings page title', + title: 'Tab General', }, Annotations: { List: { diff --git a/packages/grafana-ui/src/components/QueryEditor/Stack.tsx b/packages/grafana-ui/src/components/QueryEditor/Stack.tsx index b5d50499c22..3143cfb473f 100644 --- a/packages/grafana-ui/src/components/QueryEditor/Stack.tsx +++ b/packages/grafana-ui/src/components/QueryEditor/Stack.tsx @@ -10,6 +10,7 @@ interface StackProps { alignItems?: CSSProperties['alignItems']; wrap?: boolean; gap?: number; + flexGrow?: CSSProperties['flexGrow']; } export const Stack: React.FC = ({ children, ...props }) => { @@ -25,5 +26,6 @@ const getStyles = (theme: GrafanaTheme2, props: StackProps) => ({ flexWrap: props.wrap ?? true ? 'wrap' : undefined, alignItems: props.alignItems, gap: theme.spacing(props.gap ?? 2), + flexGrow: props.flexGrow, }), }); diff --git a/packages/grafana-ui/src/components/Tabs/VerticalTab.tsx b/packages/grafana-ui/src/components/Tabs/VerticalTab.tsx index 6ae690c19a0..fe1e90b2e24 100644 --- a/packages/grafana-ui/src/components/Tabs/VerticalTab.tsx +++ b/packages/grafana-ui/src/components/Tabs/VerticalTab.tsx @@ -25,7 +25,7 @@ export const VerticalTab = React.forwardRef( const linkClass = cx(tabsStyles.link, active && tabsStyles.activeStyle); return ( -
  • +
    ( > {content()} -
    • + ); } ); diff --git a/packages/grafana-ui/src/themes/GlobalStyles/page.ts b/packages/grafana-ui/src/themes/GlobalStyles/page.ts index 442e8ba5cfd..8e39b1b907c 100644 --- a/packages/grafana-ui/src/themes/GlobalStyles/page.ts +++ b/packages/grafana-ui/src/themes/GlobalStyles/page.ts @@ -112,5 +112,16 @@ export function getPageStyles(theme: GrafanaTheme2) { margin-left: ${theme.spacing(1)}; margin-top: ${theme.spacing(0.5)}; } + + .dashboard-content { + display: 'flex'; + flex-grow: 1; + min-height: 0; + flex-direction: 'column'; + } + + .dashboard-content--hidden { + display: none; + } `; } diff --git a/public/app/core/components/Breadcrumbs/utils.ts b/public/app/core/components/Breadcrumbs/utils.ts index 6bea1e7bf06..c8dc2b45c33 100644 --- a/public/app/core/components/Breadcrumbs/utils.ts +++ b/public/app/core/components/Breadcrumbs/utils.ts @@ -10,7 +10,9 @@ export function buildBreadcrumbs(sectionNav: NavModelItem, pageNav?: NavModelIte addCrumbs(node.parentItem); } - crumbs.push({ text: node.text, href: node.url ?? '' }); + if (!node.hideFromBreadcrumbs) { + crumbs.push({ text: node.text, href: node.url ?? '' }); + } } addCrumbs(sectionNav); diff --git a/public/app/core/components/PageNew/SectionNav.tsx b/public/app/core/components/PageNew/SectionNav.tsx index 2bed640b5e6..b309a047817 100644 --- a/public/app/core/components/PageNew/SectionNav.tsx +++ b/public/app/core/components/PageNew/SectionNav.tsx @@ -22,7 +22,7 @@ export function SectionNav(props: Props) { {main.img && {`logo} {props.model.main.text} -
    +
    {directChildren.map((child, index) => { return ( !child.hideFromTabs && @@ -81,9 +81,7 @@ const getStyles = (theme: GrafanaTheme2) => { fontSize: theme.typography.h4.fontSize, margin: 0, }), - items: css({ - // paddingLeft: '9px', - }), + items: css({}), sectionImg: css({ height: 48, }), diff --git a/public/app/core/selectors/navModel.ts b/public/app/core/selectors/navModel.ts index db34a8125d4..49ea7f6c21e 100644 --- a/public/app/core/selectors/navModel.ts +++ b/public/app/core/selectors/navModel.ts @@ -40,6 +40,7 @@ function getSectionRoot(node: NavModelItem): NavModelItem { function enrichNodeWithActiveState(node: NavModelItem): NavModelItem { const nodeCopy = { ...node }; + if (nodeCopy.parentItem) { nodeCopy.parentItem = { ...nodeCopy.parentItem }; const root = nodeCopy.parentItem; @@ -56,6 +57,7 @@ function enrichNodeWithActiveState(node: NavModelItem): NavModelItem { nodeCopy.parentItem = enrichNodeWithActiveState(root); } + return nodeCopy; } diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index a1f0d15eb22..61222b6bbaa 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -117,7 +117,7 @@ export class KeybindingSrv { const search = locationService.getSearchObject(); if (search.editview) { - locationService.partial({ editview: null }); + locationService.partial({ editview: null, editIndex: null }); return; } diff --git a/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsEdit.tsx b/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsEdit.tsx index 17ab1b1014e..0b0168f6102 100644 --- a/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsEdit.tsx +++ b/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsEdit.tsx @@ -3,8 +3,8 @@ import { useAsync } from 'react-use'; import { AnnotationQuery, DataSourceInstanceSettings, getDataSourceRef } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { DataSourcePicker, getDataSourceSrv } from '@grafana/runtime'; -import { Checkbox, CollapsableSection, Field, HorizontalGroup, Input } from '@grafana/ui'; +import { DataSourcePicker, getDataSourceSrv, locationService } from '@grafana/runtime'; +import { Button, Checkbox, Field, FieldSet, HorizontalGroup, Input, Stack } from '@grafana/ui'; import { ColorValueEditor } from 'app/core/components/OptionsUI/color'; import StandardAnnotationQueryEditor from 'app/features/annotations/components/StandardAnnotationQueryEditor'; @@ -62,52 +62,80 @@ export const AnnotationSettingsEdit: React.FC = ({ editIdx, dashboard }) }); }; + const onApply = goBackToList; + + const onPreview = () => { + locationService.partial({ editview: null, editIndex: null }); + }; + + const onDelete = () => { + const annotations = dashboard.annotations.list; + dashboard.annotations.list = [...annotations.slice(0, editIdx), ...annotations.slice(editIdx + 1)]; + goBackToList(); + }; + const isNewAnnotation = annotation.name === newAnnotationName; return (
    - - - - - - - - - - - - - - - - - - +
    + + + + + + + + + + + + + + + + + +

    Query

    {ds?.annotations && ( )} {ds && !ds.annotations && } - +
    + + + + +
    ); }; AnnotationSettingsEdit.displayName = 'AnnotationSettingsEdit'; + +function goBackToList() { + locationService.partial({ editIndex: null }); +} diff --git a/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx b/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx index 1a60995f3c2..7fd8611b5e9 100644 --- a/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx +++ b/public/app/features/dashboard/components/DashboardPermissions/AccessControlDashboardPermissions.tsx @@ -1,17 +1,18 @@ import React from 'react'; import { Permissions } from 'app/core/components/AccessControl'; +import { Page } from 'app/core/components/PageNew/Page'; import { contextSrv } from 'app/core/core'; import { AccessControlAction } from 'app/types'; -import { DashboardModel } from '../../state'; +import { SettingsPageProps } from '../DashboardSettings/types'; -interface Props { - dashboard: DashboardModel; -} - -export const AccessControlDashboardPermissions = ({ dashboard }: Props) => { +export const AccessControlDashboardPermissions = ({ dashboard, sectionNav }: SettingsPageProps) => { const canSetPermissions = contextSrv.hasPermission(AccessControlAction.DashboardsPermissionsWrite); - return ; + return ( + + + + ); }; diff --git a/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx b/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx index b417791bbb4..3c6cd2ff0b0 100644 --- a/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx +++ b/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx @@ -3,6 +3,7 @@ import { connect, ConnectedProps } from 'react-redux'; import { Tooltip, Icon, Button } from '@grafana/ui'; import { SlideDown } from 'app/core/components/Animations/SlideDown'; +import { Page } from 'app/core/components/PageNew/Page'; import AddPermission from 'app/core/components/PermissionList/AddPermission'; import PermissionList from 'app/core/components/PermissionList/PermissionList'; import PermissionsInfo from 'app/core/components/PermissionList/PermissionsInfo'; @@ -10,13 +11,13 @@ import { StoreState } from 'app/types'; import { DashboardAcl, PermissionLevel, NewDashboardAclItem } from 'app/types/acl'; import { checkFolderPermissions } from '../../../folders/state/actions'; -import { DashboardModel } from '../../state/DashboardModel'; import { getDashboardPermissions, addDashboardPermission, removeDashboardPermission, updateDashboardPermission, } from '../../state/actions'; +import { SettingsPageProps } from '../DashboardSettings/types'; const mapStateToProps = (state: StoreState) => ({ permissions: state.dashboard.permissions, @@ -33,11 +34,7 @@ const mapDispatchToProps = { const connector = connect(mapStateToProps, mapDispatchToProps); -export interface OwnProps { - dashboard: DashboardModel; -} - -export type Props = OwnProps & ConnectedProps; +export type Props = SettingsPageProps & ConnectedProps; export interface State { isAdding: boolean; @@ -91,20 +88,20 @@ export class DashboardPermissionsUnconnected extends PureComponent } render() { - const { - permissions, - dashboard: { - meta: { hasUnsavedFolderChange }, - }, - } = this.props; + const { permissions, dashboard, sectionNav } = this.props; const { isAdding } = this.state; - return hasUnsavedFolderChange ? ( -
    You have changed a folder, please save to view permissions.
    - ) : ( -
    + if (dashboard.meta.hasUnsavedFolderChange) { + return ( + +
    You have changed a folder, please save to view permissions.
    +     + ); + } + + return ( +
    -

    Permissions

    }> @@ -123,7 +120,7 @@ export class DashboardPermissionsUnconnected extends PureComponent isFetching={false} folderInfo={this.getFolder()} /> -
    +     ); } } diff --git a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx index e793a736f28..a12eda258b9 100644 --- a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx @@ -2,15 +2,35 @@ import { within } from '@testing-library/dom'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { BrowserRouter } from 'react-router-dom'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { selectors } from '@grafana/e2e-selectors'; -import { setAngularLoader, setDataSourceSrv } from '@grafana/runtime'; +import { locationService, setAngularLoader, setDataSourceSrv } from '@grafana/runtime'; +import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { mockDataSource, MockDataSourceSrv } from 'app/features/alerting/unified/mocks'; import { DashboardModel } from '../../state/DashboardModel'; import { AnnotationsSettings } from './AnnotationsSettings'; +function setup(dashboard: DashboardModel, editIndex?: number) { + const sectionNav = { + main: { text: 'Dashboard' }, + node: { + text: 'Annotations', + }, + }; + + return render( + + + + + + ); +} + describe('AnnotationsSettings', () => { let dashboard: DashboardModel; @@ -20,7 +40,6 @@ describe('AnnotationsSettings', () => { name: 'Grafana', uid: 'uid1', type: 'grafana', - isDefault: true, }, { annotations: true } ), @@ -79,52 +98,28 @@ describe('AnnotationsSettings', () => { }); }); - test('it renders a header and cta if no annotations or only builtIn annotation', async () => { - render(); + test('it renders empty list cta if only builtIn annotation', async () => { + setup(dashboard); - expect(screen.getByRole('heading', { name: /annotations/i })).toBeInTheDocument(); expect(screen.queryByRole('table')).toBeInTheDocument(); expect(screen.getByRole('row', { name: /annotations & alerts \(built\-in\) grafana/i })).toBeInTheDocument(); expect( screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query')) ).toBeInTheDocument(); expect(screen.queryByRole('link', { name: /annotations documentation/i })).toBeInTheDocument(); + }); - await userEvent.click(screen.getByRole('cell', { name: /annotations & alerts \(built\-in\)/i })); + test('it renders empty list if annotations', async () => { + dashboard.annotations.list = []; + setup(dashboard); - const heading = screen.getByRole('heading', { - name: /annotations edit/i, - }); - const nameInput = screen.getByRole('textbox', { name: /name/i }); - - expect(heading).toBeInTheDocument(); - - await userEvent.clear(nameInput); - await userEvent.type(nameInput, 'My Annotation'); - - expect(screen.queryByText(/grafana/i)).toBeInTheDocument(); - expect(screen.getByRole('checkbox', { name: /hidden/i })).toBeChecked(); - - await userEvent.click(within(heading).getByText(/annotations/i)); - - expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getByRole('row', { name: /my annotation \(built\-in\) grafana/i })).toBeInTheDocument(); - expect( - screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query')) - ).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /new query/i })).not.toBeInTheDocument(); - - await userEvent.click(screen.getAllByLabelText(/Delete query with title/)[0]); - await userEvent.click(screen.getByRole('button', { name: 'Delete' })); - - expect(screen.queryAllByRole('row').length).toBe(0); expect( screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query')) ).toBeInTheDocument(); }); - test('it renders the anotation names or uid if annotation doesnt exist', async () => { - const annotationsList = [ + test('it renders the annotation names or uid if annotation doesnt exist', async () => { + dashboard.annotations.list = [ ...dashboard.annotations.list, { builtIn: 0, @@ -145,20 +140,14 @@ describe('AnnotationsSettings', () => { type: 'dashboard', }, ]; - const dashboardWithAnnotations = new DashboardModel({ - ...dashboard, - annotations: { - list: [...annotationsList], - }, - }); - render(); + setup(dashboard); // Check that we have the correct annotations expect(screen.queryByText(/prometheus/i)).toBeInTheDocument(); expect(screen.queryByText(/deletedAnnotationId/i)).toBeInTheDocument(); }); test('it renders a sortable table of annotations', async () => { - const annotationsList = [ + dashboard.annotations.list = [ ...dashboard.annotations.list, { builtIn: 0, @@ -179,13 +168,9 @@ describe('AnnotationsSettings', () => { type: 'dashboard', }, ]; - const dashboardWithAnnotations = new DashboardModel({ - ...dashboard, - annotations: { - list: [...annotationsList], - }, - }); - render(); + + setup(dashboard); + // Check that we have sorting buttons expect(within(getTableBodyRows()[0]).queryByRole('button', { name: 'arrow-up' })).not.toBeInTheDocument(); expect(within(getTableBodyRows()[0]).queryByRole('button', { name: 'arrow-down' })).toBeInTheDocument(); @@ -211,18 +196,26 @@ describe('AnnotationsSettings', () => { expect(within(getTableBodyRows()[2]).queryByText(/annotations & alerts/i)).toBeInTheDocument(); }); - test('it renders a form for adding/editing annotations', async () => { - render(); + test('Adding a new annotation', async () => { + setup(dashboard); await userEvent.click(screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query'))); - const heading = screen.getByRole('heading', { - name: /annotations edit/i, + expect(locationService.getSearchObject().editIndex).toBe('1'); + expect(dashboard.annotations.list.length).toBe(2); + }); + + test('Editing annotation', async () => { + dashboard.annotations.list.push({ + name: 'New annotation query', + datasource: { uid: 'uid2', type: 'testdata' }, + iconColor: 'red', + enable: true, }); + + setup(dashboard, 1); + const nameInput = screen.getByRole('textbox', { name: /name/i }); - - expect(heading).toBeInTheDocument(); - await userEvent.clear(nameInput); await userEvent.type(nameInput, 'My Prometheus Annotation'); @@ -234,25 +227,14 @@ describe('AnnotationsSettings', () => { await userEvent.click(screen.getByText(/prometheus/i)); expect(screen.getByRole('checkbox', { name: /hidden/i })).not.toBeChecked(); + }); - await userEvent.click(within(heading).getByText(/annotations/i)); + test('Deleting annotation', async () => { + setup(dashboard, 0); - expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(2); - expect(screen.queryByRole('row', { name: /my prometheus annotation prometheus/i })).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /new query/i })).toBeInTheDocument(); - expect( - screen.queryByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query')) - ).not.toBeInTheDocument(); - - await userEvent.click(screen.getByRole('button', { name: /new query/i })); - - await userEvent.click(within(screen.getByRole('heading', { name: /annotations edit/i })).getByText(/annotations/i)); - - expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(3); - - await userEvent.click(screen.getAllByLabelText(/Delete query with title/)[0]); await userEvent.click(screen.getByRole('button', { name: 'Delete' })); - expect(within(screen.getAllByRole('rowgroup')[1]).getAllByRole('row').length).toBe(2); + expect(locationService.getSearchObject().editIndex).toBe(undefined); + expect(dashboard.annotations.list.length).toBe(0); }); }); diff --git a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx index 0ce57e741af..0c6157c976c 100644 --- a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.tsx @@ -1,24 +1,15 @@ -import React, { useState } from 'react'; +import React from 'react'; -import { AnnotationQuery, getDataSourceRef } from '@grafana/data'; -import { getDataSourceSrv } from '@grafana/runtime'; +import { AnnotationQuery, getDataSourceRef, NavModelItem } from '@grafana/data'; +import { getDataSourceSrv, locationService } from '@grafana/runtime'; +import { Page } from 'app/core/components/PageNew/Page'; -import { DashboardModel } from '../../state/DashboardModel'; +import { DashboardModel } from '../../state'; import { AnnotationSettingsEdit, AnnotationSettingsList, newAnnotationName } from '../AnnotationSettings'; -import { DashboardSettingsHeader } from './DashboardSettingsHeader'; - -interface Props { - dashboard: DashboardModel; -} - -export const AnnotationsSettings: React.FC = ({ dashboard }) => { - const [editIdx, setEditIdx] = useState(null); - - const onGoBack = () => { - setEditIdx(null); - }; +import { SettingsPageProps } from './types'; +export function AnnotationsSettings({ dashboard, editIndex, sectionNav }: SettingsPageProps) { const onNew = () => { const newAnnotation: AnnotationQuery = { name: newAnnotationName, @@ -28,20 +19,34 @@ export const AnnotationsSettings: React.FC = ({ dashboard }) => { }; dashboard.annotations.list = [...dashboard.annotations.list, { ...newAnnotation }]; - setEditIdx(dashboard.annotations.list.length - 1); + locationService.partial({ editIndex: dashboard.annotations.list.length - 1 }); }; const onEdit = (idx: number) => { - setEditIdx(idx); + locationService.partial({ editIndex: idx }); }; - const isEditing = editIdx !== null; + const isEditing = editIndex != null && editIndex < dashboard.annotations.list.length; return ( - <> - + {!isEditing && } - {isEditing && } - + {isEditing && } + ); -}; +} + +function getSubPageNav(dashboard: DashboardModel, editIndex: number | undefined): NavModelItem | undefined { + if (editIndex == null) { + return undefined; + } + + const editItem = dashboard.annotations.list[editIndex]; + if (editItem) { + return { + text: editItem.name, + }; + } + + return undefined; +} diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx index 2c3c2f8f19c..1c51bb3c2f5 100644 --- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx @@ -1,10 +1,12 @@ import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; import React from 'react'; import { Provider } from 'react-redux'; import { BrowserRouter } from 'react-router-dom'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; -import { locationService, setBackendSrv } from '@grafana/runtime'; +import { NavModel, NavModelItem } from '@grafana/data'; +import { setBackendSrv } from '@grafana/runtime'; +import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { configureStore } from 'app/store/configureStore'; import { DashboardModel } from '../../state'; @@ -24,7 +26,6 @@ setBackendSrv({ describe('DashboardSettings', () => { it('pressing escape navigates away correctly', async () => { - jest.spyOn(locationService, 'partial'); const dashboard = new DashboardModel( { title: 'Foo', @@ -33,23 +34,22 @@ describe('DashboardSettings', () => { folderId: 1, } ); + const store = configureStore(); + const context = getGrafanaContextMock(); + const sectionNav: NavModel = { main: { text: 'Dashboards' }, node: { text: 'Dashboards' } }; + const pageNav: NavModelItem = { text: 'My cool dashboard' }; + render( - - - - - + + + + + + + ); - expect( - screen.getByText( - (_, el) => el?.tagName.toLowerCase() === 'h1' && /Foo\s*\/\s*Settings/.test(el?.textContent ?? '') - ) - ).toBeInTheDocument(); - - await userEvent.keyboard('{Escape}'); - - expect(locationService.partial).toHaveBeenCalledWith({ editview: null }); + expect(await screen.findByRole('heading', { name: 'Settings' })).toBeInTheDocument(); }); }); diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx index 29c294e4c7d..f1a69f2eb40 100644 --- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx @@ -1,13 +1,11 @@ -import { css, cx } from '@emotion/css'; -import { useDialog } from '@react-aria/dialog'; -import { FocusScope } from '@react-aria/focus'; -import { useOverlay } from '@react-aria/overlays'; -import React, { useCallback, useMemo, useRef } from 'react'; -import { Link } from 'react-router-dom'; +import * as H from 'history'; +import React, { useMemo } from 'react'; +import { useLocation } from 'react-router-dom'; -import { GrafanaTheme2, locationUtil } from '@grafana/data'; -import { locationService, reportInteraction } from '@grafana/runtime'; -import { Button, CustomScrollbar, Icon, IconName, PageToolbar, stylesFactory, useForceUpdate } from '@grafana/ui'; +import { locationUtil, NavModel, NavModelItem } from '@grafana/data'; +import { locationService } from '@grafana/runtime'; +import { Button, PageToolbar } from '@grafana/ui'; +import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; import config from 'app/core/config'; import { contextSrv } from 'app/core/services/context_srv'; import { AccessControlAction } from 'app/types'; @@ -23,132 +21,19 @@ import { GeneralSettings } from './GeneralSettings'; import { JsonEditorSettings } from './JsonEditorSettings'; import { LinksSettings } from './LinksSettings'; import { VersionsSettings } from './VersionsSettings'; +import { SettingsPage, SettingsPageProps } from './types'; export interface Props { dashboard: DashboardModel; + sectionNav: NavModel; + pageNav: NavModelItem; editview: string; } -export interface SettingsPage { - id: string; - title: string; - icon: IconName; - component: React.ReactNode; -} +const onClose = () => locationService.partial({ editview: null, editIndex: null }); -const onClose = () => locationService.partial({ editview: null }); - -const MakeEditable = (props: { onMakeEditable: () => any }) => ( -
    -
    Dashboard not editable
    - -
    -); - -export function DashboardSettings({ dashboard, editview }: Props) { - const ref = useRef(null); - const { overlayProps } = useOverlay( - { - isOpen: true, - onClose, - }, - ref - ); - const { dialogProps } = useDialog( - { - 'aria-label': 'Dashboard settings', - }, - ref - ); - const forceUpdate = useForceUpdate(); - const onMakeEditable = useCallback(() => { - dashboard.editable = true; - dashboard.meta.canMakeEditable = false; - dashboard.meta.canEdit = true; - dashboard.meta.canSave = true; - forceUpdate(); - }, [dashboard, forceUpdate]); - - const pages = useMemo((): SettingsPage[] => { - const pages: SettingsPage[] = []; - - if (dashboard.meta.canEdit) { - pages.push({ - title: 'General', - id: 'settings', - icon: 'sliders-v-alt', - component: , - }); - - pages.push({ - title: 'Annotations', - id: 'annotations', - icon: 'comment-alt', - component: , - }); - - pages.push({ - title: 'Variables', - id: 'templating', - icon: 'calculator-alt', - component: , - }); - - pages.push({ - title: 'Links', - id: 'links', - icon: 'link', - component: , - }); - } - - if (dashboard.meta.canMakeEditable) { - pages.push({ - title: 'General', - icon: 'sliders-v-alt', - id: 'settings', - component: , - }); - } - - if (dashboard.id && dashboard.meta.canSave) { - pages.push({ - title: 'Versions', - id: 'versions', - icon: 'history', - component: , - }); - } - - if (dashboard.id && dashboard.meta.canAdmin) { - if (!config.rbacEnabled) { - pages.push({ - title: 'Permissions', - id: 'permissions', - icon: 'lock', - component: , - }); - } else if (contextSrv.hasPermission(AccessControlAction.DashboardsPermissionsRead)) { - pages.push({ - title: 'Permissions', - id: 'permissions', - icon: 'lock', - component: , - }); - } - } - - pages.push({ - title: 'JSON Model', - id: 'dashboard_json', - icon: 'arrow', - component: , - }); - - return pages; - }, [dashboard, onMakeEditable]); +export function DashboardSettings({ dashboard, editview, pageNav, sectionNav }: Props) { + const pages = useMemo(() => getSettingsPages(dashboard), [dashboard]); const onPostSave = () => { dashboard.meta.hasUnsavedFolderChange = false; @@ -158,69 +43,183 @@ export function DashboardSettings({ dashboard, editview }: Props) { const currentPage = pages.find((page) => page.id === editview) ?? pages[0]; const canSaveAs = contextSrv.hasEditPermissionInFolders; const canSave = dashboard.meta.canSave; - const styles = getStyles(config.theme2); + const location = useLocation(); + const editIndex = getEditIndex(location); + const subSectionNav = getSectionNav(pageNav, sectionNav, pages, currentPage, location); + + const actions = [ + canSaveAs && ( + + ), + canSave && , + ]; return ( - -
    - - -
    -
    - -
    {currentPage.component}
    -
    -
    -     -
    -     + <> + {!config.featureToggles.topnav ? ( + + {actions} + + ) : ( + + )} + + ); } -const getStyles = stylesFactory((theme: GrafanaTheme2) => ({ - scrollInner: css` - min-width: 100%; - display: flex; - `, - toolbar: css` - width: 60vw; - min-width: min-content; - `, - settingsWrapper: css` - margin: ${theme.spacing(0, 2, 2)}; - display: flex; - flex-grow: 1; - `, - settingsContent: css` - flex-grow: 1; - height: 100%; - padding: 32px; - border: 1px solid ${theme.colors.border.weak}; - background: ${theme.colors.background.primary}; - border-radius: ${theme.shape.borderRadius()}; - `, -})); +function getSettingsPages(dashboard: DashboardModel) { + const pages: SettingsPage[] = []; + + if (dashboard.meta.canEdit) { + pages.push({ + title: 'General', + id: 'settings', + icon: 'sliders-v-alt', + component: GeneralSettings, + }); + + pages.push({ + title: 'Annotations', + id: 'annotations', + icon: 'comment-alt', + component: AnnotationsSettings, + subTitle: + 'Annotation queries return events that can be visualized as event markers in graphs across the dashboard.', + }); + + pages.push({ + title: 'Variables', + id: 'templating', + icon: 'calculator-alt', + component: VariableEditorContainer, + subTitle: 'Variables can make your dashboard more dynamic and act as global filters.', + }); + + pages.push({ + title: 'Links', + id: 'links', + icon: 'link', + component: LinksSettings, + }); + } + + if (dashboard.meta.canMakeEditable) { + pages.push({ + title: 'General', + icon: 'sliders-v-alt', + id: 'settings', + component: MakeEditable, + }); + } + + if (dashboard.id && dashboard.meta.canSave) { + pages.push({ + title: 'Versions', + id: 'versions', + icon: 'history', + component: VersionsSettings, + }); + } + + if (dashboard.id && dashboard.meta.canAdmin) { + if (!config.rbacEnabled) { + pages.push({ + title: 'Permissions', + id: 'permissions', + icon: 'lock', + component: DashboardPermissions, + }); + } else if (contextSrv.hasPermission(AccessControlAction.DashboardsPermissionsRead)) { + pages.push({ + title: 'Permissions', + id: 'permissions', + icon: 'lock', + component: AccessControlDashboardPermissions, + }); + } + } + + pages.push({ + title: 'JSON Model', + id: 'dashboard_json', + icon: 'arrow', + component: JsonEditorSettings, + }); + + return pages; +} + +function getSectionNav( + pageNav: NavModelItem, + sectionNav: NavModel, + pages: SettingsPage[], + currentPage: SettingsPage, + location: H.Location +): NavModel { + const main: NavModelItem = { + text: 'Settings', + children: [], + icon: 'apps', + hideFromBreadcrumbs: true, + }; + + main.children = pages.map((page) => ({ + text: page.title, + icon: page.icon, + id: page.id, + url: locationUtil.getUrlForPartial(location, { editview: page.id, editIndex: null }), + active: page === currentPage, + parentItem: main, + subTitle: page.subTitle, + })); + + if (pageNav.parentItem) { + pageNav = { + ...pageNav, + parentItem: { + ...pageNav.parentItem, + parentItem: sectionNav.node, + }, + }; + } else { + pageNav = { + ...pageNav, + parentItem: sectionNav.node, + }; + } + + main.parentItem = pageNav; + + return { + main, + node: main.children.find((x) => x.active)!, + }; +} + +function MakeEditable({ dashboard }: SettingsPageProps) { + const onMakeEditable = () => { + dashboard.editable = true; + dashboard.meta.canMakeEditable = false; + dashboard.meta.canEdit = true; + dashboard.meta.canSave = true; + // TODO add some kind of reload + }; + + return ( +
    +
    Dashboard not editable
    + +
    + ); +} + +function getEditIndex(location: H.Location): number | undefined { + const editIndex = new URLSearchParams(location.search).get('editIndex'); + if (editIndex != null) { + return parseInt(editIndex, 10); + } + return undefined; +} diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettingsHeader.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettingsHeader.tsx index 4e1485f9239..da6ddcacffc 100644 --- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettingsHeader.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettingsHeader.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import { config } from '@grafana/runtime'; import { Icon, HorizontalGroup } from '@grafana/ui'; type Props = { @@ -9,6 +10,10 @@ type Props = { }; export const DashboardSettingsHeader: React.FC = ({ onGoBack, isEditing, title }) => { + if (config.featureToggles.topnav) { + return null; + } + return (
    diff --git a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.test.tsx index 0b91d43932c..b13be0bdf34 100644 --- a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.test.tsx @@ -1,11 +1,14 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { BrowserRouter } from 'react-router-dom'; import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { byRole } from 'testing-library-selector'; import { selectors } from '@grafana/e2e-selectors'; import { setBackendSrv } from '@grafana/runtime'; +import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { DashboardModel } from '../../state'; @@ -34,10 +37,23 @@ const setupTestContext = (options: Partial) => { ), updateTimeZone: jest.fn(), updateWeekStart: jest.fn(), + sectionNav: { + main: { text: 'Dashboard' }, + node: { + text: 'Settings', + }, + }, }; const props = { ...defaults, ...options }; - const { rerender } = render(); + + const { rerender } = render( + + + + + + ); return { rerender, props }; }; diff --git a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx index d76cedd757d..f67eac1933b 100644 --- a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx @@ -2,23 +2,19 @@ import React, { useState } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { TimeZone } from '@grafana/data'; -import { selectors } from '@grafana/e2e-selectors'; import { config } from '@grafana/runtime'; import { CollapsableSection, Field, Input, RadioButtonGroup, TagsInput } from '@grafana/ui'; +import { Page } from 'app/core/components/PageNew/Page'; import { FolderPicker } from 'app/core/components/Select/FolderPicker'; import { updateTimeZoneDashboard, updateWeekStartDashboard } from 'app/features/dashboard/state/actions'; -import { DashboardModel } from '../../state/DashboardModel'; import { DeleteDashboardButton } from '../DeleteDashboard/DeleteDashboardButton'; import { PreviewSettings } from './PreviewSettings'; import { TimePickerSettings } from './TimePickerSettings'; +import { SettingsPageProps } from './types'; -interface OwnProps { - dashboard: DashboardModel; -} - -export type Props = OwnProps & ConnectedProps; +export type Props = SettingsPageProps & ConnectedProps; const GRAPH_TOOLTIP_OPTIONS = [ { value: 0, label: 'Default' }, @@ -26,7 +22,12 @@ const GRAPH_TOOLTIP_OPTIONS = [ { value: 2, label: 'Shared Tooltip' }, ]; -export function GeneralSettingsUnconnected({ dashboard, updateTimeZone, updateWeekStart }: Props): JSX.Element { +export function GeneralSettingsUnconnected({ + dashboard, + updateTimeZone, + updateWeekStart, + sectionNav, +}: Props): JSX.Element { const [renderCounter, setRenderCounter] = useState(0); const onFolderChange = (folder: { id: number; title: string }) => { @@ -90,73 +91,76 @@ export function GeneralSettingsUnconnected({ dashboard, updateTimeZone, updateWe ]; return ( -
    -

    - General -

    -
    - - - - - - - - - - - - + +
    +
    + + + + + + + + + + + + - - - + + + +
    + + {config.featureToggles.dashboardPreviews && config.featureToggles.dashboardPreviewsAdmin && ( + + )} + + + + {/* @todo: Update "Graph tooltip" description to remove prompt about reloading when resolving #46581 */} + + + + + + +
    + {dashboard.meta.canDelete && } +
    - - {config.featureToggles.dashboardPreviews && config.featureToggles.dashboardPreviewsAdmin && ( - - )} - - - - {/* @todo: Update "Graph tooltip" description to remove prompt about reloading when resolving #46581 */} - - - - - - -
    - {dashboard.meta.canDelete && } -
    -
    + ); } diff --git a/public/app/features/dashboard/components/DashboardSettings/JsonEditorSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/JsonEditorSettings.tsx index 7dea62f90a5..0e30eba1448 100644 --- a/public/app/features/dashboard/components/DashboardSettings/JsonEditorSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/JsonEditorSettings.tsx @@ -3,17 +3,15 @@ import React, { useState } from 'react'; import AutoSizer from 'react-virtualized-auto-sizer'; import { GrafanaTheme2 } from '@grafana/data'; -import { Button, CodeEditor, HorizontalGroup, useStyles2 } from '@grafana/ui'; +import { Button, CodeEditor, Stack, useStyles2 } from '@grafana/ui'; +import { Page } from 'app/core/components/PageNew/Page'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; import { getDashboardSrv } from '../../services/DashboardSrv'; -import { DashboardModel } from '../../state/DashboardModel'; -interface Props { - dashboard: DashboardModel; -} +import { SettingsPageProps } from './types'; -export const JsonEditorSettings: React.FC = ({ dashboard }) => { +export function JsonEditorSettings({ dashboard, sectionNav }: SettingsPageProps) { const [dashboardJson, setDashboardJson] = useState(JSON.stringify(dashboard.getSaveModelClone(), null, 2)); const onBlur = (value: string) => { setDashboardJson(value); @@ -28,44 +26,41 @@ export const JsonEditorSettings: React.FC = ({ dashboard }) => { }; const styles = useStyles2(getStyles); + const subTitle = + 'The JSON model below is the data structure that defines the dashboard. This includes dashboard settings, panel settings, layout, queries, and so on'; return ( -
    -

    JSON Model

    -
    - The JSON model below is the data structure that defines the dashboard. This includes dashboard settings, panel - settings, layout, queries, and so on. -
    + +
    -
    - - {({ width, height }) => ( - + +
    + + {({ width, height }) => ( + + )} + +
    +
    + {dashboard.meta.canSave && ( + )} - -
    - {dashboard.meta.canSave && ( - - - - )} -
    +
    + + ); -}; +} -const getStyles = (theme: GrafanaTheme2) => ({ - editWrapper: css` - height: calc(100vh - 250px); - margin-bottom: 10px; - `, +const getStyles = (_: GrafanaTheme2) => ({ + editWrapper: css({ flexGrow: 1 }), }); diff --git a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx index 1430ae5b03b..65291bc874c 100644 --- a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.test.tsx @@ -2,72 +2,88 @@ import { within } from '@testing-library/dom'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { BrowserRouter } from 'react-router-dom'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { selectors } from '@grafana/e2e-selectors'; +import { GrafanaContext } from 'app/core/context/GrafanaContext'; + +import { DashboardModel } from '../../state'; import { LinksSettings } from './LinksSettings'; -describe('LinksSettings', () => { - let dashboard = {}; - const links = [ - { - asDropdown: false, - icon: 'external link', - includeVars: false, - keepTime: false, - tags: [], - targetBlank: false, - title: 'link 1', - tooltip: '', - type: 'link', - url: 'https://www.google.com', +function setup(dashboard: DashboardModel) { + const sectionNav = { + main: { text: 'Dashboard' }, + node: { + text: 'Links', }, - { - asDropdown: false, - icon: 'external link', - includeVars: false, - keepTime: false, - tags: ['gdev'], - targetBlank: false, - title: 'link 2', - tooltip: '', - type: 'dashboards', - url: '', - }, - { - asDropdown: false, - icon: 'external link', - includeVars: false, - keepTime: false, - tags: [], - targetBlank: false, - title: '', - tooltip: '', - type: 'link', - url: 'https://www.bing.com', - }, - ]; + }; + return render( + + + + + + ); +} + +function buildTestDashboard() { + return new DashboardModel({ + links: [ + { + asDropdown: false, + icon: 'external link', + includeVars: false, + keepTime: false, + tags: [], + targetBlank: false, + title: 'link 1', + tooltip: '', + type: 'link', + url: 'https://www.google.com', + }, + { + asDropdown: false, + icon: 'external link', + includeVars: false, + keepTime: false, + tags: ['gdev'], + targetBlank: false, + title: 'link 2', + tooltip: '', + type: 'dashboards', + url: '', + }, + { + asDropdown: false, + icon: 'external link', + includeVars: false, + keepTime: false, + tags: [], + targetBlank: false, + title: '', + tooltip: '', + type: 'link', + url: 'https://www.bing.com', + }, + ], + }); +} + +describe('LinksSettings', () => { const getTableBody = () => screen.getAllByRole('rowgroup')[1]; const getTableBodyRows = () => within(getTableBody()).getAllByRole('row'); const assertRowHasText = (index: number, text: string) => { expect(within(getTableBodyRows()[index]).queryByText(text)).toBeInTheDocument(); }; - beforeEach(() => { - dashboard = { - id: 74, - version: 7, - links: [...links], - }; - }); - test('it renders a header and cta if no links', () => { - const linklessDashboard = { ...dashboard, links: [] }; - // @ts-ignore - render(); + const linklessDashboard = new DashboardModel({ links: [] }); + setup(linklessDashboard); - expect(screen.getByRole('heading', { name: 'Dashboard links' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Links' })).toBeInTheDocument(); expect( screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add dashboard link')) ).toBeInTheDocument(); @@ -75,18 +91,19 @@ describe('LinksSettings', () => { }); test('it renders a table of links', () => { - // @ts-ignore - render(); + const dashboard = buildTestDashboard(); + setup(dashboard); - expect(getTableBodyRows().length).toBe(links.length); + expect(getTableBodyRows().length).toBe(dashboard.links.length); expect( screen.queryByTestId(selectors.components.CallToActionCard.buttonV2('Add dashboard link')) ).not.toBeInTheDocument(); }); test('it rearranges the order of dashboard links', async () => { - // @ts-ignore - render(); + const dashboard = buildTestDashboard(); + const links = dashboard.links; + setup(dashboard); // Check that we have sorting buttons expect(within(getTableBodyRows()[0]).queryByRole('button', { name: 'arrow-up' })).not.toBeInTheDocument(); @@ -114,33 +131,36 @@ describe('LinksSettings', () => { }); test('it duplicates dashboard links', async () => { - // @ts-ignore - render(); + const dashboard = buildTestDashboard(); + setup(dashboard); - expect(getTableBodyRows().length).toBe(links.length); + expect(getTableBodyRows().length).toBe(dashboard.links.length); await userEvent.click(within(getTableBody()).getAllByRole('button', { name: /copy/i })[0]); - expect(getTableBodyRows().length).toBe(links.length + 1); - expect(within(getTableBody()).getAllByText(links[0].title).length).toBe(2); + expect(getTableBodyRows().length).toBe(4); + expect(within(getTableBody()).getAllByText(dashboard.links[0].title).length).toBe(2); }); test('it deletes dashboard links', async () => { - // @ts-ignore - render(); + const dashboard = buildTestDashboard(); + const originalLinks = dashboard.links; + setup(dashboard); - expect(getTableBodyRows().length).toBe(links.length); + expect(getTableBodyRows().length).toBe(dashboard.links.length); await userEvent.click(within(getTableBody()).getAllByLabelText(/Delete link with title/)[0]); await userEvent.click(within(getTableBody()).getByRole('button', { name: 'Delete' })); - expect(getTableBodyRows().length).toBe(links.length - 1); - expect(within(getTableBody()).queryByText(links[0].title)).not.toBeInTheDocument(); + expect(getTableBodyRows().length).toBe(2); + expect(within(getTableBody()).queryByText(originalLinks[0].title)).not.toBeInTheDocument(); }); test('it renders a form which modifies dashboard links', async () => { - // @ts-ignore - render(); + const dashboard = buildTestDashboard(); + const originalLinks = dashboard.links; + setup(dashboard); + await userEvent.click(screen.getByRole('button', { name: /new/i })); expect(screen.queryByText('Type')).toBeInTheDocument(); @@ -164,21 +184,19 @@ describe('LinksSettings', () => { await userEvent.clear(screen.getByRole('textbox', { name: /title/i })); await userEvent.type(screen.getByRole('textbox', { name: /title/i }), 'New Dashboard Link'); - await userEvent.click( - within(screen.getByRole('heading', { name: /dashboard links edit/i })).getByText(/dashboard links/i) - ); - expect(getTableBodyRows().length).toBe(links.length + 1); + await userEvent.click(screen.getByRole('button', { name: /Apply/i })); + + expect(getTableBodyRows().length).toBe(4); expect(within(getTableBody()).queryByText('New Dashboard Link')).toBeInTheDocument(); - await userEvent.click(screen.getAllByText(links[0].type)[0]); + await userEvent.click(screen.getAllByText(dashboard.links[0].type)[0]); await userEvent.clear(screen.getByRole('textbox', { name: /title/i })); await userEvent.type(screen.getByRole('textbox', { name: /title/i }), 'The first dashboard link'); - await userEvent.click( - within(screen.getByRole('heading', { name: /dashboard links edit/i })).getByText(/dashboard links/i) - ); - expect(within(getTableBody()).queryByText(links[0].title)).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole('button', { name: /Apply/i })); + + expect(within(getTableBody()).queryByText(originalLinks[0].title)).not.toBeInTheDocument(); expect(within(getTableBody()).queryByText('The first dashboard link')).toBeInTheDocument(); }); }); diff --git a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.tsx index b000a62e4ae..9a677e5f5a6 100644 --- a/public/app/features/dashboard/components/DashboardSettings/LinksSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/LinksSettings.tsx @@ -1,17 +1,15 @@ import React, { useState } from 'react'; -import { DashboardModel } from '../../state/DashboardModel'; +import { Page } from 'app/core/components/PageNew/Page'; + import { LinkSettingsEdit, LinkSettingsList } from '../LinksSettings'; import { newLink } from '../LinksSettings/LinkSettingsEdit'; -import { DashboardSettingsHeader } from './DashboardSettingsHeader'; -interface Props { - dashboard: DashboardModel; -} +import { SettingsPageProps } from './types'; export type LinkSettingsMode = 'list' | 'new' | 'edit'; -export const LinksSettings: React.FC = ({ dashboard }) => { +export function LinksSettings({ dashboard, sectionNav }: SettingsPageProps) { const [editIdx, setEditIdx] = useState(null); const onGoBack = () => { @@ -30,10 +28,9 @@ export const LinksSettings: React.FC = ({ dashboard }) => { const isEditing = editIdx !== null; return ( - <> - + {!isEditing && } {isEditing && } - + ); -}; +} diff --git a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx index ddd45639e4d..0a490013b42 100644 --- a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.test.tsx @@ -3,6 +3,10 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { UserEvent } from '@testing-library/user-event/dist/types/setup'; import React from 'react'; +import { BrowserRouter } from 'react-router-dom'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; + +import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { DashboardModel } from '../../state/DashboardModel'; import { historySrv } from '../VersionHistory/HistorySrv'; @@ -23,7 +27,7 @@ const queryByFullText = (text: string) => return false; }); -describe('VersionSettings', () => { +function setup() { const dashboard = new DashboardModel({ id: 74, version: 11, @@ -31,6 +35,23 @@ describe('VersionSettings', () => { getRelativeTime: jest.fn(() => 'time ago'), }); + const sectionNav = { + main: { text: 'Dashboard' }, + node: { + text: 'Versions', + }, + }; + + return render( + + + + + + ); +} + +describe('VersionSettings', () => { let user: UserEvent; beforeEach(() => { @@ -48,7 +69,7 @@ describe('VersionSettings', () => { test('renders a header and a loading indicator followed by results in a table', async () => { // @ts-ignore historySrv.getHistoryList.mockResolvedValue(versions); - render(); + setup(); expect(screen.getByRole('heading', { name: /versions/i })).toBeInTheDocument(); expect(screen.queryByText(/fetching history list/i)).toBeInTheDocument(); @@ -67,7 +88,7 @@ describe('VersionSettings', () => { test('does not render buttons if versions === 1', async () => { // @ts-ignore historySrv.getHistoryList.mockResolvedValue(versions.slice(0, 1)); - render(); + setup(); expect(screen.queryByRole('button', { name: /show more versions/i })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: /compare versions/i })).not.toBeInTheDocument(); @@ -81,7 +102,7 @@ describe('VersionSettings', () => { test('does not render show more button if versions < VERSIONS_FETCH_LIMIT', async () => { // @ts-ignore historySrv.getHistoryList.mockResolvedValue(versions.slice(0, VERSIONS_FETCH_LIMIT - 5)); - render(); + setup(); expect(screen.queryByRole('button', { name: /show more versions|/i })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: /compare versions/i })).not.toBeInTheDocument(); @@ -95,7 +116,7 @@ describe('VersionSettings', () => { test('renders buttons if versions >= VERSIONS_FETCH_LIMIT', async () => { // @ts-ignore historySrv.getHistoryList.mockResolvedValue(versions.slice(0, VERSIONS_FETCH_LIMIT)); - render(); + setup(); expect(screen.queryByRole('button', { name: /show more versions/i })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: /compare versions/i })).not.toBeInTheDocument(); @@ -119,7 +140,7 @@ describe('VersionSettings', () => { () => new Promise((resolve) => setTimeout(() => resolve(versions.slice(VERSIONS_FETCH_LIMIT)), 1000)) ); - render(); + setup(); expect(historySrv.getHistoryList).toBeCalledTimes(1); @@ -148,7 +169,7 @@ describe('VersionSettings', () => { .mockImplementationOnce(() => Promise.resolve(diffs.lhs)) .mockImplementationOnce(() => Promise.resolve(diffs.rhs)); - render(); + setup(); expect(historySrv.getHistoryList).toBeCalledTimes(1); @@ -163,7 +184,7 @@ describe('VersionSettings', () => { await user.click(compareButton); - await waitFor(() => expect(screen.getByRole('heading', { name: /versions comparing 2 11/i })).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole('heading', { name: /comparing 2 11/i })).toBeInTheDocument()); expect(queryByFullText('Version 11 updated by admin')).toBeInTheDocument(); expect(queryByFullText('Version 2 updated by admin')).toBeInTheDocument(); diff --git a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx index 733f91a5a85..0db6b274558 100644 --- a/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/VersionsSettings.tsx @@ -1,8 +1,8 @@ import React, { PureComponent } from 'react'; import { Spinner, HorizontalGroup } from '@grafana/ui'; +import { Page } from 'app/core/components/PageNew/Page'; -import { DashboardModel } from '../../state/DashboardModel'; import { historySrv, RevisionsModel, @@ -12,9 +12,9 @@ import { VersionHistoryComparison, } from '../VersionHistory'; -interface Props { - dashboard: DashboardModel; -} +import { SettingsPageProps } from './types'; + +interface Props extends SettingsPageProps {} type State = { isLoading: boolean; @@ -141,9 +141,8 @@ export class VersionsSettings extends PureComponent { if (viewMode === 'compare') { return ( -
    + { diffData={diffData} /> )} -
    + ); } return ( -
    - + {isLoading ? ( ) : ( @@ -181,7 +179,7 @@ export class VersionsSettings extends PureComponent { isLastPage={!!this.isLastPage()} /> )} -
    + ); } } diff --git a/public/app/features/dashboard/components/DashboardSettings/types.ts b/public/app/features/dashboard/components/DashboardSettings/types.ts new file mode 100644 index 00000000000..1ab89418756 --- /dev/null +++ b/public/app/features/dashboard/components/DashboardSettings/types.ts @@ -0,0 +1,20 @@ +import { ComponentType } from 'react'; + +import { NavModel } from '@grafana/data'; +import { IconName } from '@grafana/ui'; + +import { DashboardModel } from '../../state'; + +export interface SettingsPage { + id: string; + title: string; + icon: IconName; + component: ComponentType; + subTitle?: string; +} + +export interface SettingsPageProps { + dashboard: DashboardModel; + sectionNav: NavModel; + editIndex?: number; +} diff --git a/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx b/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx index 5da57c62171..935b47d63c7 100644 --- a/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx +++ b/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx @@ -35,17 +35,19 @@ export const LinkSettingsList: React.FC = ({ dashboard, o if (isEmptyList) { return ( - Dashboard Links allow you to place links to other dashboards and web sites directly below the dashboard header.

    ', - }} - /> +
    + Dashboard Links allow you to place links to other dashboards and web sites directly below the dashboard header.

    ', + }} + /> +
    ); } diff --git a/public/app/features/dashboard/components/SaveDashboard/SaveDashboardButton.tsx b/public/app/features/dashboard/components/SaveDashboard/SaveDashboardButton.tsx index 354647194cd..001c8d32bfa 100644 --- a/public/app/features/dashboard/components/SaveDashboard/SaveDashboardButton.tsx +++ b/public/app/features/dashboard/components/SaveDashboard/SaveDashboardButton.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { selectors } from '@grafana/e2e-selectors'; -import { Button, ButtonVariant, ModalsController, FullWidthButtonContainer } from '@grafana/ui'; +import { Button, ButtonVariant, ModalsController } from '@grafana/ui'; import { DashboardModel } from 'app/features/dashboard/state'; import { SaveDashboardDrawer } from './SaveDashboardDrawer'; @@ -43,22 +43,20 @@ export const SaveDashboardAsButton: React.FC {({ showModal, hideModal }) => { return ( - - - + ); }} diff --git a/public/app/features/dashboard/components/VersionHistory/VersionHistoryButtons.tsx b/public/app/features/dashboard/components/VersionHistory/VersionHistoryButtons.tsx index f483d276b32..1f5c89e8a16 100644 --- a/public/app/features/dashboard/components/VersionHistory/VersionHistoryButtons.tsx +++ b/public/app/features/dashboard/components/VersionHistory/VersionHistoryButtons.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { HorizontalGroup, Tooltip, Button } from '@grafana/ui'; +import { Tooltip, Button, Stack } from '@grafana/ui'; type VersionsButtonsType = { hasMore: boolean; @@ -16,7 +16,7 @@ export const VersionsHistoryButtons: React.FC = ({ getDiff, isLastPage, }) => ( - + {hasMore && ( - + ); diff --git a/public/app/features/dashboard/components/VersionHistory/VersionHistoryHeader.tsx b/public/app/features/dashboard/components/VersionHistory/VersionHistoryHeader.tsx index c1109172d62..e12ac177fe1 100644 --- a/public/app/features/dashboard/components/VersionHistory/VersionHistoryHeader.tsx +++ b/public/app/features/dashboard/components/VersionHistory/VersionHistoryHeader.tsx @@ -3,10 +3,9 @@ import { noop } from 'lodash'; import React from 'react'; import { GrafanaTheme } from '@grafana/data'; -import { Icon, useStyles } from '@grafana/ui'; +import { Icon, IconButton, useStyles } from '@grafana/ui'; type VersionHistoryHeaderProps = { - isComparing?: boolean; onClick?: () => void; baseVersion?: number; newVersion?: number; @@ -14,7 +13,6 @@ type VersionHistoryHeaderProps = { }; export const VersionHistoryHeader: React.FC = ({ - isComparing = false, onClick = noop, baseVersion = 0, newVersion = 0, @@ -24,15 +22,11 @@ export const VersionHistoryHeader: React.FC = ({ return (

    - - Versions + + + Comparing {baseVersion} {newVersion}{' '} + {isNewLatest && (Latest)} - {isComparing && ( - - Comparing {baseVersion} {newVersion}{' '} - {isNewLatest && (Latest)} - - )}

    ); }; @@ -40,6 +34,8 @@ export const VersionHistoryHeader: React.FC = ({ const getStyles = (theme: GrafanaTheme) => ({ header: css` font-size: ${theme.typography.heading.h3}; + display: flex; + gap: ${theme.spacing.md}; margin-bottom: ${theme.spacing.lg}; `, }); diff --git a/public/app/features/dashboard/containers/DashboardPage.test.tsx b/public/app/features/dashboard/containers/DashboardPage.test.tsx index ad40ed62c2c..32d02964c0e 100644 --- a/public/app/features/dashboard/containers/DashboardPage.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.test.tsx @@ -111,6 +111,9 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo match: { params: { slug: 'my-dash', uid: '11' } } as any, route: { routeName: DashboardRoutes.Normal } as any, }), + navIndex: { + dashboards: { text: 'Dashboards' }, + }, initPhase: DashboardInitPhase.NotStarted, initError: null, initDashboard: jest.fn(), diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index ef9f5b15212..d2b273d3968 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -1,8 +1,8 @@ -import classnames from 'classnames'; +import { cx } from '@emotion/css'; import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; -import { locationUtil, NavModelItem, TimeRange } from '@grafana/data'; +import { locationUtil, NavModel, NavModelItem, TimeRange } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { locationService } from '@grafana/runtime'; import { Themeable2, withTheme2 } from '@grafana/ui'; @@ -12,7 +12,8 @@ import { PageLayoutType } from 'app/core/components/Page/types'; import { createErrorNotification } from 'app/core/copy/appNotification'; import { getKioskMode } from 'app/core/navigation/kiosk'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; -import { DashboardModel, PanelModel } from 'app/features/dashboard/state'; +import { getNavModel } from 'app/core/selectors/navModel'; +import { PanelModel } from 'app/features/dashboard/state'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; import { getPageNavFromSlug, getRootContentNavModel } from 'app/features/storage/StorageFolderPage'; import { DashboardRoutes, KioskMode, StoreState } from 'app/types'; @@ -58,6 +59,7 @@ export const mapStateToProps = (state: StoreState) => ({ initPhase: state.dashboard.initPhase, initError: state.dashboard.initError, dashboard: state.dashboard.getModel(), + navIndex: state.navIndex, }); const mapDispatchToProps = { @@ -88,12 +90,13 @@ export interface State { panelNotFound: boolean; editPanelAccessDenied: boolean; scrollElement?: HTMLDivElement; + pageNav?: NavModelItem; + sectionNav?: NavModel; } export class UnthemedDashboardPage extends PureComponent { private forceRouteReloadCounter = 0; state: State = this.getCleanState(); - pageNav?: NavModelItem; getCleanState(): State { return { @@ -150,8 +153,6 @@ export class UnthemedDashboardPage extends PureComponent { return; } - this.updatePageNav(dashboard); - if ( prevProps.match.params.uid !== match.params.uid || (routeReloadCounter !== undefined && this.forceRouteReloadCounter !== routeReloadCounter) @@ -226,6 +227,8 @@ export class UnthemedDashboardPage extends PureComponent { return state; } + state = updateStatePageNavFromProps(props, state); + // Entering edit mode if (!state.editPanel && urlEditPanelId) { const panel = dashboard.getPanelByUrlId(urlEditPanelId); @@ -319,59 +322,19 @@ export class UnthemedDashboardPage extends PureComponent { return inspectPanel; } - updatePageNav(dashboard: DashboardModel) { - if (!this.pageNav || dashboard.title !== this.pageNav.text) { - this.pageNav = { - text: dashboard.title, - url: locationUtil.getUrlForPartial(this.props.history.location, { - editview: null, - editPanel: null, - viewPanel: null, - }), - }; - } - - // Check if folder changed - if ( - dashboard.meta.folderTitle && - (!this.pageNav.parentItem || this.pageNav.parentItem.text !== dashboard.meta.folderTitle) - ) { - this.pageNav.parentItem = { - text: dashboard.meta.folderTitle, - url: `/dashboards/f/${dashboard.meta.folderUid}`, - }; - } - - if (this.props.route.routeName === DashboardRoutes.Path) { - const pageNav = getPageNavFromSlug(this.props.match.params.slug!); - if (pageNav?.parentItem) { - this.pageNav.parentItem = pageNav.parentItem; - } - } - } - - getPageProps() { - if (this.props.route.routeName === DashboardRoutes.Path) { - return { navModel: getRootContentNavModel(), pageNav: this.pageNav }; - } else { - return { navId: 'dashboards', pageNav: this.pageNav }; - } - } - render() { const { dashboard, initError, queryParams, isPublic } = this.props; - const { editPanel, viewPanel, updateScrollTop } = this.state; + const { editPanel, viewPanel, updateScrollTop, pageNav, sectionNav } = this.state; const kioskMode = !isPublic ? getKioskMode() : KioskMode.Full; - if (!dashboard) { + if (!dashboard || !pageNav || !sectionNav) { return ; } const inspectPanel = this.getInspectPanel(); - const containerClassNames = classnames({ 'panel-in-fullscreen': viewPanel }); - const showSubMenu = !editPanel && kioskMode === KioskMode.Off && !this.props.queryParams.editview; - const toolbar = kioskMode !== KioskMode.Full && ( + + const toolbar = kioskMode !== KioskMode.Full && !queryParams.editview && (
    { ); return ( - - + <> + + - {initError && } - {showSubMenu && ( -
    - -
    + {initError && } + {showSubMenu && ( +
    + +
    + )} + + + + {inspectPanel && } + {editPanel && } +     + {queryParams.editview && ( + )} - - - - {inspectPanel && } - {editPanel && } - {queryParams.editview && } -     + ); } } +function updateStatePageNavFromProps(props: Props, state: State): State { + const { dashboard } = props; + + if (!dashboard) { + return state; + } + + let pageNav = state.pageNav; + let sectionNav = state.sectionNav; + + if (!pageNav || dashboard.title !== pageNav.text) { + pageNav = { + text: dashboard.title, + url: locationUtil.getUrlForPartial(props.history.location, { + editview: null, + editPanel: null, + viewPanel: null, + }), + }; + } + + // Check if folder changed + const { folderTitle } = dashboard.meta; + if (folderTitle && pageNav && pageNav.parentItem?.text !== folderTitle) { + pageNav = { + ...pageNav, + parentItem: { + text: folderTitle, + url: `/dashboards/f/${dashboard.meta.folderUid}`, + }, + }; + } + + if (props.route.routeName === DashboardRoutes.Path) { + sectionNav = getRootContentNavModel(); + const pageNav = getPageNavFromSlug(props.match.params.slug!); + if (pageNav?.parentItem) { + pageNav.parentItem = pageNav.parentItem; + } + } else { + sectionNav = getNavModel(props.navIndex, 'dashboards'); + } + + if (state.pageNav === pageNav && state.sectionNav === sectionNav) { + return state; + } + + return { + ...state, + pageNav, + sectionNav, + }; +} + export const DashboardPage = withTheme2(UnthemedDashboardPage); DashboardPage.displayName = 'DashboardPage'; export default connector(DashboardPage); diff --git a/public/app/features/variables/adhoc/actions.test.ts b/public/app/features/variables/adhoc/actions.test.ts index b070c470385..b99336f9285 100644 --- a/public/app/features/variables/adhoc/actions.test.ts +++ b/public/app/features/variables/adhoc/actions.test.ts @@ -3,7 +3,7 @@ import { locationService } from '@grafana/runtime'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { variableAdapters } from '../adapters'; -import { changeVariableEditorExtended, setIdInEditor } from '../editor/reducer'; +import { changeVariableEditorExtended } from '../editor/reducer'; import { adHocBuilder } from '../shared/testing/builders'; import { getPreloadedState, getRootReducer, RootReducerType } from '../state/helpers'; import { toKeyedAction } from '../state/keyedVariablesReducer'; @@ -441,7 +441,6 @@ describe('adhoc actions', () => { const tester = await reduxTester() .givenRootReducer(getRootReducer()) .whenActionIsDispatched(createAddVariableAction(variable)) - .whenActionIsDispatched(toKeyedAction(key, setIdInEditor({ id: variable.id }))) .whenActionIsDispatched(initAdHocVariableEditor(key)) .whenAsyncActionIsDispatched(changeVariableDatasource(toKeyedVariableIdentifier(variable), datasource), true); @@ -483,7 +482,6 @@ describe('adhoc actions', () => { const tester = await reduxTester() .givenRootReducer(getRootReducer()) .whenActionIsDispatched(createAddVariableAction(variable)) - .whenActionIsDispatched(toKeyedAction(key, setIdInEditor({ id: variable.id }))) .whenActionIsDispatched(initAdHocVariableEditor(key)) .whenAsyncActionIsDispatched(changeVariableDatasource(toKeyedVariableIdentifier(variable), datasource), true); diff --git a/public/app/features/variables/editor/VariableEditorContainer.tsx b/public/app/features/variables/editor/VariableEditorContainer.tsx index 9245d4d93ce..17ca426b9da 100644 --- a/public/app/features/variables/editor/VariableEditorContainer.tsx +++ b/public/app/features/variables/editor/VariableEditorContainer.tsx @@ -1,13 +1,12 @@ -import React, { MouseEvent, PureComponent } from 'react'; +import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { bindActionCreators } from 'redux'; -import { selectors } from '@grafana/e2e-selectors'; -import { Button, Icon } from '@grafana/ui'; -import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; +import { locationService } from '@grafana/runtime'; +import { Page } from 'app/core/components/PageNew/Page'; +import { SettingsPageProps } from 'app/features/dashboard/components/DashboardSettings/types'; import { StoreState, ThunkDispatch } from '../../../types'; -import { VariablesDependenciesButton } from '../inspect/VariablesDependenciesButton'; import { VariablesUnknownTable } from '../inspect/VariablesUnknownTable'; import { toKeyedAction } from '../state/keyedVariablesReducer'; import { getEditorVariables, getVariablesState } from '../state/selectors'; @@ -17,7 +16,7 @@ import { toKeyedVariableIdentifier, toVariablePayload } from '../utils'; import { VariableEditorEditor } from './VariableEditorEditor'; import { VariableEditorList } from './VariableEditorList'; -import { switchToEditMode, switchToListMode, switchToNewMode } from './actions'; +import { createNewVariable, initListMode } from './actions'; const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { const { uid } = ownProps.dashboard; @@ -32,7 +31,7 @@ const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { const mapDispatchToProps = (dispatch: ThunkDispatch) => { return { - ...bindActionCreators({ switchToNewMode, switchToEditMode, switchToListMode }, dispatch), + ...bindActionCreators({ createNewVariable, initListMode }, dispatch), changeVariableOrder: (identifier: KeyedVariableIdentifier, fromIndex: number, toIndex: number) => dispatch( toKeyedAction( @@ -55,30 +54,24 @@ const mapDispatchToProps = (dispatch: ThunkDispatch) => { }; }; -interface OwnProps { - dashboard: DashboardModel; -} +interface OwnProps extends SettingsPageProps {} const connector = connect(mapStateToProps, mapDispatchToProps); type Props = OwnProps & ConnectedProps; class VariableEditorContainerUnconnected extends PureComponent { - componentDidMount(): void { - this.props.switchToListMode(this.props.dashboard.uid); + componentDidMount() { + this.props.initListMode(this.props.dashboard.uid); } - onChangeToListMode = (event: MouseEvent) => { - event.preventDefault(); - this.props.switchToListMode(this.props.dashboard.uid); - }; - onEditVariable = (identifier: KeyedVariableIdentifier) => { - this.props.switchToEditMode(identifier); + const index = this.props.variables.findIndex((x) => x.id === identifier.id); + locationService.partial({ editIndex: index }); }; onNewVariable = () => { - this.props.switchToNewMode(this.props.dashboard.uid); + this.props.createNewVariable(this.props.dashboard.uid); }; onChangeVariableOrder = (identifier: KeyedVariableIdentifier, fromIndex: number, toIndex: number) => { @@ -94,41 +87,12 @@ class VariableEditorContainerUnconnected extends PureComponent { }; render() { - const variableToEdit = this.props.variables.find((s) => s.id === this.props.idInEditor) ?? null; + const { editIndex, variables } = this.props; + const variableToEdit = editIndex != null ? variables[editIndex] : undefined; + const subPageNav = variableToEdit ? { text: variableToEdit.name } : undefined; return ( -
    -
    -

    - - Variables - - {this.props.idInEditor && ( - - - Edit - - )} -

    - -
    - {this.props.variables.length > 0 && variableToEdit === null && ( - <> - - - - )} -
    - + {!variableToEdit && ( { )} {variableToEdit && } -
    + ); } } diff --git a/public/app/features/variables/editor/VariableEditorEditor.tsx b/public/app/features/variables/editor/VariableEditorEditor.tsx index 0433137c267..c427b483630 100644 --- a/public/app/features/variables/editor/VariableEditorEditor.tsx +++ b/public/app/features/variables/editor/VariableEditorEditor.tsx @@ -5,7 +5,8 @@ import { bindActionCreators } from 'redux'; import { AppEvents, LoadingState, SelectableValue, VariableType } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { Button, Icon, InlineFieldRow, VerticalGroup } from '@grafana/ui'; +import { locationService } from '@grafana/runtime'; +import { Button, HorizontalGroup, Icon, InlineFieldRow, VerticalGroup } from '@grafana/ui'; import { appEvents } from '../../../core/core'; import { StoreState, ThunkDispatch } from '../../../types'; @@ -14,7 +15,7 @@ import { hasOptions } from '../guard'; import { updateOptions } from '../state/actions'; import { toKeyedAction } from '../state/keyedVariablesReducer'; import { getVariable, getVariablesState } from '../state/selectors'; -import { changeVariableProp, changeVariableType } from '../state/sharedReducer'; +import { changeVariableProp, changeVariableType, removeVariable } from '../state/sharedReducer'; import { KeyedVariableIdentifier } from '../state/types'; import { VariableHide } from '../types'; import { toKeyedVariableIdentifier, toVariablePayload } from '../utils'; @@ -24,7 +25,7 @@ import { VariableSectionHeader } from './VariableSectionHeader'; import { VariableTextField } from './VariableTextField'; import { VariableTypeSelect } from './VariableTypeSelect'; import { VariableValuesPreview } from './VariableValuesPreview'; -import { changeVariableName, onEditorUpdate, variableEditorMount, variableEditorUnMount } from './actions'; +import { changeVariableName, variableEditorMount, variableEditorUnMount } from './actions'; import { OnPropChangeArguments } from './types'; const mapStateToProps = (state: StoreState, ownProps: OwnProps) => ({ @@ -34,10 +35,7 @@ const mapStateToProps = (state: StoreState, ownProps: OwnProps) => ({ const mapDispatchToProps = (dispatch: ThunkDispatch) => { return { - ...bindActionCreators( - { variableEditorMount, variableEditorUnMount, changeVariableName, onEditorUpdate, updateOptions }, - dispatch - ), + ...bindActionCreators({ variableEditorMount, variableEditorUnMount, changeVariableName, updateOptions }, dispatch), changeVariableProp: (identifier: KeyedVariableIdentifier, propName: string, propValue: any) => dispatch( toKeyedAction( @@ -47,6 +45,11 @@ const mapDispatchToProps = (dispatch: ThunkDispatch) => { ), changeVariableType: (identifier: KeyedVariableIdentifier, newType: VariableType) => dispatch(toKeyedAction(identifier.rootStateKey, changeVariableType(toVariablePayload(identifier, { newType })))), + removeVariable: (identifier: KeyedVariableIdentifier) => { + dispatch( + toKeyedAction(identifier.rootStateKey, removeVariable(toVariablePayload(identifier, { reIndex: true }))) + ); + }, }; }; @@ -100,10 +103,11 @@ export class VariableEditorEditorUnConnected extends PureComponent { this.props.changeVariableProp(this.props.identifier, 'hide', option.value); }; - onPropChanged = async ({ propName, propValue, updateOptions = false }: OnPropChangeArguments) => { + onPropChanged = ({ propName, propValue, updateOptions = false }: OnPropChangeArguments) => { this.props.changeVariableProp(this.props.identifier, propName, propValue); + if (updateOptions) { - await this.props.updateOptions(toKeyedVariableIdentifier(this.props.variable)); + this.props.updateOptions(toKeyedVariableIdentifier(this.props.variable)); } }; @@ -113,7 +117,16 @@ export class VariableEditorEditorUnConnected extends PureComponent { return; } - await this.props.onEditorUpdate(this.props.identifier); + this.props.updateOptions(toKeyedVariableIdentifier(this.props.variable)); + }; + + onDelete = () => { + this.props.removeVariable(this.props.identifier); + locationService.partial({ editIndex: null }); + }; + + onApply = () => { + locationService.partial({ editIndex: null }); }; render() { @@ -176,18 +189,25 @@ export class VariableEditorEditorUnConnected extends PureComponent { {hasOptions(this.props.variable) ? : null} - + + - + +
    diff --git a/public/app/features/variables/editor/VariableEditorList.tsx b/public/app/features/variables/editor/VariableEditorList.tsx index e6c052e358a..98634fc4b02 100644 --- a/public/app/features/variables/editor/VariableEditorList.tsx +++ b/public/app/features/variables/editor/VariableEditorList.tsx @@ -3,8 +3,10 @@ import { DragDropContext, Droppable, DropResult } from 'react-beautiful-dnd'; import { selectors } from '@grafana/e2e-selectors'; import { reportInteraction } from '@grafana/runtime'; +import { Button, Stack } from '@grafana/ui'; import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA'; +import { VariablesDependenciesButton } from '../inspect/VariablesDependenciesButton'; import { UsagesToNetwork, VariableUsageTree } from '../inspect/utils'; import { KeyedVariableIdentifier } from '../state/types'; import { VariableModel } from '../types'; @@ -47,7 +49,7 @@ export function VariableEditorList({ {variables.length === 0 && } {variables.length > 0 && ( -
    +
    -
    + + + + + )}
    diff --git a/public/app/features/variables/editor/actions.test.ts b/public/app/features/variables/editor/actions.test.ts index 57ee80e95a7..fb27b94dbaf 100644 --- a/public/app/features/variables/editor/actions.test.ts +++ b/public/app/features/variables/editor/actions.test.ts @@ -8,8 +8,7 @@ import { initialKeyedVariablesState, toKeyedAction } from '../state/keyedVariabl import * as selectors from '../state/selectors'; import { addVariable } from '../state/sharedReducer'; -import { getNextAvailableId, switchToListMode, switchToNewMode } from './actions'; -import { setIdInEditor } from './reducer'; +import { getNextAvailableId, initListMode, createNewVariable } from './actions'; describe('getNextAvailableId', () => { describe('when called with a custom type and there is already 2 variables', () => { @@ -26,7 +25,7 @@ describe('getNextAvailableId', () => { }); }); -describe('switchToNewMode', () => { +describe('createNewVariable', () => { variableAdapters.setInit(() => [createConstantVariableAdapter()]); it('should dispatch with the correct rootStateKey', () => { @@ -37,16 +36,15 @@ describe('switchToNewMode', () => { const mockDispatch = jest.fn(); const model = { ...initialConstantVariableModelState, name: mockId, id: mockId, rootStateKey: 'null' }; - switchToNewMode(null, 'constant')(mockDispatch, mockGetState, undefined); - expect(mockDispatch).toHaveBeenCalledTimes(2); + createNewVariable(null, 'constant')(mockDispatch, mockGetState, undefined); + expect(mockDispatch).toHaveBeenCalledTimes(1); expect(mockDispatch.mock.calls[0][0]).toEqual( toKeyedAction('null', addVariable({ data: { global: false, index: 0, model }, type: 'constant', id: mockId })) ); - expect(mockDispatch.mock.calls[1][0]).toEqual(toKeyedAction('null', setIdInEditor({ id: mockId }))); }); }); -describe('switchToListMode', () => { +describe('initListMode', () => { variableAdapters.setInit(() => [createConstantVariableAdapter()]); it('should dispatch with the correct rootStateKey', () => { @@ -56,7 +54,7 @@ describe('switchToListMode', () => { const mockGetState = jest.fn().mockReturnValue({ templating: initialKeyedVariablesState, dashboard: initialState }); const mockDispatch = jest.fn(); - switchToListMode(null)(mockDispatch, mockGetState, undefined); + initListMode(null)(mockDispatch, mockGetState, undefined); const keyedAction = { type: expect.any(String), payload: { @@ -64,8 +62,7 @@ describe('switchToListMode', () => { action: expect.any(Object), }, }; - expect(mockDispatch).toHaveBeenCalledTimes(2); + expect(mockDispatch).toHaveBeenCalledTimes(1); expect(mockDispatch.mock.calls[0][0]).toMatchObject(keyedAction); - expect(mockDispatch.mock.calls[1][0]).toMatchObject(keyedAction); }); }); diff --git a/public/app/features/variables/editor/actions.ts b/public/app/features/variables/editor/actions.ts index 263ccac3b0c..e2cd953072b 100644 --- a/public/app/features/variables/editor/actions.ts +++ b/public/app/features/variables/editor/actions.ts @@ -1,12 +1,12 @@ import { cloneDeep } from 'lodash'; import { VariableType } from '@grafana/data'; +import { locationService } from '@grafana/runtime'; import { ThunkResult } from '../../../types'; import { variableAdapters } from '../adapters'; import { initInspect } from '../inspect/reducer'; import { createUsagesNetwork, transformUsagesToNetwork } from '../inspect/utils'; -import { updateOptions } from '../state/actions'; import { toKeyedAction } from '../state/keyedVariablesReducer'; import { getEditorVariables, getNewVariableIndex, getVariable, getVariablesByKey } from '../state/selectors'; import { addVariable, removeVariable } from '../state/sharedReducer'; @@ -17,8 +17,6 @@ import { toKeyedVariableIdentifier, toStateKey, toVariablePayload } from '../uti import { changeVariableNameFailed, changeVariableNameSucceeded, - clearIdInEditor, - setIdInEditor, variableEditorMounted, variableEditorUnMounted, } from './reducer'; @@ -26,7 +24,9 @@ import { export const variableEditorMount = (identifier: KeyedVariableIdentifier): ThunkResult => { return async (dispatch) => { const { rootStateKey } = identifier; - dispatch(toKeyedAction(rootStateKey, variableEditorMounted({ name: getVariable(identifier).name }))); + dispatch( + toKeyedAction(rootStateKey, variableEditorMounted({ name: getVariable(identifier).name, id: identifier.id })) + ); }; }; @@ -37,13 +37,6 @@ export const variableEditorUnMount = (identifier: KeyedVariableIdentifier): Thun }; }; -export const onEditorUpdate = (identifier: KeyedVariableIdentifier): ThunkResult => { - return async (dispatch) => { - await dispatch(updateOptions(identifier)); - dispatch(switchToListMode(identifier.rootStateKey)); - }; -}; - export const changeVariableName = (identifier: KeyedVariableIdentifier, newName: string): ThunkResult => { return (dispatch, getState) => { const { id, rootStateKey: uid } = identifier; @@ -90,11 +83,10 @@ export const completeChangeVariableName = dispatch( toKeyedAction(rootStateKey, changeVariableNameSucceeded(toVariablePayload(renamedIdentifier, { newName }))) ); - dispatch(switchToEditMode(renamedIdentifier)); dispatch(toKeyedAction(rootStateKey, removeVariable(toVariablePayload(identifier, { reIndex: false })))); }; -export const switchToNewMode = +export const createNewVariable = (key: string | null | undefined, type: VariableType = 'query'): ThunkResult => (dispatch, getState) => { const rootStateKey = toStateKey(key); @@ -109,21 +101,14 @@ export const switchToNewMode = dispatch( toKeyedAction(rootStateKey, addVariable(toVariablePayload(identifier, { global, model, index }))) ); - dispatch(toKeyedAction(rootStateKey, setIdInEditor({ id: identifier.id }))); + + locationService.partial({ editIndex: index }); }; -export const switchToEditMode = - (identifier: KeyedVariableIdentifier): ThunkResult => - (dispatch) => { - const { rootStateKey } = identifier; - dispatch(toKeyedAction(rootStateKey, setIdInEditor({ id: identifier.id }))); - }; - -export const switchToListMode = +export const initListMode = (key: string | null | undefined): ThunkResult => (dispatch, getState) => { const rootStateKey = toStateKey(key); - dispatch(toKeyedAction(rootStateKey, clearIdInEditor())); const state = getState(); const variables = getEditorVariables(rootStateKey, state); const dashboard = state.dashboard.getModel(); diff --git a/public/app/features/variables/editor/reducer.test.ts b/public/app/features/variables/editor/reducer.test.ts index dde10201c0a..42e51025fb1 100644 --- a/public/app/features/variables/editor/reducer.test.ts +++ b/public/app/features/variables/editor/reducer.test.ts @@ -7,10 +7,8 @@ import { changeVariableNameFailed, changeVariableNameSucceeded, cleanEditorState, - clearIdInEditor, initialVariableEditorState, removeVariableEditorError, - setIdInEditor, variableEditorMounted, variableEditorReducer, VariableEditorState, @@ -18,39 +16,16 @@ import { } from './reducer'; describe('variableEditorReducer', () => { - describe('when setIdInEditor is dispatched', () => { - it('then state should be correct', () => { - const payload = { id: '0' }; - reducerTester() - .givenReducer(variableEditorReducer, { ...initialVariableEditorState }) - .whenActionIsDispatched(setIdInEditor(payload)) - .thenStateShouldEqual({ - ...initialVariableEditorState, - id: '0', - }); - }); - }); - - describe('when clearIdInEditor is dispatched', () => { - it('then state should be correct', () => { - reducerTester() - .givenReducer(variableEditorReducer, { ...initialVariableEditorState, id: '0' }) - .whenActionIsDispatched(clearIdInEditor()) - .thenStateShouldEqual({ - ...initialVariableEditorState, - }); - }); - }); - describe('when variableEditorMounted is dispatched', () => { it('then state should be correct', () => { - const payload = { name: 'A name' }; + const payload = { name: 'A name', id: '123' }; reducerTester() .givenReducer(variableEditorReducer, { ...initialVariableEditorState }) .whenActionIsDispatched(variableEditorMounted(payload)) .thenStateShouldEqual({ ...initialVariableEditorState, name: 'A name', + id: '123', }); }); }); diff --git a/public/app/features/variables/editor/reducer.ts b/public/app/features/variables/editor/reducer.ts index 530a486bd6c..18eff36a981 100644 --- a/public/app/features/variables/editor/reducer.ts +++ b/public/app/features/variables/editor/reducer.ts @@ -41,14 +41,9 @@ const variableEditorReducerSlice = createSlice({ name: 'templating/editor', initialState: initialVariableEditorState, reducers: { - setIdInEditor: (state: VariableEditorState, action: PayloadAction<{ id: string }>) => { - state.id = action.payload.id; - }, - clearIdInEditor: (state: VariableEditorState, action: PayloadAction) => { - state.id = ''; - }, - variableEditorMounted: (state: VariableEditorState, action: PayloadAction<{ name: string }>) => { + variableEditorMounted: (state: VariableEditorState, action: PayloadAction<{ name: string; id: string }>) => { state.name = action.payload.name; + state.id = action.payload.id; }, variableEditorUnMounted: (state: VariableEditorState, action: PayloadAction) => { return initialVariableEditorState; @@ -93,8 +88,6 @@ const variableEditorReducerSlice = createSlice({ export const variableEditorReducer = variableEditorReducerSlice.reducer; export const { - setIdInEditor, - clearIdInEditor, changeVariableNameSucceeded, changeVariableNameFailed, variableEditorMounted, diff --git a/public/app/features/variables/query/actions.test.tsx b/public/app/features/variables/query/actions.test.tsx index 36c3875f27b..028efcc7431 100644 --- a/public/app/features/variables/query/actions.test.tsx +++ b/public/app/features/variables/query/actions.test.tsx @@ -15,7 +15,7 @@ import { changeVariableEditorExtended, initialVariableEditorState, removeVariableEditorError, - setIdInEditor, + variableEditorMounted, } from '../editor/reducer'; import { updateOptions } from '../state/actions'; import { getPreloadedState, getRootReducer, RootReducerType } from '../state/helpers'; @@ -167,8 +167,8 @@ describe('query actions', () => { .whenActionIsDispatched( toKeyedAction('key', addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable }))) ) + .whenActionIsDispatched(toKeyedAction('key', variableEditorMounted({ name: variable.name, id: variable.id }))) .whenActionIsDispatched(toKeyedAction('key', variablesInitTransaction({ uid: 'key' }))) - .whenActionIsDispatched(toKeyedAction('key', setIdInEditor({ id: variable.id }))) .whenAsyncActionIsDispatched(updateQueryVariableOptions(toKeyedVariableIdentifier(variable)), true); const option = createOption(ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE); @@ -195,13 +195,11 @@ describe('query actions', () => { toKeyedAction('key', addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable }))) ) .whenActionIsDispatched(toKeyedAction('key', variablesInitTransaction({ uid: 'key' }))) - .whenActionIsDispatched(toKeyedAction('key', setIdInEditor({ id: variable.id }))) .whenAsyncActionIsDispatched(updateQueryVariableOptions(toKeyedVariableIdentifier(variable), 'search'), true); const update = { results: optionsMetrics, templatedRegex: '' }; tester.thenDispatchedActionsShouldEqual( - toKeyedAction('key', removeVariableEditorError({ errorProp: 'update' })), toKeyedAction('key', updateVariableOptions(toVariablePayload(variable, update))) ); }); @@ -221,24 +219,19 @@ describe('query actions', () => { toKeyedAction('key', addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable }))) ) .whenActionIsDispatched(toKeyedAction('key', variablesInitTransaction({ uid: 'key' }))) - .whenActionIsDispatched(toKeyedAction('key', setIdInEditor({ id: variable.id }))) .whenAsyncActionIsDispatched(updateOptions(toKeyedVariableIdentifier(variable)), true); tester.thenDispatchedActionsPredicateShouldEqual((dispatchedActions) => { - const expectedNumberOfActions = 5; + const expectedNumberOfActions = 3; expect(dispatchedActions[0]).toEqual(toKeyedAction('key', variableStateFetching(toVariablePayload(variable)))); - expect(dispatchedActions[1]).toEqual(toKeyedAction('key', removeVariableEditorError({ errorProp: 'update' }))); - expect(dispatchedActions[2]).toEqual( - toKeyedAction('key', addVariableEditorError({ errorProp: 'update', errorText: error.message })) - ); - expect(dispatchedActions[3]).toEqual( + expect(dispatchedActions[1]).toEqual( toKeyedAction('key', variableStateFailed(toVariablePayload(variable, { error }))) ); - expect(dispatchedActions[4].type).toEqual(notifyApp.type); - expect(dispatchedActions[4].payload.title).toEqual('Templating [0]'); - expect(dispatchedActions[4].payload.text).toEqual('Error updating options: failed to fetch metrics'); - expect(dispatchedActions[4].payload.severity).toEqual('error'); + expect(dispatchedActions[2].type).toEqual(notifyApp.type); + expect(dispatchedActions[2].payload.title).toEqual('Templating [0]'); + expect(dispatchedActions[2].payload.text).toEqual('Error updating options: failed to fetch metrics'); + expect(dispatchedActions[2].payload.severity).toEqual('error'); return dispatchedActions.length === expectedNumberOfActions; }); diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index 89f5579348f..e2978bc3a26 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -10,12 +10,7 @@ import { createConstantVariableAdapter } from '../constant/adapter'; import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, NEW_VARIABLE_ID } from '../constants'; import { createCustomVariableAdapter } from '../custom/adapter'; import { changeVariableName } from '../editor/actions'; -import { - changeVariableNameFailed, - changeVariableNameSucceeded, - cleanEditorState, - setIdInEditor, -} from '../editor/reducer'; +import { changeVariableNameFailed, changeVariableNameSucceeded, cleanEditorState } from '../editor/reducer'; import { cleanPickerState } from '../pickers/OptionsPicker/reducer'; import { setVariableQueryRunner, VariableQueryRunner } from '../query/VariableQueryRunner'; import { createQueryVariableAdapter } from '../query/adapter'; @@ -511,7 +506,6 @@ describe('shared actions', () => { key, changeVariableNameSucceeded({ type: 'constant', id: 'constant1', data: { newName: 'constant1' } }) ), - toKeyedAction(key, setIdInEditor({ id: 'constant1' })), toKeyedAction(key, removeVariable({ type: 'constant', id: 'constant', data: { reIndex: false } })) ); }); @@ -557,7 +551,6 @@ describe('shared actions', () => { key, changeVariableNameSucceeded({ type: 'constant', id: 'constant1', data: { newName: 'constant1' } }) ), - toKeyedAction(key, setIdInEditor({ id: 'constant1' })), toKeyedAction(key, removeVariable({ type: 'constant', id: NEW_VARIABLE_ID, data: { reIndex: false } })) ); });