From e07abd76c0a053de73f0dac7b762e5583743fde7 Mon Sep 17 00:00:00 2001 From: Nathan Marrs Date: Thu, 2 Dec 2021 15:54:45 -0800 Subject: [PATCH] Canvas: refactor layer editor (#42562) Co-authored-by: Ryan McKinley --- .github/CODEOWNERS | 1 + .../core/components/Layers/AddLayerButton.tsx | 23 +++ .../components/Layers/LayerDragDropList.tsx | 171 ++++++++++++++++++ .../components/Layers/LayerName.test.tsx} | 22 +-- .../components/Layers/LayerName.tsx} | 25 ++- public/app/core/components/Layers/types.ts | 4 + public/app/features/canvas/element.ts | 5 +- public/app/features/canvas/registry.ts | 3 +- .../app/features/canvas/runtime/element.tsx | 13 +- public/app/features/canvas/runtime/group.tsx | 5 +- public/app/features/canvas/runtime/scene.tsx | 3 +- .../canvas/editor/LayerElementListEditor.tsx | 171 +++++------------- .../panel/canvas/editor/elementEditor.tsx | 1 + .../app/plugins/panel/geomap/GeomapPanel.tsx | 2 + .../panel/geomap/editor/LayersEditor.tsx | 81 +++++++++ .../editor/LayersEditor/AddLayerButton.tsx | 21 --- .../geomap/editor/LayersEditor/LayerList.tsx | 83 --------- .../editor/LayersEditor/LayersEditor.tsx | 47 ----- public/app/plugins/panel/geomap/module.tsx | 2 +- public/app/plugins/panel/geomap/types.ts | 3 +- public/app/plugins/panel/icon/models.gen.ts | 2 +- 21 files changed, 372 insertions(+), 316 deletions(-) create mode 100644 public/app/core/components/Layers/AddLayerButton.tsx create mode 100644 public/app/core/components/Layers/LayerDragDropList.tsx rename public/app/{plugins/panel/geomap/editor/LayersEditor/LayerHeader.test.tsx => core/components/Layers/LayerName.test.tsx} (74%) rename public/app/{plugins/panel/geomap/editor/LayersEditor/LayerHeader.tsx => core/components/Layers/LayerName.tsx} (86%) create mode 100644 public/app/core/components/Layers/types.ts create mode 100644 public/app/plugins/panel/geomap/editor/LayersEditor.tsx delete mode 100644 public/app/plugins/panel/geomap/editor/LayersEditor/AddLayerButton.tsx delete mode 100644 public/app/plugins/panel/geomap/editor/LayersEditor/LayerList.tsx delete mode 100644 public/app/plugins/panel/geomap/editor/LayersEditor/LayersEditor.tsx diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 51d2261def4..8a87d4c8c0b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -85,6 +85,7 @@ go.sum @grafana/backend-platform /plugins-bundled @grafana/plugins-platform-frontend /public @grafana/user-essentials /public/app/core/components/TimePicker @grafana/grafana-bi-squad +/public/app/core/components/Layers @grafana/grafana-edge-squad /public/app/features/canvas/ @grafana/grafana-edge-squad /public/app/features/dimensions/ @grafana/grafana-edge-squad /public/app/features/live/ @grafana/grafana-edge-squad diff --git a/public/app/core/components/Layers/AddLayerButton.tsx b/public/app/core/components/Layers/AddLayerButton.tsx new file mode 100644 index 00000000000..272735285ad --- /dev/null +++ b/public/app/core/components/Layers/AddLayerButton.tsx @@ -0,0 +1,23 @@ +import React from 'react'; + +import { ValuePicker } from '@grafana/ui'; +import { SelectableValue } from '@grafana/data'; + +type AddLayerButtonProps = { + onChange: (sel: SelectableValue) => void; + options: Array>; + label: string; +}; + +export const AddLayerButton = ({ onChange, options, label }: AddLayerButtonProps) => { + return ( + + ); +}; diff --git a/public/app/core/components/Layers/LayerDragDropList.tsx b/public/app/core/components/Layers/LayerDragDropList.tsx new file mode 100644 index 00000000000..06b36e57848 --- /dev/null +++ b/public/app/core/components/Layers/LayerDragDropList.tsx @@ -0,0 +1,171 @@ +import React from 'react'; +import { DragDropContext, Draggable, Droppable, DropResult } from 'react-beautiful-dnd'; +import { css, cx } from '@emotion/css'; +import { Icon, IconButton, stylesFactory } from '@grafana/ui'; +import { GrafanaTheme } from '@grafana/data'; +import { config } from '@grafana/runtime'; + +import { LayerName } from './LayerName'; +import { LayerElement } from './types'; + +type LayerDragDropListProps = { + layers: T[]; + getLayerInfo: (element: T) => string; + onDragEnd: (result: DropResult) => void; + onSelect: (element: T) => any; + onDelete: (element: T) => any; + onDuplicate?: (element: T) => any; + isGroup?: (element: T) => boolean; + selection?: string[]; // list of unique ids (names) + excludeBaseLayer?: boolean; + onNameChange: (element: T, newName: string) => any; + verifyLayerNameUniqueness?: (nameToCheck: string) => boolean; +}; + +export const LayerDragDropList = ({ + layers, + getLayerInfo, + onDragEnd, + onSelect, + onDelete, + onDuplicate, + isGroup, + selection, + excludeBaseLayer, + onNameChange, + verifyLayerNameUniqueness, +}: LayerDragDropListProps) => { + const style = styles(config.theme); + + const getRowStyle = (isSelected: boolean) => { + return isSelected ? `${style.row} ${style.sel}` : style.row; + }; + + return ( + + + {(provided, snapshot) => ( +
+ {(() => { + // reverse order + const rows: any = []; + const lastLayerIndex = excludeBaseLayer ? 1 : 0; + for (let i = layers.length - 1; i >= lastLayerIndex; i--) { + const element = layers[i]; + const uid = element.getName(); + + const isSelected = Boolean(selection?.includes(uid)); + rows.push( + + {(provided, snapshot) => ( +
onSelect(element)} + > + onNameChange(element, v)} + verifyLayerNameUniqueness={verifyLayerNameUniqueness ?? undefined} + /> +
  {getLayerInfo(element)}
+ + {!isGroup!(element) && ( + <> + {onDuplicate ? ( + onDuplicate(element)} + surface="header" + /> + ) : null} + + onDelete(element)} + surface="header" + /> + {layers.length > 2 && ( + + )} + + )} +
+ )} + + ); + } + + return rows; + })()} + + {provided.placeholder} +
+ )} + + + ); +}; + +LayerDragDropList.defaultProps = { + isGroup: () => false, +}; + +const styles = stylesFactory((theme: GrafanaTheme) => ({ + wrapper: css` + margin-bottom: ${theme.spacing.md}; + `, + row: css` + padding: ${theme.spacing.xs} ${theme.spacing.sm}; + border-radius: ${theme.border.radius.sm}; + background: ${theme.colors.bg2}; + min-height: ${theme.spacing.formInputHeight}px; + display: flex; + align-items: center; + justify-content: space-between; + margin-bottom: 3px; + cursor: pointer; + + border: 1px solid ${theme.colors.formInputBorder}; + &:hover { + border: 1px solid ${theme.colors.formInputBorderHover}; + } + `, + sel: css` + border: 1px solid ${theme.colors.formInputBorderActive}; + &:hover { + border: 1px solid ${theme.colors.formInputBorderActive}; + } + `, + dragIcon: css` + cursor: drag; + `, + actionIcon: css` + color: ${theme.colors.textWeak}; + &:hover { + color: ${theme.colors.text}; + } + `, + typeWrapper: css` + color: ${theme.colors.textBlue}; + margin-right: 5px; + `, + textWrapper: css` + display: flex; + align-items: center; + flex-grow: 1; + overflow: hidden; + margin-right: ${theme.spacing.sm}; + `, +})); diff --git a/public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.test.tsx b/public/app/core/components/Layers/LayerName.test.tsx similarity index 74% rename from public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.test.tsx rename to public/app/core/components/Layers/LayerName.test.tsx index 6528a82e4d0..0fda27cf1ad 100644 --- a/public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.test.tsx +++ b/public/app/core/components/Layers/LayerName.test.tsx @@ -1,8 +1,8 @@ import React from 'react'; import { fireEvent, render, screen } from '@testing-library/react'; -import { LayerHeaderProps, LayerHeader } from './LayerHeader'; +import { LayerNameProps, LayerName } from './LayerName'; -describe('LayerHeader', () => { +describe('LayerName', () => { it('Can edit title', () => { const scenario = renderScenario({}); screen.getByTestId('layer-name-div').click(); @@ -11,7 +11,7 @@ describe('LayerHeader', () => { fireEvent.change(input, { target: { value: 'new name' } }); fireEvent.blur(input); - expect((scenario.props.onChange as any).mock.calls[0][0].name).toBe('new name'); + expect((scenario.props.onChange as any).mock.calls[0][0]).toBe('new name'); }); it('Show error when empty name is specified', async () => { @@ -36,21 +36,21 @@ describe('LayerHeader', () => { expect(alert.textContent).toBe('Layer name already exists'); }); - function renderScenario(overrides: Partial) { - const props: LayerHeaderProps = { - layer: { name: 'Layer 1', type: '?' }, - canRename: (v: string) => { - const names = new Set(['Layer 1', 'Layer 2']); - return !names.has(v); - }, + function renderScenario(overrides: Partial) { + const props: LayerNameProps = { + name: 'Layer 1', onChange: jest.fn(), + verifyLayerNameUniqueness: (nameToCheck: string) => { + const names = new Set(['Layer 1', 'Layer 2']); + return !names.has(nameToCheck); + }, }; Object.assign(props, overrides); return { props, - renderResult: render(), + renderResult: render(), }; } }); diff --git a/public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.tsx b/public/app/core/components/Layers/LayerName.tsx similarity index 86% rename from public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.tsx rename to public/app/core/components/Layers/LayerName.tsx index 5994cf91056..208c1523c9d 100644 --- a/public/app/plugins/panel/geomap/editor/LayersEditor/LayerHeader.tsx +++ b/public/app/core/components/Layers/LayerName.tsx @@ -1,15 +1,15 @@ import React, { useState } from 'react'; import { css, cx } from '@emotion/css'; import { Icon, Input, FieldValidationMessage, useStyles } from '@grafana/ui'; -import { GrafanaTheme, MapLayerOptions } from '@grafana/data'; +import { GrafanaTheme } from '@grafana/data'; -export interface LayerHeaderProps { - layer: MapLayerOptions; - canRename: (v: string) => boolean; - onChange: (layer: MapLayerOptions) => void; +export interface LayerNameProps { + name: string; + onChange: (v: string) => void; + verifyLayerNameUniqueness?: (nameToCheck: string) => boolean; } -export const LayerHeader = ({ layer, canRename, onChange }: LayerHeaderProps) => { +export const LayerName = ({ name, onChange, verifyLayerNameUniqueness }: LayerNameProps) => { const styles = useStyles(getStyles); const [isEditing, setIsEditing] = useState(false); @@ -27,11 +27,8 @@ export const LayerHeader = ({ layer, canRename, onChange }: LayerHeaderProps) => return; } - if (layer.name !== newName) { - onChange({ - ...layer, - name: newName, - }); + if (name !== newName) { + onChange(newName); } }; @@ -43,7 +40,7 @@ export const LayerHeader = ({ layer, canRename, onChange }: LayerHeaderProps) => return; } - if (!canRename(newName)) { + if (verifyLayerNameUniqueness && !verifyLayerNameUniqueness(newName)) { setValidationError('Layer name already exists'); return; } @@ -77,7 +74,7 @@ export const LayerHeader = ({ layer, canRename, onChange }: LayerHeaderProps) => onClick={onEditLayer} data-testid="layer-name-div" > - {layer.name} + {name} )} @@ -86,7 +83,7 @@ export const LayerHeader = ({ layer, canRename, onChange }: LayerHeaderProps) => <> string; +} diff --git a/public/app/features/canvas/element.ts b/public/app/features/canvas/element.ts index 0faf45eac61..90c6d4b650b 100644 --- a/public/app/features/canvas/element.ts +++ b/public/app/features/canvas/element.ts @@ -12,12 +12,13 @@ import { PanelOptionsSupplier } from '@grafana/data/src/panel/PanelPlugin'; * @alpha */ export interface CanvasElementOptions { + name: string; // configured unique display name type: string; // Custom options depending on the type config?: TConfig; - // Standard options avaliable for all elements + // Standard options available for all elements anchor?: Anchor; // defaults top, left, width and height placement?: Placement; background?: BackgroundConfig; @@ -50,7 +51,7 @@ export interface CanvasElementItem extends RegistryI /** Component used to draw */ display: ComponentType>; - getNewOptions: (options?: CanvasElementOptions) => Omit, 'type'>; + getNewOptions: (options?: CanvasElementOptions) => Omit, 'type' | 'name'>; /** Build the configuraiton UI */ registerOptionsUI?: PanelOptionsSupplier>; diff --git a/public/app/features/canvas/registry.ts b/public/app/features/canvas/registry.ts index 6791f7a8682..7e5d2331060 100644 --- a/public/app/features/canvas/registry.ts +++ b/public/app/features/canvas/registry.ts @@ -4,8 +4,9 @@ import { iconItem } from './elements/icon'; import { textBoxItem } from './elements/textBox'; export const DEFAULT_CANVAS_ELEMENT_CONFIG: CanvasElementOptions = { - type: iconItem.id, ...iconItem.getNewOptions(), + type: iconItem.id, + name: `Group ${Date.now()}.${Math.floor(Math.random() * 100)}`, }; export const canvasElementRegistry = new Registry(() => [ diff --git a/public/app/features/canvas/runtime/element.tsx b/public/app/features/canvas/runtime/element.tsx index ff33b093a10..49aad239ed8 100644 --- a/public/app/features/canvas/runtime/element.tsx +++ b/public/app/features/canvas/runtime/element.tsx @@ -12,10 +12,11 @@ import { import { DimensionContext } from 'app/features/dimensions'; import { notFoundItem } from 'app/features/canvas/elements/notFound'; import { GroupState } from './group'; +import { LayerElement } from 'app/core/components/Layers/types'; let counter = 0; -export class ElementState { +export class ElementState implements LayerElement { readonly UID = counter++; revId = 0; @@ -36,12 +37,20 @@ export class ElementState { constructor(public item: CanvasElementItem, public options: CanvasElementOptions, public parent?: GroupState) { if (!options) { - this.options = { type: item.id }; + this.options = { type: item.id, name: `Element ${this.UID}` }; } this.anchor = options.anchor ?? {}; this.placement = options.placement ?? {}; options.anchor = this.anchor; options.placement = this.placement; + + if (!options.name) { + options.name = `Element ${this.UID}`; + } + } + + getName() { + return this.options.name; } validatePlacement() { diff --git a/public/app/features/canvas/runtime/group.tsx b/public/app/features/canvas/runtime/group.tsx index b3eafae7a6c..d8d1ccfd398 100644 --- a/public/app/features/canvas/runtime/group.tsx +++ b/public/app/features/canvas/runtime/group.tsx @@ -99,7 +99,7 @@ export class GroupState extends ElementState { // ??? or should this be on the element directly? // are actions scoped to layers? - doAction = (action: LayerActionID, element: ElementState) => { + doAction = (action: LayerActionID, element: ElementState, updateName = true) => { switch (action) { case LayerActionID.Delete: this.elements = this.elements.filter((e) => e !== element); @@ -128,6 +128,9 @@ export class GroupState extends ElementState { const copy = new ElementState(element.item, opts, this); copy.updateSize(element.width, element.height); copy.updateData(this.scene.context); + if (updateName) { + copy.options.name = `Element ${copy.UID} (duplicate)`; + } this.elements.push(copy); this.scene.save(); this.reinitializeMoveable(); diff --git a/public/app/features/canvas/runtime/scene.tsx b/public/app/features/canvas/runtime/scene.tsx index 354f5c99481..ad1c9d3e0af 100644 --- a/public/app/features/canvas/runtime/scene.tsx +++ b/public/app/features/canvas/runtime/scene.tsx @@ -103,6 +103,7 @@ export class Scene { const newLayer = new GroupState( { type: 'group', + name: `Group ${Date.now()}.${Math.floor(Math.random() * 100)}`, elements: [], }, this, @@ -110,7 +111,7 @@ export class Scene { ); currentSelectedElements.forEach((element: ElementState) => { - newLayer.doAction(LayerActionID.Duplicate, element); + newLayer.doAction(LayerActionID.Duplicate, element, false); currentLayer.doAction(LayerActionID.Delete, element); }); diff --git a/public/app/plugins/panel/canvas/editor/LayerElementListEditor.tsx b/public/app/plugins/panel/canvas/editor/LayerElementListEditor.tsx index 7ee01464394..927aad0e3b8 100644 --- a/public/app/plugins/panel/canvas/editor/LayerElementListEditor.tsx +++ b/public/app/plugins/panel/canvas/editor/LayerElementListEditor.tsx @@ -1,9 +1,7 @@ import React, { PureComponent } from 'react'; -import { css, cx } from '@emotion/css'; -import { Button, HorizontalGroup, Icon, IconButton, stylesFactory, ValuePicker } from '@grafana/ui'; -import { AppEvents, GrafanaTheme, SelectableValue, StandardEditorProps } from '@grafana/data'; -import { config } from '@grafana/runtime'; -import { DragDropContext, Droppable, Draggable, DropResult } from 'react-beautiful-dnd'; +import { Button, HorizontalGroup } from '@grafana/ui'; +import { AppEvents, SelectableValue, StandardEditorProps } from '@grafana/data'; +import { DropResult } from 'react-beautiful-dnd'; import { PanelOptions } from '../models.gen'; import { LayerActionID } from '../types'; @@ -15,12 +13,12 @@ import { GroupState } from 'app/features/canvas/runtime/group'; import { LayerEditorProps } from './layerEditor'; import { SelectionParams } from 'app/features/canvas/runtime/scene'; import { ShowConfirmModalEvent } from 'app/types/events'; +import { LayerDragDropList } from 'app/core/components/Layers/LayerDragDropList'; +import { AddLayerButton } from 'app/core/components/Layers/AddLayerButton'; type Props = StandardEditorProps; export class LayerElementListEditor extends PureComponent { - style = getLayerDragStyles(config.theme); - getScene = () => { const { settings } = this.props.item; if (!settings?.layer) { @@ -86,10 +84,6 @@ export class LayerElementListEditor extends PureComponent { layer.scene.clearCurrentSelection(); }; - getRowStyle = (sel: boolean) => { - return sel ? `${this.style.row} ${this.style.sel}` : this.style.row; - }; - onDragEnd = (result: DropResult) => { if (!result.destination) { return; @@ -133,7 +127,7 @@ export class LayerElementListEditor extends PureComponent { const { layer } = settings; layer.elements.forEach((element: ElementState) => { - layer.parent?.doAction(LayerActionID.Duplicate, element); + layer.parent?.doAction(LayerActionID.Duplicate, element, false); }); this.deleteGroup(); }; @@ -201,8 +195,27 @@ export class LayerElementListEditor extends PureComponent { return
Missing layer?
; } - const styles = this.style; - const selection: number[] = settings.selected ? settings.selected.map((v) => v.UID) : []; + const onDelete = (element: ElementState) => { + layer.doAction(LayerActionID.Delete, element); + }; + + const onDuplicate = (element: ElementState) => { + layer.doAction(LayerActionID.Duplicate, element); + }; + + const getLayerInfo = (element: ElementState) => { + return element.options.type; + }; + + const onNameChange = (element: ElementState, name: string) => { + element.onChange({ ...element.options, name }); + }; + + const isGroup = (element: ElementState) => { + return element instanceof GroupState; + }; + + const selection: string[] = settings.selected ? settings.selected.map((v) => v.getName()) : []; return ( <> {!layer.isRoot() && ( @@ -221,78 +234,24 @@ export class LayerElementListEditor extends PureComponent { )} - - - {(provided, snapshot) => ( -
- {(() => { - // reverse order - const rows: any = []; - for (let i = layer.elements.length - 1; i >= 0; i--) { - const element = layer.elements[i]; - rows.push( - - {(provided, snapshot) => ( -
this.onSelect(element)} - > - {element.item.name} -
-   {element.UID} ({i}) -
- - {element.item.id !== 'group' && ( - <> - layer.doAction(LayerActionID.Duplicate, element)} - surface="header" - /> - - layer.doAction(LayerActionID.Delete, element)} - surface="header" - /> - - - )} -
- )} - - ); - } - return rows; - })()} - - {provided.placeholder} -
- )} - - +
- {selection.length > 0 && (