From c3e4f1f8766fc2d9d788652daf5b3a8bf1ea8951 Mon Sep 17 00:00:00 2001 From: Kristina Date: Fri, 23 Sep 2022 08:20:33 -0500 Subject: [PATCH] Explore: Add resize to split view, with Min/Max button (#54420) * Add split resize to Explore without keeping width in state * debug commit * ugly hack around split lib only supporting one child * Use SplitView to accomodate one or two elements, remove debug code, fix test * More cleanup, fix state action * Fix even split from manual size scenario * cleanup * Add new state elements to test * Handle scrollable on internal element for virtualized lists * Left align overflow button for explore * Change min/max buttons * Apply suggestions from code review Co-authored-by: Giordano Ricci * Add more suggestions from the code review * Fix problems tests found * commit broken test with debug info * Add test, remove debug code * Remove second get of panes * Remove second get of panes Co-authored-by: Elfo404 --- .../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 | 79 ++++++++++++++-- .../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, 312 insertions(+), 44 deletions(-) create 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 28fe0b9a307..38e6b93e340 100644 --- a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx +++ b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx @@ -24,6 +24,7 @@ export interface Props { className?: string; isFullscreen?: boolean; 'aria-label'?: string; + buttonOverflowAlignment?: 'left' | 'right'; } /** @alpha */ @@ -42,6 +43,7 @@ export const PageToolbar: FC = React.memo( className, /** main nav-container aria-label **/ 'aria-label': ariaLabel, + buttonOverflowAlignment = 'right', }) => { const styles = useStyles2(getStyles); @@ -132,7 +134,9 @@ 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 8b06c108c50..a84d543b627 100644 --- a/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx +++ b/public/app/core/components/SplitPaneWrapper/SplitPaneWrapper.tsx @@ -6,6 +6,8 @@ import { GrafanaTheme } from '@grafana/data'; import { stylesFactory } from '@grafana/ui'; import { config } from 'app/core/config'; +import { SplitView } from './SplitView'; + enum Pane { Right, Top, @@ -100,8 +102,6 @@ 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,18 +111,10 @@ 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 new file mode 100644 index 00000000000..cca03f04eea --- /dev/null +++ b/public/app/core/components/SplitPaneWrapper/SplitView.tsx @@ -0,0 +1,93 @@ +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 028d1a5d387..49d12f76d57 100644 --- a/public/app/features/explore/ExplorePaneContainer.tsx +++ b/public/app/features/explore/ExplorePaneContainer.tsx @@ -1,4 +1,4 @@ -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import memoizeOne from 'memoize-one'; import React from 'react'; import { connect, ConnectedProps } from 'react-redux'; @@ -36,13 +36,12 @@ 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%; - `, }; }; @@ -123,7 +122,6 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { componentWillUnmount() { this.exploreEvents.removeAllListeners(); - this.props.cleanupPaneAction({ exploreId: this.props.exploreId }); } componentDidUpdate(prevProps: Props) { @@ -144,11 +142,10 @@ class ExplorePaneContainerUnconnected extends React.PureComponent { }; render() { - const { theme, split, exploreId, initialized } = this.props; + const { theme, 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 dfaced53e30..59b666ebb27 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 { splitClose, splitOpen } from './state/main'; +import { evenPaneResizeAction, maximizePaneAction, splitClose, splitOpen } from './state/main'; import { cancelQueries, runQueries } from './state/query'; import { isSplit } from './state/selectors'; import { syncTimes, changeRefreshInterval } from './state/time'; @@ -126,6 +126,7 @@ class UnConnectedExploreToolbar extends PureComponent { onChangeTimeZone, onChangeFiscalYearStartMonth, topOfViewRef, + largerExploreId, } = this.props; const showSmallDataSourcePicker = (splitted ? containerWidth < 700 : containerWidth < 800) || false; @@ -135,12 +136,23 @@ 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 && ( @@ -234,7 +258,7 @@ class UnConnectedExploreToolbar extends PureComponent { } const mapStateToProps = (state: StoreState, { exploreId }: OwnProps) => { - const { syncedTimes } = state.explore; + const { syncedTimes, largerExploreId } = state.explore; const exploreItem = state.explore[exploreId]!; const { datasourceInstance, datasourceMissing, range, refreshInterval, loading, isLive, isPaused, containerWidth } = exploreItem; @@ -256,6 +280,7 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps) => { isPaused, syncedTimes, containerWidth, + largerExploreId, }; }; @@ -269,6 +294,8 @@ 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 a1b7b0ce782..fa51d05e9a8 100644 --- a/public/app/features/explore/QueryRows.tsx +++ b/public/app/features/explore/QueryRows.tsx @@ -67,6 +67,7 @@ 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 5ae2f7a1ea5..978f80ed2b9 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 { splitOpen } from './state/main'; +import * as mainState 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 in UI and URL', () => { + describe('Handles open/close splits and related events in UI and URL', () => { it('opens the split pane when split button is clicked', async () => { setupExplore(); // Wait for rendering the editor @@ -218,10 +218,15 @@ describe('Wrapper', () => { it('can close a panel from a split', async () => { const urlParams = { - left: JSON.stringify(['now-1h', 'now', 'loki', { refId: 'A' }]), - right: JSON.stringify(['now-1h', 'now', 'elastic', { refId: 'A' }]), + left: JSON.stringify(['now-1h', 'now', 'loki-uid', { refId: 'A' }]), + right: JSON.stringify(['now-1h', 'now', 'elastic-uid', { refId: 'A' }]), }; - setupExplore({ urlParams }); + const { datasources } = setupExplore({ urlParams }); + jest.mocked(datasources.loki.query).mockReturnValueOnce(makeLogsQueryResponse()); + jest.mocked(datasources.elastic.query).mockReturnValueOnce(makeLogsQueryResponse()); + + await screen.findByText(/^loki Editor input:$/); + const closeButtons = await screen.findAllByLabelText(/Close split pane/i); await userEvent.click(closeButtons[1]); @@ -261,12 +266,35 @@ describe('Wrapper', () => { // to work await screen.findByText(`loki Editor input: { label="value"}`); - store.dispatch(splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); + store.dispatch(mainState.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', () => { @@ -295,7 +323,7 @@ describe('Wrapper', () => { // to work await screen.findByText(`loki Editor input: { label="value"}`); - store.dispatch(splitOpen({ datasourceUid: 'elastic', query: { expr: 'error' } }) as any); + store.dispatch(mainState.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 1a1529b2159..412d62123e5 100644 --- a/public/app/features/explore/Wrapper.tsx +++ b/public/app/features/explore/Wrapper.tsx @@ -1,9 +1,11 @@ import { css } from '@emotion/css'; +import { 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'; @@ -14,7 +16,13 @@ import { getNavModel } from '../../core/selectors/navModel'; import { ExploreActions } from './ExploreActions'; import { ExplorePaneContainer } from './ExplorePaneContainer'; -import { lastSavedUrl, resetExploreAction, richHistoryUpdatedAction } from './state/main'; +import { + lastSavedUrl, + resetExploreAction, + richHistoryUpdatedAction, + cleanupPaneAction, + splitSizeUpdateAction, +} from './state/main'; const styles = { pageScrollbarWrapper: css` @@ -31,6 +39,10 @@ const styles = { interface RouteProps extends GrafanaRouteComponentProps<{}, ExploreQueryParams> {} interface OwnProps {} +interface WrapperState { + rightPaneWidth?: number; +} + const mapStateToProps = (state: StoreState) => { return { navModel: getNavModel(state.navIndex, 'explore'), @@ -41,16 +53,35 @@ const mapStateToProps = (state: StoreState) => { const mapDispatchToProps = { resetExploreAction, richHistoryUpdatedAction, + cleanupPaneAction, + splitSizeUpdateAction, }; const connector = connect(mapStateToProps, mapDispatchToProps); type Props = OwnProps & RouteProps & ConnectedProps; -class WrapperUnconnected extends PureComponent { +class WrapperUnconnected extends PureComponent { + minWidth = 200; static contextType = GrafanaContext; + constructor(props: Props) { + super(props); + this.state = { + rightPaneWidth: 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 }); + } } componentDidMount() { @@ -87,22 +118,52 @@ class WrapperUnconnected extends PureComponent { document.title = documentTitle; } + 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({ 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 3a5d5839784..1802d78c06f 100644 --- a/public/app/features/explore/state/main.test.ts +++ b/public/app/features/explore/state/main.test.ts @@ -139,7 +139,10 @@ 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); }); @@ -162,7 +165,10 @@ 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 1a61b49e714..6e74afdc048 100644 --- a/public/app/features/explore/state/main.ts +++ b/public/app/features/explore/state/main.ts @@ -37,6 +37,16 @@ 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. */ @@ -173,6 +183,9 @@ export const initialExploreState: ExploreState = { richHistoryStorageFull: false, richHistoryLimitExceededWarningShown: false, richHistoryMigrationFailed: false, + largerExploreId: undefined, + maxedExploreId: undefined, + evenSplitPanes: true, }; /** @@ -189,6 +202,38 @@ 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 8034777cd52..317986803ac 100644 --- a/public/app/features/query/components/QueryEditorRow.tsx +++ b/public/app/features/query/components/QueryEditorRow.tsx @@ -84,11 +84,10 @@ export class QueryEditorRow extends PureComponent