From a425170bff19ca6ad430ba37f4a2fe22df167fc6 Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Tue, 4 Oct 2022 16:08:49 +0100 Subject: [PATCH] Backport 56310 to v9.2.x (#56311) * 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 | 13 ++- .../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 | 104 ++---------------- .../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(+), 337 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 49d12f76d57..028d1a5d387 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%; + `, }; }; @@ -122,6 +123,7 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { componentWillUnmount() { this.exploreEvents.removeAllListeners(); + this.props.cleanupPaneAction({ exploreId: this.props.exploreId }); } componentDidUpdate(prevProps: Props) { @@ -142,10 +144,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 9a33de82a4d..1a1529b2159 100644 --- a/public/app/features/explore/Wrapper.tsx +++ b/public/app/features/explore/Wrapper.tsx @@ -1,11 +1,9 @@ import { css } from '@emotion/css'; -import { debounce, inRange } from 'lodash'; import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { locationService } from '@grafana/runtime'; import { ErrorBoundaryAlert } from '@grafana/ui'; -import { SplitView } from 'app/core/components/SplitPaneWrapper/SplitView'; import { GrafanaContext } from 'app/core/context/GrafanaContext'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; import { StoreState } from 'app/types'; @@ -16,13 +14,7 @@ import { getNavModel } from '../../core/selectors/navModel'; import { ExploreActions } from './ExploreActions'; import { ExplorePaneContainer } from './ExplorePaneContainer'; -import { - lastSavedUrl, - resetExploreAction, - richHistoryUpdatedAction, - cleanupPaneAction, - splitSizeUpdateAction, -} from './state/main'; +import { lastSavedUrl, resetExploreAction, richHistoryUpdatedAction } from './state/main'; const styles = { pageScrollbarWrapper: css` @@ -39,11 +31,6 @@ const styles = { interface RouteProps extends GrafanaRouteComponentProps<{}, ExploreQueryParams> {} interface OwnProps {} -interface WrapperState { - rightPaneWidth?: number; - windowWidth?: number; -} - const mapStateToProps = (state: StoreState) => { return { navModel: getNavModel(state.navIndex, 'explore'), @@ -54,38 +41,16 @@ const mapStateToProps = (state: StoreState) => { const mapDispatchToProps = { resetExploreAction, richHistoryUpdatedAction, - cleanupPaneAction, - splitSizeUpdateAction, }; const connector = connect(mapStateToProps, mapDispatchToProps); type Props = OwnProps & RouteProps & ConnectedProps; -class WrapperUnconnected extends PureComponent { - minWidth = 200; +class WrapperUnconnected extends PureComponent { static contextType = GrafanaContext; - constructor(props: Props) { - super(props); - this.state = { - rightPaneWidth: undefined, - windowWidth: undefined, - }; - } - componentWillUnmount() { - const { left, right } = this.props.queryParams; this.props.resetExploreAction({}); - - if (Boolean(left)) { - this.props.cleanupPaneAction({ exploreId: ExploreId.left }); - } - - if (Boolean(right)) { - this.props.cleanupPaneAction({ exploreId: ExploreId.right }); - } - - window.removeEventListener('resize', this.windowResizeListener); } componentDidMount() { @@ -110,8 +75,6 @@ class WrapperUnconnected extends PureComponent { if (searchParams.from || searchParams.to) { locationService.partial({ from: undefined, to: undefined }, true); } - - window.addEventListener('resize', this.windowResizeListener); } componentDidUpdate() { @@ -124,71 +87,22 @@ class WrapperUnconnected extends PureComponent { document.title = documentTitle; } - windowResizeListener = debounce(() => { - let rightPaneRatio = 0.5; - const windowWidth = window.innerWidth; - // get the ratio of the previous rightPane to the window width - if (this.state.rightPaneWidth && this.state.windowWidth) { - rightPaneRatio = this.state.rightPaneWidth / this.state.windowWidth; - } - let newRightPaneWidth = Math.floor(windowWidth * rightPaneRatio); - if (newRightPaneWidth < this.minWidth) { - // if right pane is too narrow, make min width - newRightPaneWidth = this.minWidth; - } else if (windowWidth - newRightPaneWidth < this.minWidth) { - // if left pane is too narrow, make right pane = window - minWidth - newRightPaneWidth = windowWidth - this.minWidth; - } - - this.setState({ windowWidth, rightPaneWidth: newRightPaneWidth }); - }, 500); - - updateSplitSize = (rightPaneWidth: number) => { - const evenSplitWidth = window.innerWidth / 2; - const areBothSimilar = inRange(rightPaneWidth, evenSplitWidth - 100, evenSplitWidth + 100); - if (areBothSimilar) { - this.props.splitSizeUpdateAction({ largerExploreId: undefined }); - } else { - this.props.splitSizeUpdateAction({ - largerExploreId: rightPaneWidth > evenSplitWidth ? ExploreId.right : ExploreId.left, - }); - } - - this.setState({ ...this.state, rightPaneWidth }); - }; - render() { const { left, right } = this.props.queryParams; - const { maxedExploreId, evenSplitPanes } = this.props.exploreState; const hasSplit = Boolean(left) && Boolean(right); - let widthCalc = 0; - - if (hasSplit) { - if (!evenSplitPanes && maxedExploreId) { - widthCalc = maxedExploreId === ExploreId.right ? window.innerWidth - this.minWidth : this.minWidth; - } else if (evenSplitPanes) { - widthCalc = Math.floor(window.innerWidth / 2); - } else if (this.state.rightPaneWidth !== undefined) { - widthCalc = this.state.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 6e74afdc048..1a61b49e714 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. */ @@ -183,9 +173,6 @@ export const initialExploreState: ExploreState = { richHistoryStorageFull: false, richHistoryLimitExceededWarningShown: false, richHistoryMigrationFailed: false, - largerExploreId: undefined, - maxedExploreId: undefined, - evenSplitPanes: true, }; /** @@ -202,38 +189,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