From a7de5182b0318df1441c83c28ad218799bbddebd Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Tue, 4 Oct 2022 15:42:43 +0100 Subject: [PATCH] Explore: Revert split pane resize feature (#56310) * Revert "Explore: Prevent panes from disappearing when resizing window in split view (#55696)" This reverts commit 0a5aa19ca2907c70f86269ddd4b67defe08fd0aa. * Revert "Explore: Add resize to split view, with Min/Max button (#54420)" This reverts commit c3e4f1f8766fc2d9d788652daf5b3a8bf1ea8951. --- .../src/components/PageLayout/PageToolbar.tsx | 6 +- .../SplitPaneWrapper/SplitPaneWrapper.tsx | 16 +++- .../components/SplitPaneWrapper/SplitView.tsx | 93 ------------------- .../features/explore/ExplorePaneContainer.tsx | 12 ++- .../app/features/explore/ExploreToolbar.tsx | 37 +------- public/app/features/explore/QueryRows.tsx | 1 - public/app/features/explore/Wrapper.test.tsx | 42 ++------- public/app/features/explore/Wrapper.tsx | 89 ++---------------- .../app/features/explore/state/main.test.ts | 6 -- public/app/features/explore/state/main.ts | 45 --------- .../query/components/QueryEditorRow.tsx | 3 +- public/app/types/explore.ts | 15 --- 12 files changed, 44 insertions(+), 321 deletions(-) delete mode 100644 public/app/core/components/SplitPaneWrapper/SplitView.tsx diff --git a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx index 8b65ddc728e..c4c5b58a412 100644 --- a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx +++ b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx @@ -24,7 +24,6 @@ export interface Props { className?: string; isFullscreen?: boolean; 'aria-label'?: string; - buttonOverflowAlignment?: 'left' | 'right'; } /** @alpha */ @@ -43,7 +42,6 @@ export const PageToolbar: FC = React.memo( className, /** main nav-container aria-label **/ 'aria-label': ariaLabel, - buttonOverflowAlignment = 'right', }) => { const styles = useStyles2(getStyles); @@ -134,9 +132,7 @@ export const PageToolbar: FC = React.memo( )} - - {React.Children.toArray(children).filter(Boolean)} - + {React.Children.toArray(children).filter(Boolean)} ); } diff --git a/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx b/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx index a84d543b627..8b06c108c50 100644 --- a/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx +++ b/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx @@ -6,8 +6,6 @@ import { GrafanaTheme } from '@grafana/data'; import { stylesFactory } from '@grafana/ui'; import { config } from 'app/core/config'; -import { SplitView } from './SplitView'; - enum Pane { Right, Top, @@ -102,6 +100,8 @@ export class SplitPaneWrapper extends PureComponent { render() { const { rightPaneVisible, rightPaneComponents, uiState } = this.props; // Limit options pane width to 90% of screen. + const styles = getStyles(config.theme); + // Need to handle when width is relative. ie a percentage of the viewport const rightPaneSize = uiState.rightPaneSize <= 1 ? uiState.rightPaneSize * window.innerWidth : uiState.rightPaneSize; @@ -111,10 +111,18 @@ export class SplitPaneWrapper extends PureComponent { } return ( - + (document.body.style.cursor = 'col-resize')} + onDragFinished={(size) => this.onDragFinished(Pane.Right, size)} + > {this.renderHorizontalSplit()} {rightPaneComponents} - + ); } } diff --git a/public/app/core/components/SplitPaneWrapper/SplitView.tsx b/public/app/core/components/SplitPaneWrapper/SplitView.tsx deleted file mode 100644 index cca03f04eea..00000000000 --- a/public/app/core/components/SplitPaneWrapper/SplitView.tsx +++ /dev/null @@ -1,93 +0,0 @@ -import { css } from '@emotion/css'; -import { useViewportSize } from '@react-aria/utils'; -import React, { ReactNode } from 'react'; -import SplitPane from 'react-split-pane'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { useStyles2 } from '@grafana/ui'; - -interface Props { - children: [ReactNode, ReactNode]; - uiState: { rightPaneSize: number }; - minSize?: number; - onResize?: (size: number) => void; -} - -const onDragFinished = (size: number, onResize?: (size: number) => void) => { - document.body.style.cursor = 'auto'; - onResize?.(size); -}; - -const onDragStarted = () => { - document.body.style.cursor = 'row-resize'; -}; - -const getResizerStyles = (hasSplit: boolean) => (theme: GrafanaTheme2) => - css` - position: relative; - display: ${hasSplit ? 'block' : 'none'}; - - &::before { - content: ''; - position: absolute; - transition: 0.2s border-color ease-in-out; - border-right: 1px solid ${theme.colors.border.weak}; - height: 100%; - left: 50%; - transform: translateX(-50%); - } - - &::after { - background: ${theme.colors.border.weak}; - content: ''; - position: absolute; - left: 50%; - top: 50%; - transition: 0.2s background ease-in-out; - transform: translate(-50%, -50%); - border-radius: 4px; - height: 200px; - width: 4px; - } - - &:hover { - &::before { - border-color: ${theme.colors.primary.main}; - } - - &::after { - background: ${theme.colors.primary.main}; - } - } - - cursor: col-resize; - width: ${theme.spacing(2)}; - `; - -export const SplitView = ({ uiState: { rightPaneSize }, children, minSize = 200, onResize }: Props) => { - const { width } = useViewportSize(); - - // create two elements for library, even if only one exists (one will be hidden) - const hasSplit = children.filter(Boolean).length === 2; - - const existingChildren = [ - {children[0]}, - {hasSplit && children[1]}, - ]; - - return ( - onDragFinished(size, onResize)} - > - {existingChildren} - - ); -}; diff --git a/public/app/features/explore/ExplorePaneContainer.tsx b/public/app/features/explore/ExplorePaneContainer.tsx index b9a946c33c8..fb0db9616ef 100644 --- a/public/app/features/explore/ExplorePaneContainer.tsx +++ b/public/app/features/explore/ExplorePaneContainer.tsx @@ -1,4 +1,4 @@ -import { css } from '@emotion/css'; +import { css, cx } from '@emotion/css'; import memoizeOne from 'memoize-one'; import React from 'react'; import { connect, ConnectedProps } from 'react-redux'; @@ -36,12 +36,13 @@ const getStyles = (theme: GrafanaTheme2) => { display: flex; flex: 1 1 auto; flex-direction: column; - overflow: scroll; - min-width: 600px; & + & { border-left: 1px dotted ${theme.colors.border.medium}; } `, + exploreSplit: css` + width: 50%; + `, }; }; @@ -142,10 +143,11 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { }; render() { - const { theme, exploreId, initialized } = this.props; + const { theme, split, exploreId, initialized } = this.props; const styles = getStyles(theme); + const exploreClass = cx(styles.explore, split && styles.exploreSplit); return ( -
+
{initialized && }
); diff --git a/public/app/features/explore/ExploreToolbar.tsx b/public/app/features/explore/ExploreToolbar.tsx index 59b666ebb27..dfaced53e30 100644 --- a/public/app/features/explore/ExploreToolbar.tsx +++ b/public/app/features/explore/ExploreToolbar.tsx @@ -18,7 +18,7 @@ import { getFiscalYearStartMonth, getTimeZone } from '../profile/state/selectors import { ExploreTimeControls } from './ExploreTimeControls'; import { LiveTailButton } from './LiveTailButton'; import { changeDatasource } from './state/datasource'; -import { evenPaneResizeAction, maximizePaneAction, splitClose, splitOpen } from './state/main'; +import { splitClose, splitOpen } from './state/main'; import { cancelQueries, runQueries } from './state/query'; import { isSplit } from './state/selectors'; import { syncTimes, changeRefreshInterval } from './state/time'; @@ -126,7 +126,6 @@ class UnConnectedExploreToolbar extends PureComponent { onChangeTimeZone, onChangeFiscalYearStartMonth, topOfViewRef, - largerExploreId, } = this.props; const showSmallDataSourcePicker = (splitted ? containerWidth < 700 : containerWidth < 800) || false; @@ -136,23 +135,12 @@ class UnConnectedExploreToolbar extends PureComponent { contextSrv.hasAccess(AccessControlAction.DashboardsCreate, contextSrv.isEditor) || contextSrv.hasAccess(AccessControlAction.DashboardsWrite, contextSrv.isEditor); - const isLargerExploreId = largerExploreId === exploreId; - - const onClickResize = () => { - if (isLargerExploreId) { - this.props.evenPaneResizeAction(); - } else { - this.props.maximizePaneAction({ exploreId: exploreId }); - } - }; - return (
{ Split ) : ( - <> - - - Close - - + + Close + )} {config.featureToggles.explore2Dashboard && showExploreToDashboard && ( @@ -258,7 +234,7 @@ class UnConnectedExploreToolbar extends PureComponent { } const mapStateToProps = (state: StoreState, { exploreId }: OwnProps) => { - const { syncedTimes, largerExploreId } = state.explore; + const { syncedTimes } = state.explore; const exploreItem = state.explore[exploreId]!; const { datasourceInstance, datasourceMissing, range, refreshInterval, loading, isLive, isPaused, containerWidth } = exploreItem; @@ -280,7 +256,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps) => { isPaused, syncedTimes, containerWidth, - largerExploreId, }; }; @@ -294,8 +269,6 @@ const mapDispatchToProps = { syncTimes, onChangeTimeZone: updateTimeZoneForSession, onChangeFiscalYearStartMonth: updateFiscalYearStartMonthForSession, - maximizePaneAction, - evenPaneResizeAction, }; const connector = connect(mapStateToProps, mapDispatchToProps); diff --git a/public/app/features/explore/QueryRows.tsx b/public/app/features/explore/QueryRows.tsx index fa51d05e9a8..a1b7b0ce782 100644 --- a/public/app/features/explore/QueryRows.tsx +++ b/public/app/features/explore/QueryRows.tsx @@ -67,7 +67,6 @@ export const QueryRows = ({ exploreId }: Props) => { [onChange, queries] ); - // a datasource change on the query row level means the root datasource is mixed const onMixedDataSourceChange = async (ds: DataSourceInstanceSettings, query: DataQuery) => { const queryDatasource = await getDataSourceSrv().get(query.datasource); const targetDS = await getDataSourceSrv().get({ uid: ds.uid }); diff --git a/public/app/features/explore/Wrapper.test.tsx b/public/app/features/explore/Wrapper.test.tsx index 978f80ed2b9..5ae2f7a1ea5 100644 --- a/public/app/features/explore/Wrapper.test.tsx +++ b/public/app/features/explore/Wrapper.test.tsx @@ -8,7 +8,7 @@ import { locationService, config } from '@grafana/runtime'; import { changeDatasource } from './spec/helper/interactions'; import { makeLogsQueryResponse, makeMetricsQueryResponse } from './spec/helper/query'; import { setupExplore, tearDown, waitForExplore } from './spec/helper/setup'; -import * as mainState from './state/main'; +import { splitOpen } from './state/main'; import * as queryState from './state/query'; jest.mock('app/core/core', () => { @@ -154,7 +154,7 @@ describe('Wrapper', () => { }); }); - describe('Handles open/close splits and related events in UI and URL', () => { + describe('Handles open/close splits in UI and URL', () => { it('opens the split pane when split button is clicked', async () => { setupExplore(); // Wait for rendering the editor @@ -218,15 +218,10 @@ describe('Wrapper', () => { it('can close a panel from a split', async () => { const urlParams = { - left: JSON.stringify(['now-1h', 'now', 'loki-uid', { refId: 'A' }]), - right: JSON.stringify(['now-1h', 'now', 'elastic-uid', { refId: 'A' }]), + left: JSON.stringify(['now-1h', 'now', 'loki', { refId: 'A' }]), + right: JSON.stringify(['now-1h', 'now', 'elastic', { refId: 'A' }]), }; - const { datasources } = setupExplore({ urlParams }); - jest.mocked(datasources.loki.query).mockReturnValueOnce(makeLogsQueryResponse()); - jest.mocked(datasources.elastic.query).mockReturnValueOnce(makeLogsQueryResponse()); - - await screen.findByText(/^loki Editor input:$/); - + setupExplore({ urlParams }); const closeButtons = await screen.findAllByLabelText(/Close split pane/i); await userEvent.click(closeButtons[1]); @@ -266,35 +261,12 @@ describe('Wrapper', () => { // to work await screen.findByText(`loki Editor input: { label="value"}`); - store.dispatch(mainState.splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); + store.dispatch(splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); // Editor renders the new query await screen.findByText(`elastic Editor input: error`); await screen.findByText(`loki Editor input: { label="value"}`); }); - - it('handles split size events and sets relevant variables', async () => { - setupExplore(); - const splitButton = await screen.findByText(/split/i); - fireEvent.click(splitButton); - await waitForExplore(undefined, true); - let widenButton = await screen.findAllByLabelText('Widen pane'); - let narrowButton = await screen.queryAllByLabelText('Narrow pane'); - const panes = screen.getAllByRole('main'); - expect(widenButton.length).toBe(2); - expect(narrowButton.length).toBe(0); - expect(Number.parseInt(getComputedStyle(panes[0]).width, 10)).toBe(1000); - expect(Number.parseInt(getComputedStyle(panes[1]).width, 10)).toBe(1000); - const resizer = screen.getByRole('presentation'); - fireEvent.mouseDown(resizer, { buttons: 1 }); - fireEvent.mouseMove(resizer, { clientX: -700, buttons: 1 }); - fireEvent.mouseUp(resizer); - widenButton = await screen.findAllByLabelText('Widen pane'); - narrowButton = await screen.queryAllByLabelText('Narrow pane'); - expect(widenButton.length).toBe(1); - expect(narrowButton.length).toBe(1); - // the autosizer is mocked so there is no actual resize here - }); }); describe('Handles document title changes', () => { @@ -323,7 +295,7 @@ describe('Wrapper', () => { // to work await screen.findByText(`loki Editor input: { label="value"}`); - store.dispatch(mainState.splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); + store.dispatch(splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); await waitFor(() => expect(document.title).toEqual('Explore - loki | elastic - Grafana')); }); }); diff --git a/public/app/features/explore/Wrapper.tsx b/public/app/features/explore/Wrapper.tsx index 85c49964d66..c54644228db 100644 --- a/public/app/features/explore/Wrapper.tsx +++ b/public/app/features/explore/Wrapper.tsx @@ -1,11 +1,8 @@ import { css } from '@emotion/css'; -import { useResizeObserver } from '@react-aria/utils'; -import { debounce, inRange } from 'lodash'; -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useEffect } from 'react'; import { locationService } from '@grafana/runtime'; import { ErrorBoundaryAlert } from '@grafana/ui'; -import { SplitView } from 'app/core/components/SplitPaneWrapper/SplitView'; import { useGrafana } from 'app/core/context/GrafanaContext'; import { useNavModel } from 'app/core/hooks/useNavModel'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; @@ -17,7 +14,7 @@ import { Branding } from '../../core/components/Branding/Branding'; import { ExploreActions } from './ExploreActions'; import { ExplorePaneContainer } from './ExplorePaneContainer'; -import { lastSavedUrl, resetExploreAction, splitSizeUpdateAction } from './state/main'; +import { lastSavedUrl, resetExploreAction } from './state/main'; const styles = { pageScrollbarWrapper: css` @@ -31,14 +28,9 @@ const styles = { `, }; -const MIN_PANE_WIDTH = 200; function Wrapper(props: GrafanaRouteComponentProps<{}, ExploreQueryParams>) { useExplorePageTitle(); - const { maxedExploreId, evenSplitPanes } = useSelector((state) => state.explore); - const [rightPaneWidth, setRightPaneWidth] = useState(); - const [prevWindowWidth, setWindowWidth] = useState(); const dispatch = useDispatch(); - const containerRef = useRef(null); const queryParams = props.queryParams; const { keybindings, chrome } = useGrafana(); const navModel = useNavModel('explore'); @@ -78,81 +70,20 @@ function Wrapper(props: GrafanaRouteComponentProps<{}, ExploreQueryParams>) { // eslint-disable-next-line react-hooks/exhaustive-deps -- dispatch is stable, doesn't need to be in the deps array }, []); - const debouncedFunctionRef = useRef((prevWindowWidth?: number, rightPaneWidth?: number) => { - let rightPaneRatio = 0.5; - if (!containerRef.current) { - return; - } - const windowWidth = containerRef.current.clientWidth; - // get the ratio of the previous rightPane to the window width - if (rightPaneWidth && prevWindowWidth) { - rightPaneRatio = rightPaneWidth / prevWindowWidth; - } - let newRightPaneWidth = Math.floor(windowWidth * rightPaneRatio); - if (newRightPaneWidth < MIN_PANE_WIDTH) { - // if right pane is too narrow, make min width - newRightPaneWidth = MIN_PANE_WIDTH; - } else if (windowWidth - newRightPaneWidth < MIN_PANE_WIDTH) { - // if left pane is too narrow, make right pane = window - minWidth - newRightPaneWidth = windowWidth - MIN_PANE_WIDTH; - } - - setRightPaneWidth(newRightPaneWidth); - setWindowWidth(windowWidth); - }); - - // eslint needs the callback to be inline to analyze the dependencies, but we need to use debounce from lodash - // eslint-disable-next-line react-hooks/exhaustive-deps - const onResize = useCallback( - debounce(() => debouncedFunctionRef.current(prevWindowWidth, rightPaneWidth), 500), - [prevWindowWidth, rightPaneWidth] - ); - - const updateSplitSize = (rightPaneWidth: number) => { - const evenSplitWidth = window.innerWidth / 2; - const areBothSimilar = inRange(rightPaneWidth, evenSplitWidth - 100, evenSplitWidth + 100); - if (areBothSimilar) { - dispatch(splitSizeUpdateAction({ largerExploreId: undefined })); - } else { - dispatch( - splitSizeUpdateAction({ - largerExploreId: rightPaneWidth > evenSplitWidth ? ExploreId.right : ExploreId.left, - }) - ); - } - - setRightPaneWidth(rightPaneWidth); - }; - - useResizeObserver({ onResize, ref: containerRef }); const hasSplit = Boolean(queryParams.left) && Boolean(queryParams.right); - let widthCalc = 0; - if (hasSplit) { - if (!evenSplitPanes && maxedExploreId) { - widthCalc = maxedExploreId === ExploreId.right ? window.innerWidth - MIN_PANE_WIDTH : MIN_PANE_WIDTH; - } else if (evenSplitPanes) { - widthCalc = Math.floor(window.innerWidth / 2); - } else if (rightPaneWidth !== undefined) { - widthCalc = rightPaneWidth; - } - } - - const splitSizeObj = { rightPaneSize: widthCalc }; return ( -
+
- - - + + + + {hasSplit && ( + + - {hasSplit && ( - - - - )} - + )}
); diff --git a/public/app/features/explore/state/main.test.ts b/public/app/features/explore/state/main.test.ts index 1802d78c06f..3a5d5839784 100644 --- a/public/app/features/explore/state/main.test.ts +++ b/public/app/features/explore/state/main.test.ts @@ -139,10 +139,7 @@ describe('Explore reducer', () => { .givenReducer(exploreReducer, initialState) .whenActionIsDispatched(splitCloseAction({ itemId: ExploreId.left })) .thenStateShouldEqual({ - evenSplitPanes: true, - largerExploreId: undefined, left: rightItemMock, - maxedExploreId: undefined, right: undefined, } as unknown as ExploreState); }); @@ -165,10 +162,7 @@ describe('Explore reducer', () => { .givenReducer(exploreReducer, initialState) .whenActionIsDispatched(splitCloseAction({ itemId: ExploreId.right })) .thenStateShouldEqual({ - evenSplitPanes: true, - largerExploreId: undefined, left: leftItemMock, - maxedExploreId: undefined, right: undefined, } as unknown as ExploreState); }); diff --git a/public/app/features/explore/state/main.ts b/public/app/features/explore/state/main.ts index 641bdbcaae7..125b4654e11 100644 --- a/public/app/features/explore/state/main.ts +++ b/public/app/features/explore/state/main.ts @@ -37,16 +37,6 @@ export const richHistorySearchFiltersUpdatedAction = createAction<{ filters?: RichHistorySearchFilters; }>('explore/richHistorySearchFiltersUpdatedAction'); -export const splitSizeUpdateAction = createAction<{ - largerExploreId?: ExploreId; -}>('explore/splitSizeUpdateAction'); - -export const maximizePaneAction = createAction<{ - exploreId?: ExploreId; -}>('explore/maximizePaneAction'); - -export const evenPaneResizeAction = createAction('explore/evenPaneResizeAction'); - /** * Resets state for explore. */ @@ -169,9 +159,6 @@ export const initialExploreState: ExploreState = { richHistoryStorageFull: false, richHistoryLimitExceededWarningShown: false, richHistoryMigrationFailed: false, - largerExploreId: undefined, - maxedExploreId: undefined, - evenSplitPanes: true, }; /** @@ -188,38 +175,6 @@ export const exploreReducer = (state = initialExploreState, action: AnyAction): return { ...state, ...targetSplit, - largerExploreId: undefined, - maxedExploreId: undefined, - evenSplitPanes: true, - }; - } - - if (splitSizeUpdateAction.match(action)) { - const { largerExploreId } = action.payload; - return { - ...state, - largerExploreId, - maxedExploreId: undefined, - evenSplitPanes: largerExploreId === undefined, - }; - } - - if (maximizePaneAction.match(action)) { - const { exploreId } = action.payload; - return { - ...state, - largerExploreId: exploreId, - maxedExploreId: exploreId, - evenSplitPanes: false, - }; - } - - if (evenPaneResizeAction.match(action)) { - return { - ...state, - largerExploreId: undefined, - maxedExploreId: undefined, - evenSplitPanes: true, }; } diff --git a/public/app/features/query/components/QueryEditorRow.tsx b/public/app/features/query/components/QueryEditorRow.tsx index 8ae8c463f22..629338f3e57 100644 --- a/public/app/features/query/components/QueryEditorRow.tsx +++ b/public/app/features/query/components/QueryEditorRow.tsx @@ -84,10 +84,11 @@ export class QueryEditorRow extends PureComponent