diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts index 8e6024148b2..a420df76ebc 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-add-panel.spec.ts @@ -41,7 +41,7 @@ test.describe( // Check that pressing the configure button shows the panel editor await dashboardPage .getByGrafanaSelector(selectors.components.Panels.Panel.content) - .filter({ hasText: 'Configure' }) + .getByRole('button', { name: /configure/i }) .click(); await expect(dashboardPage.getByGrafanaSelector(selectors.components.PanelEditor.General.content)).toBeVisible(); }); diff --git a/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx b/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx index be08c16a5b4..36cd5367f00 100644 --- a/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx +++ b/packages/grafana-ui/src/components/Sidebar/Sidebar.tsx @@ -78,14 +78,12 @@ export function SiderbarToolbar({ children }: SiderbarToolbarProps) {
{children}
- {context.hasOpenPane && ( - - )} +
); } diff --git a/packages/grafana-ui/src/components/Sidebar/useSidebar.test.tsx b/packages/grafana-ui/src/components/Sidebar/useSidebar.test.tsx new file mode 100644 index 00000000000..d368236fb6a --- /dev/null +++ b/packages/grafana-ui/src/components/Sidebar/useSidebar.test.tsx @@ -0,0 +1,113 @@ +import { renderHook } from '@testing-library/react'; + +import { store } from '@grafana/data'; + +import { useSidebarSavedState } from './useSidebar'; + +jest.mock('@grafana/data', () => ({ + ...jest.requireActual('@grafana/data'), + store: { + get: jest.fn(), + set: jest.fn(), + getBool: jest.fn(), + }, +})); + +const mockedStore = jest.mocked(store, { shallow: true }); + +describe('useSidebarSavedState(persistanceKey, subKey, defaultValue)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('when persistanceKey is not passed', () => { + test('returns the default value', () => { + const { result } = renderHook(() => useSidebarSavedState(undefined, 'subKey1', 1)); + + const [state] = result.current; + expect(state).toBe(1); + }); + }); + + describe('if defaultValue is a boolean', () => { + test('reads a boolean value from the store', () => { + mockedStore.getBool.mockReturnValue(true); + + const [persistanceKey, subKey, defaultValue] = ['persistanceKey2', 'subKey2', false]; + const { result } = renderHook(() => useSidebarSavedState(persistanceKey, subKey, defaultValue)); + + expect(mockedStore.getBool).toHaveBeenCalledWith(`grafana.ui.sidebar.${persistanceKey}.${subKey}`, defaultValue); + + const [state] = result.current; + expect(state).toBe(true); + }); + }); + + describe('if defaultValue is a number', () => { + test('converts the value read from the store to an integer', () => { + mockedStore.get.mockReturnValue('7'); + + const [persistanceKey, subKey, defaultValue] = ['persistanceKey3', 'subKey3', 3]; + const { result } = renderHook(() => useSidebarSavedState(persistanceKey, subKey, defaultValue)); + + expect(mockedStore.get).toHaveBeenCalledWith(`grafana.ui.sidebar.${persistanceKey}.${subKey}`); + + const [state] = result.current; + expect(state).toBe(7); + }); + + test('returns defaultValue when the conversion fails', () => { + mockedStore.get.mockReturnValue('seven'); + + const [persistanceKey, subKey, defaultValue] = ['persistanceKey4', 'subKey4', 4]; + const { result } = renderHook(() => useSidebarSavedState(persistanceKey, subKey, defaultValue)); + + expect(mockedStore.get).toHaveBeenCalledWith(`grafana.ui.sidebar.${persistanceKey}.${subKey}`); + + const [state] = result.current; + expect(state).toBe(4); + }); + }); + + describe('if defaultValue is neither a boolean nor a number', () => { + test('returns the default value', () => { + const [persistanceKey, subKey, defaultValue] = ['persistanceKey5', 'subKey5', { test: 'five' }]; + const { result } = renderHook(() => useSidebarSavedState(persistanceKey, subKey, defaultValue)); + + expect(mockedStore.get).not.toHaveBeenCalled(); + + const [state] = result.current; + expect(state).toBe(defaultValue); + }); + }); + + describe('when persistanceKey or subKey changes across re-renders', () => { + test('re-reads the value from the store using the new key', () => { + mockedStore.get.mockImplementation((key: string) => { + const storeValues: Record = { + 'grafana.ui.sidebar.persistanceKey6.subKeyA': '10', + 'grafana.ui.sidebar.persistanceKey7.subKeyA': '20', + 'grafana.ui.sidebar.persistanceKey7.subKeyB': '30', + }; + return storeValues[key]; + }); + + const initialProps = { persistanceKey: 'persistanceKey6', subKey: 'subKeyA', defaultValue: 42 }; + + const { result, rerender } = renderHook( + (props) => useSidebarSavedState(props.persistanceKey, props.subKey, props.defaultValue), + { initialProps } + ); + + expect(result.current[0]).toBe(10); + + rerender({ ...initialProps, persistanceKey: 'persistanceKey7' }); + + expect(result.current[0]).toBe(20); + + rerender({ ...initialProps, persistanceKey: 'persistanceKey7', subKey: 'subKeyB' }); + + expect(result.current[0]).toBe(30); + }); + }); +}); diff --git a/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx b/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx index a7b3dd89919..86bbfb3717f 100644 --- a/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx +++ b/packages/grafana-ui/src/components/Sidebar/useSidebar.tsx @@ -1,5 +1,5 @@ import { clamp } from 'lodash'; -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { store } from '@grafana/data'; @@ -144,33 +144,38 @@ export function useSidebar({ }; } -function useSidebarSavedState( +function readFromStore(persistanceKey: string | undefined, subKey: string, defaultValue: T): T { + if (!persistanceKey) { + return defaultValue; + } + + if (typeof defaultValue === 'boolean') { + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + return store.getBool(`grafana.ui.sidebar.${persistanceKey}.${subKey}`, defaultValue) as T; + } + + if (typeof defaultValue === 'number') { + const value = Number.parseInt(store.get(`grafana.ui.sidebar.${persistanceKey}.${subKey}`), 10); + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + return Number.isNaN(value) ? defaultValue : (value as T); + } + + return defaultValue; +} + +export function useSidebarSavedState( persistanceKey: string | undefined, subKey: string, defaultValue: T ) { - const [state, setState] = React.useState(() => { - if (!persistanceKey) { - return defaultValue; - } + const [state, setState] = React.useState(() => readFromStore(persistanceKey, subKey, defaultValue)); - if (typeof defaultValue === 'boolean') { - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - return store.getBool(`grafana.ui.sidebar.${persistanceKey}.${subKey}`, defaultValue) as T; - } - - if (typeof defaultValue === 'number') { - const value = Number.parseInt(store.get(`grafana.ui.sidebar.${persistanceKey}.${subKey}`), 10); - if (Number.isNaN(value)) { - return defaultValue; - } - - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - return value as T; - } - - return defaultValue; - }); + useEffect(() => { + setState(readFromStore(persistanceKey, subKey, defaultValue)); + // Re-read from storage when the persistence key changes, but not when defaultValue changes + // to avoid overriding a user-persisted value. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [persistanceKey, subKey]); const setPersisted = useCallback( (cb: (prevState: T) => T) => { diff --git a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx index 4b15163d3e3..18200257780 100644 --- a/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx +++ b/public/app/features/dashboard-scene/edit-pane/DashboardEditPaneSplitter.tsx @@ -80,7 +80,8 @@ function DashboardEditPaneSplitterNewLayouts({ dashboard, isEditing, body, contr hasOpenPane: Boolean(openPane), contentMargin: 1, position: 'right', - persistanceKey: 'dashboard', + persistanceKey: isEditing ? 'dashboard' : 'dashboard-view', + defaultToDocked: isEditing ? true : false, onClosePane: () => editPane.closePane(), });