From 211c9991c5e0f742fe46298daa6f4e5ab572f72c Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 24 Aug 2022 11:19:36 +0100 Subject: [PATCH] Navigation: Add responsive behaviour to `ToolbarButtonRow` (#53739) * hacky first attempt * slightly cleaner... * behaviour mostly working... * remove unnecessary wrapper * css tweaks * much cleaner implementation with intersectionobserver * set style props directly on children * separate story, integrate when toggle is off * improve story, integrate when toggle is on * remove styles from DashNavTimeControls * mock IntersectionObserver for all unit tests * prettier * don't use dropdown anymore * add some basic documentation * add right alignment to scenes toolbarbuttonrow * just use the react children api to prevent duplicating children --- .betterer.results | 4 +- .../src/components/PageLayout/PageToolbar.tsx | 21 +-- .../ToolbarButton/ToolbarButton.tsx | 5 +- .../ToolbarButton/ToolbarButtonRow.mdx | 25 ++++ .../ToolbarButton/ToolbarButtonRow.story.tsx | 40 +++++ .../ToolbarButton/ToolbarButtonRow.tsx | 137 +++++++++++++++--- .../src/themes/GlobalStyles/page.ts | 1 + .../core/components/AppChrome/NavToolbar.tsx | 1 + .../core/components/NavBar/NavBar.test.tsx | 11 -- .../components/NavBar/NavBarItem.test.tsx | 11 -- .../alerting/unified/RuleEditor.test.tsx | 2 +- .../alerting/unified/RuleViewer.test.tsx | 12 +- .../dashboard/components/DashNav/DashNav.tsx | 12 +- .../DashNav/DashNavTimeControls.tsx | 6 +- .../app/features/explore/ExploreToolbar.tsx | 13 +- .../scenes/components/SceneTimePicker.tsx | 2 +- public/test/jest-setup.ts | 8 + 17 files changed, 225 insertions(+), 86 deletions(-) create mode 100644 packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.mdx create mode 100644 packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.story.tsx diff --git a/.betterer.results b/.betterer.results index c2f23e31bc3..d25b88cfaaa 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1821,9 +1821,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "3"] ], "packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Do not use any type assertions.", "2"] + [0, 0, 0, "Do not use any type assertions.", "0"] ], "packages/grafana-ui/src/components/Tooltip/Tooltip.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], diff --git a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx index 184efde10f1..44c760a6f95 100644 --- a/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx +++ b/packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx @@ -4,7 +4,7 @@ import React, { FC, ReactNode } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { Link } from '..'; +import { Link, ToolbarButtonRow } from '..'; import { useStyles2 } from '../../themes/ThemeContext'; import { getFocusStyles } from '../../themes/mixins'; import { IconName } from '../../types'; @@ -132,15 +132,7 @@ export const PageToolbar: FC = React.memo( )} - {React.Children.toArray(children) - .filter(Boolean) - .map((child, index) => { - return ( -
- {child} -
- ); - })} + {React.Children.toArray(children).filter(Boolean)} ); } @@ -161,14 +153,13 @@ const getStyles = (theme: GrafanaTheme2) => { align-items: center; background: ${theme.colors.background.canvas}; display: flex; - flex-wrap: wrap; - justify-content: flex-end; + gap: ${theme.spacing(2)}; + justify-content: space-between; padding: ${theme.spacing(1.5, 2)}; `, leftWrapper: css` display: flex; flex-wrap: nowrap; - flex-grow: 1; `, pageIcon: css` display: none; @@ -190,7 +181,6 @@ const getStyles = (theme: GrafanaTheme2) => { `, navElement: css` display: flex; - flex-grow: 1; align-items: center; max-width: calc(100vw - 78px); `, @@ -224,9 +214,6 @@ const getStyles = (theme: GrafanaTheme2) => { display: unset; } `, - actionWrapper: css` - padding: ${spacing(0.5, 0, 0.5, 1)}; - `, leftActionItem: css` display: none; ${theme.breakpoints.up('md')} { diff --git a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx index c1fdfd7ddc2..080d4c6041b 100644 --- a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx +++ b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx @@ -62,13 +62,12 @@ export const ToolbarButton = forwardRef( const styles = useStyles2(getStyles); const buttonStyles = cx( - 'toolbar-button', { [styles.button]: true, [styles.buttonFullWidth]: fullWidth, [styles.narrow]: narrow, }, - (styles as any)[variant], + styles[variant], className ); @@ -140,7 +139,7 @@ const getStyles = (theme: GrafanaTheme2) => { const defaultTopNav = css` color: ${theme.colors.text.secondary}; background-color: transparent; - border: none; + border-color: transparent; &:hover { color: ${theme.colors.text.primary}; diff --git a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.mdx b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.mdx new file mode 100644 index 00000000000..0fa31596845 --- /dev/null +++ b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.mdx @@ -0,0 +1,25 @@ +import { Meta, Props } from '@storybook/addon-docs/blocks'; +import { ToolbarButtonRow } from './ToolbarButton'; + +# ToolbarButtonRow + +A container for multiple `ToolbarButton`s. Provides automatic overflow behaviour when the buttons no longer fit in the container. + +## Usage + +This example shows how to use several buttons in a `ToolbarButtonRow`. + +```jsx + + + Last 6 hours + + + + + + + +``` + + diff --git a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.story.tsx b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.story.tsx new file mode 100644 index 00000000000..d91524457a2 --- /dev/null +++ b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.story.tsx @@ -0,0 +1,40 @@ +import { ComponentMeta, ComponentStory } from '@storybook/react'; +import React from 'react'; + +import { DashboardStoryCanvas } from '../../utils/storybook/DashboardStoryCanvas'; +import { withCenteredStory } from '../../utils/storybook/withCenteredStory'; + +import { ToolbarButton } from './ToolbarButton'; +import { ToolbarButtonRow } from './ToolbarButtonRow'; +import mdx from './ToolbarButtonRow.mdx'; + +const meta: ComponentMeta = { + title: 'Buttons/ToolbarButton/ToolbarButtonRow', + component: ToolbarButtonRow, + decorators: [withCenteredStory], + parameters: { + docs: { + page: mdx, + }, + controls: { + exclude: ['className'], + }, + }, +}; + +export const Basic: ComponentStory = (args) => { + return ( + + + Just text + + With imgSrc + Just text + + With imgSrc + + + ); +}; + +export default meta; diff --git a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.tsx b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.tsx index cfc2493a68e..59984a96099 100644 --- a/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.tsx +++ b/packages/grafana-ui/src/components/ToolbarButton/ToolbarButtonRow.tsx @@ -1,37 +1,132 @@ import { css, cx } from '@emotion/css'; -import React, { forwardRef, HTMLAttributes } from 'react'; +import { useDialog } from '@react-aria/dialog'; +import { FocusScope } from '@react-aria/focus'; +import { useOverlay } from '@react-aria/overlays'; +import React, { forwardRef, HTMLAttributes, useState, useRef, useLayoutEffect, createRef } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; -import { useStyles2 } from '../../themes'; +import { useTheme2 } from '../../themes'; +import { ToolbarButton } from './ToolbarButton'; export interface Props extends HTMLAttributes { className?: string; + /** Determine flex-alignment of child buttons. Needed for overflow behaviour. */ + alignment?: 'left' | 'right'; } -export const ToolbarButtonRow = forwardRef(({ className, children, ...rest }, ref) => { - const styles = useStyles2(getStyles); +export const ToolbarButtonRow = forwardRef( + ({ alignment = 'left', className, children, ...rest }, ref) => { + const [childVisibility, setChildVisibility] = useState( + Array(React.Children.toArray(children).length).fill(true) + ); + const containerRef = useRef(null); + const [showOverflowItems, setShowOverflowItems] = useState(false); + const overflowItemsRef = createRef(); + const { overlayProps } = useOverlay( + { onClose: () => setShowOverflowItems(false), isDismissable: true, isOpen: showOverflowItems }, + overflowItemsRef + ); + const { dialogProps } = useDialog({}, overflowItemsRef); + const theme = useTheme2(); + const overflowButtonOrder = alignment === 'left' ? childVisibility.indexOf(false) - 1 : childVisibility.length; + const styles = getStyles(theme, overflowButtonOrder, alignment); - return ( -
- {children} -
- ); -}); + useLayoutEffect(() => { + const intersectionObserver = new IntersectionObserver( + (entries) => { + entries.forEach((entry) => { + if (entry.target instanceof HTMLElement && entry.target.parentNode) { + const index = Array.prototype.indexOf.call(entry.target.parentNode.children, entry.target); + setChildVisibility((prev) => { + const newVisibility = [...prev]; + newVisibility[index] = entry.isIntersecting; + return newVisibility; + }); + } + }); + }, + { + threshold: 1, + root: containerRef.current, + } + ); + if (containerRef.current) { + Array.from(containerRef.current.children).forEach((item) => { + intersectionObserver.observe(item); + }); + } + return () => intersectionObserver.disconnect(); + }, []); + + return ( +
+ {React.Children.map(children, (child, index) => ( +
+ {child} +
+ ))} + {childVisibility.includes(false) && ( + <> + setShowOverflowItems(!showOverflowItems)} + className={styles.overflowButton} + icon="ellipsis-v" + iconOnly + narrow + /> + {showOverflowItems && ( + +
+ {React.Children.toArray(children).map((child, index) => !childVisibility[index] && child)} +
+ + )} + + )} +
+ ); + } +); ToolbarButtonRow.displayName = 'ToolbarButtonRow'; -const getStyles = (theme: GrafanaTheme2) => ({ - wrapper: css` +const getStyles = (theme: GrafanaTheme2, overflowButtonOrder: number, alignment: Props['alignment']) => ({ + overflowButton: css` + order: ${overflowButtonOrder}; + `, + overflowItems: css` + align-items: center; + background-color: ${theme.colors.background.primary}; + border-radius: ${theme.shape.borderRadius()}; + box-shadow: ${theme.shadows.z3}; + display: flex; + flex-wrap: wrap; + gap: ${theme.spacing(1)}; + margin-top: ${theme.spacing(1)}; + max-width: 80vw; + padding: ${theme.spacing(0.5, 1)}; + position: absolute; + right: 0; + top: 100%; + width: max-content; + z-index: ${theme.zIndex.sidemenu}; + `, + container: css` + align-items: center; + display: flex; + gap: ${theme.spacing(1)}; + justify-content: ${alignment === 'left' ? 'flex-start' : 'flex-end'}; + min-width: 0; + position: relative; + `, + childWrapper: css` + align-items: center; display: flex; - - > .button-group, - > .toolbar-button { - margin-left: ${theme.spacing(1)}; - - &:first-child { - margin-left: 0; - } - } `, }); diff --git a/packages/grafana-ui/src/themes/GlobalStyles/page.ts b/packages/grafana-ui/src/themes/GlobalStyles/page.ts index 481d690b233..442e8ba5cfd 100644 --- a/packages/grafana-ui/src/themes/GlobalStyles/page.ts +++ b/packages/grafana-ui/src/themes/GlobalStyles/page.ts @@ -24,6 +24,7 @@ export function getPageStyles(theme: GrafanaTheme2) { flex-grow: 1; height: 100%; flex: 1 1 0; + min-width: 0; } .page-scrollbar-content { diff --git a/public/app/core/components/AppChrome/NavToolbar.tsx b/public/app/core/components/AppChrome/NavToolbar.tsx index 260e4229e07..af5c2367553 100644 --- a/public/app/core/components/AppChrome/NavToolbar.tsx +++ b/public/app/core/components/AppChrome/NavToolbar.tsx @@ -68,6 +68,7 @@ const getStyles = (theme: GrafanaTheme2) => { paddingLeft: theme.spacing(1), flexGrow: 1, gap: theme.spacing(0.5), + minWidth: 0, }), }; }; diff --git a/public/app/core/components/NavBar/NavBar.test.tsx b/public/app/core/components/NavBar/NavBar.test.tsx index 066c4c3f652..39e2949a651 100644 --- a/public/app/core/components/NavBar/NavBar.test.tsx +++ b/public/app/core/components/NavBar/NavBar.test.tsx @@ -36,17 +36,6 @@ const setup = () => { }; describe('Render', () => { - beforeEach(() => { - // IntersectionObserver isn't available in test environment - const mockIntersectionObserver = jest.fn(); - mockIntersectionObserver.mockReturnValue({ - observe: () => null, - unobserve: () => null, - disconnect: () => null, - }); - window.IntersectionObserver = mockIntersectionObserver; - }); - it('should render component', async () => { setup(); const sidemenu = await screen.findByTestId('sidemenu'); diff --git a/public/app/core/components/NavBar/NavBarItem.test.tsx b/public/app/core/components/NavBar/NavBarItem.test.tsx index 77ccbf9400b..86b0eca5fae 100644 --- a/public/app/core/components/NavBar/NavBarItem.test.tsx +++ b/public/app/core/components/NavBar/NavBarItem.test.tsx @@ -66,17 +66,6 @@ async function getTestContext(overrides: Partial = {}, subUrl = '', isMen } describe('NavBarItem', () => { - beforeEach(() => { - // IntersectionObserver isn't available in test environment - const mockIntersectionObserver = jest.fn(); - mockIntersectionObserver.mockReturnValue({ - observe: () => null, - unobserve: () => null, - disconnect: () => null, - }); - window.IntersectionObserver = mockIntersectionObserver; - }); - describe('when url property is not set', () => { it('then it renders the menu trigger as a button', async () => { await getTestContext(); diff --git a/public/app/features/alerting/unified/RuleEditor.test.tsx b/public/app/features/alerting/unified/RuleEditor.test.tsx index 8f712516386..c986a3df1cc 100644 --- a/public/app/features/alerting/unified/RuleEditor.test.tsx +++ b/public/app/features/alerting/unified/RuleEditor.test.tsx @@ -99,7 +99,7 @@ const ui = { describe('RuleEditor', () => { beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); contextSrv.isEditor = true; contextSrv.hasEditPermissionInFolders = true; }); diff --git a/public/app/features/alerting/unified/RuleViewer.test.tsx b/public/app/features/alerting/unified/RuleViewer.test.tsx index 225b4ce02ca..126a857e00f 100644 --- a/public/app/features/alerting/unified/RuleViewer.test.tsx +++ b/public/app/features/alerting/unified/RuleViewer.test.tsx @@ -41,12 +41,18 @@ const renderRuleViewer = () => { }); }; describe('RuleViewer', () => { + let mockCombinedRule: jest.MockedFn; + + beforeEach(() => { + mockCombinedRule = jest.mocked(useCombinedRule); + }); + afterEach(() => { - jest.resetAllMocks(); + mockCombinedRule.mockReset(); }); it('should render page with grafana alert', async () => { - jest.mocked(useCombinedRule).mockReturnValue({ + mockCombinedRule.mockReturnValue({ result: mockGrafanaRule as CombinedRule, loading: false, dispatched: true, @@ -60,7 +66,7 @@ describe('RuleViewer', () => { }); it('should render page with cloud alert', async () => { - jest.mocked(useCombinedRule).mockReturnValue({ + mockCombinedRule.mockReturnValue({ result: mockCloudRule as CombinedRule, loading: false, dispatched: true, diff --git a/public/app/features/dashboard/components/DashNav/DashNav.tsx b/public/app/features/dashboard/components/DashNav/DashNav.tsx index 2a1154e1a32..845b9c89aee 100644 --- a/public/app/features/dashboard/components/DashNav/DashNav.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNav.tsx @@ -6,7 +6,15 @@ import { useLocation } from 'react-router-dom'; import { locationUtil, textUtil } from '@grafana/data'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors/src'; import { locationService } from '@grafana/runtime'; -import { ButtonGroup, ModalsController, ToolbarButton, PageToolbar, useForceUpdate, Tag } from '@grafana/ui'; +import { + ButtonGroup, + ModalsController, + ToolbarButton, + PageToolbar, + useForceUpdate, + Tag, + ToolbarButtonRow, +} from '@grafana/ui'; import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate'; import { NavToolbarSeparator } from 'app/core/components/AppChrome/NavToolbarSeparator'; import config from 'app/core/config'; @@ -322,7 +330,7 @@ export const DashNav = React.memo((props) => { <> {renderLeftActions()} - {renderRightActions()} + {renderRightActions()} } /> diff --git a/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx b/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx index b2570002848..4a3a63d718f 100644 --- a/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx @@ -4,7 +4,7 @@ import { Unsubscribable } from 'rxjs'; import { dateMath, TimeRange, TimeZone } from '@grafana/data'; import { TimeRangeUpdatedEvent } from '@grafana/runtime'; -import { defaultIntervals, RefreshPicker, ToolbarButtonRow } from '@grafana/ui'; +import { defaultIntervals, RefreshPicker } from '@grafana/ui'; import { TimePickerWithHistory } from 'app/core/components/TimePicker/TimePickerWithHistory'; import { appEvents } from 'app/core/core'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; @@ -87,7 +87,7 @@ export class DashNavTimeControls extends Component { const hideIntervalPicker = dashboard.panelInEdit?.isEditing; return ( - + <> { offOptionLabelMsg={t({ id: 'dashboard.refresh-picker.off-label', message: 'Off' })} offOptionAriaLabelMsg={t({ id: 'dashboard.refresh-picker.off-arialabel', message: 'Turn off auto refresh' })} /> - + ); } } diff --git a/public/app/features/explore/ExploreToolbar.tsx b/public/app/features/explore/ExploreToolbar.tsx index be9eea29b9d..e35cd7b66b8 100644 --- a/public/app/features/explore/ExploreToolbar.tsx +++ b/public/app/features/explore/ExploreToolbar.tsx @@ -3,14 +3,7 @@ import { connect, ConnectedProps } from 'react-redux'; import { DataSourceInstanceSettings, RawTimeRange } from '@grafana/data'; import { config, DataSourcePicker, reportInteraction } from '@grafana/runtime'; -import { - defaultIntervals, - PageToolbar, - RefreshPicker, - SetInterval, - ToolbarButton, - ToolbarButtonRow, -} from '@grafana/ui'; +import { defaultIntervals, PageToolbar, RefreshPicker, SetInterval, ToolbarButton } from '@grafana/ui'; import { contextSrv } from 'app/core/core'; import { createAndCopyShortLink } from 'app/core/utils/shortLinks'; import { AccessControlAction } from 'app/types'; @@ -153,7 +146,7 @@ class UnConnectedExploreToolbar extends PureComponent { ), ].filter(Boolean)} > - + <> {!splitted ? ( split()} icon="columns" disabled={isLive}> Split @@ -216,7 +209,7 @@ class UnConnectedExploreToolbar extends PureComponent { }} )} - + ); diff --git a/public/app/features/scenes/components/SceneTimePicker.tsx b/public/app/features/scenes/components/SceneTimePicker.tsx index 7d2c15e9630..09640d778c7 100644 --- a/public/app/features/scenes/components/SceneTimePicker.tsx +++ b/public/app/features/scenes/components/SceneTimePicker.tsx @@ -24,7 +24,7 @@ function SceneTimePickerRenderer({ model }: SceneComponentProps } return ( - + ({ ...jest.requireActual('../app/core/core'), appEvents: testAppEvents,