From 712564f66a5bcbf1bc2edec3393c20aeed1c4a9b Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Thu, 9 Apr 2020 21:23:22 +0200 Subject: [PATCH] NewPanelEdit: Add unified UI to queries and transformations (#23478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Do not use pointer cursor on icon by default * Allow items alignment in the HorizontalGroup layout * Add util for rendering components based on their type (element or function) * Components for rendering query and transformation rows in a unified way * Apply new UI fo query and transformation rows * Add some tests * Minor fix for scroll area Co-authored-by: Torkel Ödegaard --- .../grafana-ui/src/components/Icon/Icon.tsx | 1 - .../src/components/Layout/Layout.tsx | 4 +- packages/grafana-ui/src/utils/index.ts | 2 + .../src/utils/renderOrCallToRender.ts | 22 +++ .../QueryOperationAction.test.tsx | 23 +++ .../QueryOperationAction.tsx | 45 ++++++ .../QueryOperationRow.test.tsx | 136 ++++++++++++++++ .../QueryOperationRow/QueryOperationRow.tsx | 115 ++++++++++++++ .../TransformationEditor.tsx | 132 ++++++++++++++++ .../TransformationOperationRow.tsx | 44 ++++++ .../TransformationRow.tsx | 83 ---------- .../TransformationsEditor.tsx | 41 +++-- .../dashboard/panel_editor/QueryEditorRow.tsx | 149 ++++++++++-------- .../panel_editor/QueryEditorRowTitle.tsx | 72 +++++++++ public/sass/components/_query_editor.scss | 46 ------ 15 files changed, 705 insertions(+), 210 deletions(-) create mode 100644 packages/grafana-ui/src/utils/renderOrCallToRender.ts create mode 100644 public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx create mode 100644 public/app/core/components/QueryOperationRow/QueryOperationAction.tsx create mode 100644 public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx create mode 100644 public/app/core/components/QueryOperationRow/QueryOperationRow.tsx create mode 100644 public/app/features/dashboard/components/TransformationsEditor/TransformationEditor.tsx create mode 100644 public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx delete mode 100644 public/app/features/dashboard/components/TransformationsEditor/TransformationRow.tsx create mode 100644 public/app/features/dashboard/panel_editor/QueryEditorRowTitle.tsx diff --git a/packages/grafana-ui/src/components/Icon/Icon.tsx b/packages/grafana-ui/src/components/Icon/Icon.tsx index 58b4083208a..8349be35327 100644 --- a/packages/grafana-ui/src/components/Icon/Icon.tsx +++ b/packages/grafana-ui/src/components/Icon/Icon.tsx @@ -24,7 +24,6 @@ const getIconStyles = stylesFactory((theme: GrafanaTheme) => { vertical-align: middle; display: inline-block; margin-bottom: ${theme.spacing.xxs}; - cursor: pointer; fill: currentColor; `, orange: css` diff --git a/packages/grafana-ui/src/components/Layout/Layout.tsx b/packages/grafana-ui/src/components/Layout/Layout.tsx index 0d050d8e6bc..e6b8d89eea1 100644 --- a/packages/grafana-ui/src/components/Layout/Layout.tsx +++ b/packages/grafana-ui/src/components/Layout/Layout.tsx @@ -42,8 +42,8 @@ export const Layout: React.FC = ({ ); }; -export const HorizontalGroup: React.FC> = ({ children, spacing, justify }) => ( - +export const HorizontalGroup: React.FC> = ({ children, spacing, justify, align }) => ( + {children} ); diff --git a/packages/grafana-ui/src/utils/index.ts b/packages/grafana-ui/src/utils/index.ts index 659ae52851b..f6750009a53 100644 --- a/packages/grafana-ui/src/utils/index.ts +++ b/packages/grafana-ui/src/utils/index.ts @@ -11,3 +11,5 @@ export { DOMUtil }; // Exposes standard editors for registries of optionsUi config and panel options UI export { getStandardFieldConfigs, getStandardOptionEditors } from './standardEditors'; + +export { renderOrCallToRender } from './renderOrCallToRender'; diff --git a/packages/grafana-ui/src/utils/renderOrCallToRender.ts b/packages/grafana-ui/src/utils/renderOrCallToRender.ts new file mode 100644 index 00000000000..b99d97dfda6 --- /dev/null +++ b/packages/grafana-ui/src/utils/renderOrCallToRender.ts @@ -0,0 +1,22 @@ +import React from 'react'; + +/** + * Given react node or function returns element accordingly + * + * @param itemToRender + * @param props props to be passed to the function if item provided as such + */ +export function renderOrCallToRender( + itemToRender: ((props?: TProps) => React.ReactNode) | React.ReactNode, + props?: TProps +): React.ReactNode { + if (React.isValidElement(itemToRender) || typeof itemToRender === 'string' || typeof itemToRender === 'number') { + return itemToRender; + } + + if (typeof itemToRender === 'function') { + return itemToRender(props); + } + + throw new Error(`${itemToRender} is not a React element nor a function that returns React element`); +} diff --git a/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx b/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx new file mode 100644 index 00000000000..81edf6201ab --- /dev/null +++ b/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx @@ -0,0 +1,23 @@ +import React from 'react'; +import { QueryOperationAction } from './QueryOperationAction'; +import { shallow } from 'enzyme'; + +describe('QueryOperationAction', () => { + it('renders', () => { + expect(() => shallow( {}} />)).not.toThrow(); + }); + describe('when disabled', () => { + it('does not call onClick handler', () => { + const clickSpy = jest.fn(); + const wrapper = shallow(); + const actionEl = wrapper.find({ 'aria-label': 'Test action query operation action' }); + + expect(actionEl).toHaveLength(1); + expect(clickSpy).not.toBeCalled(); + + actionEl.first().simulate('click'); + + expect(clickSpy).toBeCalledTimes(1); + }); + }); +}); diff --git a/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx b/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx new file mode 100644 index 00000000000..6767503734e --- /dev/null +++ b/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx @@ -0,0 +1,45 @@ +import { Icon, IconName, stylesFactory, useTheme } from '@grafana/ui'; +import React from 'react'; +import { css, cx } from 'emotion'; +import { GrafanaTheme } from '@grafana/data'; + +interface QueryOperationActionProps { + icon: IconName; + title?: string; + onClick: (e: React.MouseEvent) => void; + disabled?: boolean; +} + +export const QueryOperationAction: React.FC = ({ icon, disabled, title, ...otherProps }) => { + const theme = useTheme(); + const styles = getQueryOperationActionStyles(theme, !!disabled); + const onClick = (e: React.MouseEvent) => { + if (!disabled) { + otherProps.onClick(e); + } + }; + return ( +
+ +
+ ); +}; + +const getQueryOperationActionStyles = stylesFactory((theme: GrafanaTheme, disabled: boolean) => { + return { + icon: cx( + !disabled && + css` + cursor: pointer; + color: ${theme.colors.textWeak}; + `, + disabled && + css` + color: ${theme.colors.gray25}; + cursor: disabled; + ` + ), + }; +}); + +QueryOperationAction.displayName = 'QueryOperationAction'; diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx new file mode 100644 index 00000000000..439e3e82b1c --- /dev/null +++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx @@ -0,0 +1,136 @@ +import React from 'react'; +import { QueryOperationRow } from './QueryOperationRow'; +import { shallow, mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; + +describe('QueryOperationRow', () => { + it('renders', () => { + expect(() => + shallow( + +
Test
+ + ) + ).not.toThrow(); + }); + + describe('callbacks', () => { + it('should not call onOpen when component is shallowed', async () => { + const onOpenSpy = jest.fn(); + await act(async () => { + shallow( + +
Test
+ + ); + }); + expect(onOpenSpy).not.toBeCalled(); + }); + + it('should call onOpen when row is opened and onClose when row is collapsed', async () => { + const onOpenSpy = jest.fn(); + const onCloseSpy = jest.fn(); + const wrapper = mount( + +
Test
+ + ); + const titleEl = wrapper.find({ 'aria-label': 'Query operation row title' }); + expect(titleEl).toHaveLength(1); + + await act(async () => { + // open + titleEl.first().simulate('click'); + }); + await act(async () => { + // close + titleEl.first().simulate('click'); + }); + + expect(onOpenSpy).toBeCalledTimes(1); + expect(onCloseSpy).toBeCalledTimes(1); + }); + }); + + describe('title rendering', () => { + it('should render title provided as element', () => { + const title =
Test
; + const wrapper = shallow( + +
Test
+ + ); + + const titleEl = wrapper.find({ 'aria-label': 'test title' }); + expect(titleEl).toHaveLength(1); + }); + it('should render title provided as function', () => { + const title = () =>
Test
; + const wrapper = shallow( + +
Test
+ + ); + + const titleEl = wrapper.find({ 'aria-label': 'test title' }); + expect(titleEl).toHaveLength(1); + }); + + it('should expose api to title rendered as function', () => { + const propsSpy = jest.fn(); + const title = (props: any) => { + propsSpy(props); + return
Test
; + }; + shallow( + +
Test
+ + ); + + expect(Object.keys(propsSpy.mock.calls[0][0])).toContain('isOpen'); + }); + }); + + describe('actions rendering', () => { + it('should render actions provided as element', () => { + const actions =
Test
; + const wrapper = shallow( + +
Test
+ + ); + + const actionsEl = wrapper.find({ 'aria-label': 'test actions' }); + expect(actionsEl).toHaveLength(1); + }); + it('should render actions provided as function', () => { + const actions = () =>
Test
; + const wrapper = shallow( + +
Test
+ + ); + + const actionsEl = wrapper.find({ 'aria-label': 'test actions' }); + expect(actionsEl).toHaveLength(1); + }); + + it('should expose api to title rendered as function', () => { + const propsSpy = jest.fn(); + const actions = (props: any) => { + propsSpy(props); + return
Test
; + }; + shallow( + +
Test
+ + ); + + expect(Object.keys(propsSpy.mock.calls[0][0])).toContainEqual('isOpen'); + expect(Object.keys(propsSpy.mock.calls[0][0])).toContainEqual('openRow'); + expect(Object.keys(propsSpy.mock.calls[0][0])).toContainEqual('closeRow'); + }); + }); +}); diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx new file mode 100644 index 00000000000..600c86402e1 --- /dev/null +++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx @@ -0,0 +1,115 @@ +import React, { useState } from 'react'; +import { renderOrCallToRender, HorizontalGroup, Icon, stylesFactory, useTheme } from '@grafana/ui'; +import { GrafanaTheme } from '@grafana/data'; +import { css } from 'emotion'; +import { useUpdateEffect } from 'react-use'; + +interface QueryOperationRowProps { + title?: ((props: { isOpen: boolean }) => React.ReactNode) | React.ReactNode; + actions?: + | ((props: { isOpen: boolean; openRow: () => void; closeRow: () => void }) => React.ReactNode) + | React.ReactNode; + onOpen?: () => void; + onClose?: () => void; + children: React.ReactNode; + isOpen?: boolean; +} + +export const QueryOperationRow: React.FC = ({ + children, + actions, + title, + onClose, + onOpen, + isOpen, +}: QueryOperationRowProps) => { + const [isContentVisible, setIsContentVisible] = useState(isOpen !== undefined ? isOpen : true); + const theme = useTheme(); + const styles = getQueryOperationRowStyles(theme); + useUpdateEffect(() => { + if (isContentVisible) { + if (onOpen) { + onOpen(); + } + } else { + if (onClose) { + onClose(); + } + } + }, [isContentVisible]); + + const titleElement = title && renderOrCallToRender(title, { isOpen: isContentVisible }); + const actionsElement = + actions && + renderOrCallToRender(actions, { + isOpen: isContentVisible, + openRow: () => { + setIsContentVisible(true); + }, + closeRow: () => { + setIsContentVisible(false); + }, + }); + + return ( +
+
+ +
{ + setIsContentVisible(!isContentVisible); + }} + aria-label="Query operation row title" + > + + {title && {titleElement}} +
+ {actions &&
{actionsElement}
} + +
+ {isContentVisible &&
{children}
} +
+ ); +}; + +const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => { + const borderColor = theme.isLight ? theme.colors.gray85 : theme.colors.gray25; + + return { + wrapper: css` + margin-bottom: ${theme.spacing.formSpacingBase * 2}px; + `, + header: css` + padding: ${theme.spacing.sm}; + border-radius: ${theme.border.radius.sm}; + border: 1px solid ${borderColor}; + background: ${theme.colors.pageBg}; + `, + collapseIcon: css` + color: ${theme.colors.textWeak}; + `, + titleWrapper: css` + display: flex; + align-items: center; + cursor: pointer; + `, + + title: css` + font-weight: ${theme.typography.weight.semibold}; + color: ${theme.colors.blue95}; + margin-left: ${theme.spacing.sm}; + `, + content: css` + border: 1px solid ${borderColor}; + margin-top: -1px; + background: ${theme.colors.pageBg}; + margin-left: ${theme.spacing.xl}; + border-top: 1px solid ${theme.colors.pageBg}; + border-radis: 0 ${theme.border.radius.sm}; + padding: 0 ${theme.spacing.sm} ${theme.spacing.sm} ${theme.spacing.lg}; + `, + }; +}); + +QueryOperationRow.displayName = 'QueryOperationRow'; diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationEditor.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationEditor.tsx new file mode 100644 index 00000000000..4110504d52a --- /dev/null +++ b/public/app/features/dashboard/components/TransformationsEditor/TransformationEditor.tsx @@ -0,0 +1,132 @@ +import React, { useContext } from 'react'; +import { css } from 'emotion'; +import { CustomScrollbar, Icon, JSONFormatter, ThemeContext } from '@grafana/ui'; +import { GrafanaTheme, DataFrame } from '@grafana/data'; + +interface TransformationEditorProps { + name: string; + description: string; + editor?: JSX.Element; + input: DataFrame[]; + output?: DataFrame[]; + debugMode?: boolean; +} + +export const TransformationEditor = ({ editor, input, output, debugMode }: TransformationEditorProps) => { + const theme = useContext(ThemeContext); + const styles = getStyles(theme); + + return ( +
+
+ {editor} + {debugMode && ( +
+
+
Input
+
+ + + +
+
+
+ +
+
+
Output
+ +
+ + + +
+
+
+ )} +
+
+ ); +}; + +const getStyles = (theme: GrafanaTheme) => ({ + title: css` + display: flex; + padding: 4px 8px 4px 8px; + position: relative; + height: 35px; + background: ${theme.colors.textFaint}; + border-radius: 4px 4px 0 0; + flex-wrap: nowrap; + justify-content: space-between; + align-items: center; + `, + name: css` + font-weight: ${theme.typography.weight.semibold}; + color: ${theme.colors.blue}; + `, + iconRow: css` + display: flex; + `, + icon: css` + background: transparent; + border: none; + box-shadow: none; + cursor: pointer; + color: ${theme.colors.textWeak}; + margin-left: ${theme.spacing.sm}; + &:hover { + color: ${theme.colors.text}; + } + `, + editor: css` + padding-top: ${theme.spacing.sm}; + `, + debugWrapper: css` + display: flex; + flex-direction: row; + `, + debugSeparator: css` + width: 48px; + height: 300px; + display: flex; + align-items: center; + justify-content: center; + margin: 0 ${theme.spacing.xs}; + `, + debugTitle: css` + padding: ${theme.spacing.xxs}; + text-align: center; + font-family: ${theme.typography.fontFamily.monospace}; + font-size: ${theme.typography.size.sm}; + color: ${theme.colors.blueBase}; + border-bottom: 1px dashed ${theme.colors.gray15}; + flex-grow: 0; + flex-shrink: 1; + `, + + debug: css` + margin-top: ${theme.spacing.md}; + padding: 0 ${theme.spacing.sm} ${theme.spacing.sm} ${theme.spacing.sm}; + border: 1px dashed ${theme.colors.gray15}; + background: ${theme.colors.gray05}; + border-radius: ${theme.border.radius.sm}; + width: 100%; + height: 300px; + display: flex; + flex-direction: column; + `, + debugJson: css` + flex-grow: 1; + height: 100%; + overflow: hidden; + `, +}); diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx new file mode 100644 index 00000000000..cec515f1f15 --- /dev/null +++ b/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx @@ -0,0 +1,44 @@ +import { DataFrame } from '@grafana/data'; +import React, { useState } from 'react'; +import { HorizontalGroup } from '@grafana/ui'; +import { TransformationEditor } from './TransformationEditor'; +import { QueryOperationRow } from 'app/core/components/QueryOperationRow/QueryOperationRow'; +import { QueryOperationAction } from 'app/core/components/QueryOperationRow/QueryOperationAction'; + +interface TransformationOperationRowProps { + name: string; + description: string; + editor?: JSX.Element; + onRemove: () => void; + input: DataFrame[]; + output: DataFrame[]; +} + +export const TransformationOperationRow: React.FC = ({ + children, + onRemove, + ...props +}) => { + const [showDebug, setShowDebug] = useState(false); + + const renderActions = ({ isOpen }: { isOpen: boolean }) => { + return ( + + { + setShowDebug(!showDebug); + }} + /> + + + ); + }; + + return ( + + + + ); +}; diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationRow.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationRow.tsx deleted file mode 100644 index 24ec55c6374..00000000000 --- a/public/app/features/dashboard/components/TransformationsEditor/TransformationRow.tsx +++ /dev/null @@ -1,83 +0,0 @@ -import React, { useContext, useState } from 'react'; -import { css } from 'emotion'; -import { JSONFormatter, ThemeContext } from '@grafana/ui'; -import { GrafanaTheme, DataFrame } from '@grafana/data'; - -interface TransformationRowProps { - name: string; - description: string; - editor?: JSX.Element; - onRemove: () => void; - input: DataFrame[]; -} - -export const TransformationRow = ({ onRemove, editor, name, input }: TransformationRowProps) => { - const theme = useContext(ThemeContext); - const [viewDebug, setViewDebug] = useState(false); - const styles = getStyles(theme); - return ( -
-
-
{name}
-
-
setViewDebug(!viewDebug)} className={styles.icon}> - -
-
- -
-
-
-
- {editor} - {viewDebug && ( -
- -
- )} -
-
- ); -}; - -const getStyles = (theme: GrafanaTheme) => ({ - title: css` - display: flex; - padding: 4px 8px 4px 8px; - position: relative; - height: 35px; - background: ${theme.colors.textFaint}; - border-radius: 4px 4px 0 0; - flex-wrap: nowrap; - justify-content: space-between; - align-items: center; - `, - name: css` - font-weight: ${theme.typography.weight.semibold}; - color: ${theme.colors.blue}; - `, - iconRow: css` - display: flex; - `, - icon: css` - background: transparent; - border: none; - box-shadow: none; - cursor: pointer; - color: ${theme.colors.textWeak}; - margin-left: ${theme.spacing.sm}; - &:hover { - color: ${theme.colors.text}; - } - `, - editor: css` - border: 2px dashed ${theme.colors.textFaint}; - border-top: none; - border-radius: 0 0 4px 4px; - padding: 8px; - `, -}); diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationsEditor.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationsEditor.tsx index 179927a93a2..1459a97ad8a 100644 --- a/public/app/features/dashboard/components/TransformationsEditor/TransformationsEditor.tsx +++ b/public/app/features/dashboard/components/TransformationsEditor/TransformationsEditor.tsx @@ -1,9 +1,9 @@ import { css } from 'emotion'; import React from 'react'; -import { transformersUIRegistry } from '@grafana/ui/src/components/TransformersUI/transformers'; +import { transformersUIRegistry } from '@grafana/ui'; import { DataTransformerConfig, DataFrame, transformDataFrame, SelectableValue } from '@grafana/data'; -import { Button, Select } from '@grafana/ui'; -import { TransformationRow } from './TransformationRow'; +import { Button, CustomScrollbar, Select, Container } from '@grafana/ui'; +import { TransformationOperationRow } from './TransformationOperationRow'; interface Props { onChange: (transformations: DataTransformerConfig[]) => void; @@ -60,12 +60,15 @@ export class TransformationsEditor extends React.PureComponent {