From 5e517d1c8e5d893b083cc31a128dbcf1ceac109c Mon Sep 17 00:00:00 2001 From: Ivana Huckova Date: Tue, 30 Jan 2024 15:18:44 +0100 Subject: [PATCH 1/3] Fix renderinf og OperationEditor --- .../components/OperationEditor.tsx | 275 ++--------------- .../components/OperationEditorBody.tsx | 288 ++++++++++++++++++ 2 files changed, 308 insertions(+), 255 deletions(-) create mode 100644 src/VisualQueryBuilder/components/OperationEditorBody.tsx diff --git a/src/VisualQueryBuilder/components/OperationEditor.tsx b/src/VisualQueryBuilder/components/OperationEditor.tsx index 21d04ba..6d0beeb 100644 --- a/src/VisualQueryBuilder/components/OperationEditor.tsx +++ b/src/VisualQueryBuilder/components/OperationEditor.tsx @@ -1,22 +1,16 @@ import { css, cx } from '@emotion/css'; -import React, { useEffect, useRef, useState } from 'react'; -import { Draggable, DraggableProvided } from 'react-beautiful-dnd'; -import { v4 } from 'uuid'; +import React from 'react'; +import { Draggable } from 'react-beautiful-dnd'; import { DataSourceApi, GrafanaTheme2, TimeRange } from '@grafana/data'; -import { Button, Icon, InlineField, Tooltip, useTheme2 } from '@grafana/ui'; +import { InlineField, useTheme2 } from '@grafana/ui'; -import { OperationHeader } from './OperationHeader'; import { QueryBuilderOperation, - QueryBuilderOperationDefinition, - QueryBuilderOperationParamDef, - QueryBuilderOperationParamValue, VisualQuery, VisualQueryModeller, } from '../types'; -import { Stack } from '../../QueryEditor/Stack'; -import { getOperationParamEditor, getOperationParamId } from './OperationParamEditor'; +import { OperationEditorBody } from './OperationEditorBody'; interface Props { operation: QueryBuilderOperation; @@ -48,8 +42,6 @@ export function OperationEditor({ isConflictingOperation, }: Props) { const def = queryModeller.getOperationDefinition(operation.id); - const shouldFlash = useFlash(flash); - const { current: id } = useRef(v4()); const theme = useTheme2(); const isConflicting = isConflictingOperation ? isConflictingOperation(operation, query.operations) : false; @@ -59,83 +51,6 @@ export function OperationEditor({ return Operation {operation.id} not found; } - const onParamValueChanged = (paramIdx: number, value: QueryBuilderOperationParamValue) => { - const update: QueryBuilderOperation = { ...operation, params: [...operation.params] }; - update.params[paramIdx] = value; - callParamChangedThenOnChange(def, update, index, paramIdx, onChange); - }; - - const onAddRestParam = () => { - const update: QueryBuilderOperation = { ...operation, params: [...operation.params, ''] }; - callParamChangedThenOnChange(def, update, index, operation.params.length, onChange); - }; - - const onRemoveRestParam = (paramIdx: number) => { - const update: QueryBuilderOperation = { - ...operation, - params: [...operation.params.slice(0, paramIdx), ...operation.params.slice(paramIdx + 1)], - }; - callParamChangedThenOnChange(def, update, index, paramIdx, onChange); - }; - - const operationElements: React.ReactNode[] = []; - - for (let paramIndex = 0; paramIndex < operation.params.length; paramIndex++) { - const paramDef = def.params[Math.min(def.params.length - 1, paramIndex)]; - const Editor = getOperationParamEditor(paramDef); - - operationElements.push( -
- {!paramDef.hideName && ( -
- - {paramDef.description && ( - - - - )} -
- )} -
- - - {paramDef.restParam && (operation.params.length > def.params.length || paramDef.optional) && ( -
-
- ); - } - - // Handle adding button for rest params - let restParam: React.ReactNode | undefined; - if (def.params.length > 0) { - const lastParamDef = def.params[def.params.length - 1]; - if (lastParamDef.restParam) { - restParam = renderAddRestParamButton(lastParamDef, onAddRestParam, index, operation.params.length, styles); - } - } const isInvalid = (isDragging: boolean) => { if (isDragging) { @@ -145,38 +60,6 @@ export function OperationEditor({ return isConflicting ? true : undefined; }; - // We need to extract this into a component to prevent InlineField passing invalid to div which produces console error - const StyledOperationHeader = ({ provided }: { provided: DraggableProvided }) => ( -
- -
{operationElements}
- {restParam} - {index < query.operations.length - 1 && ( -
-
-
-
- )} -
- ); - return ( {(provided, snapshot) => ( @@ -185,72 +68,28 @@ export function OperationEditor({ invalid={isInvalid(snapshot.isDragging)} className={cx(styles.error, styles.cardWrapper)} > - + )} ); } -/** - * When flash is switched on makes sure it is switched of right away, so we just flash the highlight and then fade - * out. - * @param flash - */ -function useFlash(flash?: boolean) { - const [keepFlash, setKeepFlash] = useState(true); - useEffect(() => { - let t: ReturnType; - if (flash) { - t = setTimeout(() => { - setKeepFlash(false); - }, 1000); - } else { - setKeepFlash(true); - } - - return () => clearTimeout(t); - }, [flash]); - - return keepFlash && flash; -} - -function renderAddRestParamButton( - paramDef: QueryBuilderOperationParamDef, - onAddRestParam: () => void, - operationIndex: number, - paramIndex: number, - styles: OperationEditorStyles -) { - return ( -
- -
- ); -} - -function callParamChangedThenOnChange( - def: QueryBuilderOperationDefinition, - operation: QueryBuilderOperation, - operationIndex: number, - paramIndex: number, - onChange: (index: number, update: QueryBuilderOperation) => void -) { - if (def.paramChangedHandler) { - onChange(operationIndex, def.paramChangedHandler(paramIndex, operation, def)); - } else { - onChange(operationIndex, operation); - } -} const getStyles = (theme: GrafanaTheme2, isConflicting: boolean) => { return { @@ -260,80 +99,6 @@ const getStyles = (theme: GrafanaTheme2, isConflicting: boolean) => { error: css({ marginBottom: theme.spacing(1), }), - card: css({ - background: theme.colors.background.primary, - border: `1px solid ${theme.colors.border.medium}`, - cursor: 'grab', - borderRadius: theme.shape.radius.default, - position: 'relative', - transition: 'all 0.5s ease-in 0s', - height: isConflicting ? 'auto' : '100%', - }), - cardError: css({ - boxShadow: `0px 0px 4px 0px ${theme.colors.warning.main}`, - border: `1px solid ${theme.colors.warning.main}`, - }), - cardHighlight: css({ - boxShadow: `0px 0px 4px 0px ${theme.colors.primary.border}`, - border: `1px solid ${theme.colors.primary.border}`, - }), - infoIcon: css({ - marginLeft: theme.spacing(0.5), - color: theme.colors.text.secondary, - ':hover': { - color: theme.colors.text.primary, - }, - }), - body: css({ - margin: theme.spacing(1, 1, 0.5, 1), - display: 'table', - }), - paramRow: css({ - label: 'paramRow', - display: 'table-row', - verticalAlign: 'middle', - }), - paramName: css({ - display: 'table-cell', - padding: theme.spacing(0, 1, 0, 0), - fontSize: theme.typography.bodySmall.fontSize, - fontWeight: theme.typography.fontWeightMedium, - verticalAlign: 'middle', - height: '32px', - }), - paramValue: css({ - label: 'paramValue', - display: 'table-cell', - verticalAlign: 'middle', - }), - restParam: css({ - padding: theme.spacing(0, 1, 1, 1), - }), - arrow: css({ - position: 'absolute', - top: '0', - right: '-18px', - display: 'flex', - }), - arrowLine: css({ - height: '2px', - width: '8px', - backgroundColor: theme.colors.border.strong, - position: 'relative', - top: '14px', - }), - arrowArrow: css({ - width: 0, - height: 0, - borderTop: `5px solid transparent`, - borderBottom: `5px solid transparent`, - borderLeft: `7px solid ${theme.colors.border.strong}`, - position: 'relative', - top: '10px', - }), }; }; -type OperationEditorStyles = ReturnType; - -export { getOperationParamId }; diff --git a/src/VisualQueryBuilder/components/OperationEditorBody.tsx b/src/VisualQueryBuilder/components/OperationEditorBody.tsx new file mode 100644 index 0000000..b24f1a0 --- /dev/null +++ b/src/VisualQueryBuilder/components/OperationEditorBody.tsx @@ -0,0 +1,288 @@ +import React, { useEffect, useRef, useState } from 'react'; +import { DraggableProvided } from 'react-beautiful-dnd'; +import { Button, Icon, Tooltip, useTheme2 } from '@grafana/ui'; +import { css, cx } from '@emotion/css'; +import { DataSourceApi, GrafanaTheme2, TimeRange } from '@grafana/data'; +import { OperationHeader } from './OperationHeader'; +import { QueryBuilderOperation, QueryBuilderOperationDefinition, QueryBuilderOperationParamDef, QueryBuilderOperationParamValue, VisualQuery, VisualQueryModeller } from '../types'; +import { getOperationParamEditor, getOperationParamId } from './OperationParamEditor'; +import { Stack } from '../../QueryEditor/Stack'; +import { v4 } from 'uuid'; + +type Props = { + provided: DraggableProvided; + isConflicting: boolean, + index: number, + operation: QueryBuilderOperation, + definition: QueryBuilderOperationDefinition, + queryModeller: VisualQueryModeller; + query: VisualQuery; + onChange: (index: number, update: QueryBuilderOperation) => void; + onRemove: (index: number) => void; + onRunQuery: () => void; + datasource: DataSourceApi; + flash?: boolean, + highlight?: boolean, + timeRange?: TimeRange; + } + +export function OperationEditorBody({ provided, flash, isConflicting, highlight, index, queryModeller, onChange, onRemove, operation, definition, query, timeRange, onRunQuery, datasource }: Props) { + const theme = useTheme2(); + const styles = getStyles(theme, isConflicting); + const shouldFlash = useFlash(flash); + const { current: id } = useRef(v4()); + + const onParamValueChanged = (paramIdx: number, value: QueryBuilderOperationParamValue) => { + const update: QueryBuilderOperation = { ...operation, params: [...operation.params] }; + update.params[paramIdx] = value; + callParamChangedThenOnChange(definition, update, index, paramIdx, onChange); + }; + + const onAddRestParam = () => { + const update: QueryBuilderOperation = { ...operation, params: [...operation.params, ''] }; + callParamChangedThenOnChange(definition, update, index, operation.params.length, onChange); + }; + + const onRemoveRestParam = (paramIdx: number) => { + const update: QueryBuilderOperation = { + ...operation, + params: [...operation.params.slice(0, paramIdx), ...operation.params.slice(paramIdx + 1)], + }; + callParamChangedThenOnChange(definition, update, index, paramIdx, onChange); + }; + + // Handle adding button for rest params + let restParam: React.ReactNode | undefined; + if (definition.params.length > 0) { + const lastParamDef = definition.params[definition.params.length - 1]; + if (lastParamDef.restParam) { + restParam = renderAddRestParamButton(lastParamDef, onAddRestParam, index, operation.params.length, styles); + } + } + + return ( +
+ +
+ { + operation.params.map((param, paramIndex) => { + const paramDef = definition.params[Math.min(definition.params.length - 1, paramIndex)]; + const Editor = getOperationParamEditor(paramDef); + + return ( +
+ {!paramDef.hideName && ( +
+ + {paramDef.description && ( + + + + )} +
+ )} +
+ + + {paramDef.restParam && (operation.params.length > definition.params.length || paramDef.optional) && ( +
+
+ ) + }) + } +
+ {restParam} + {index < query.operations.length - 1 && ( +
+
+
+
+ )} +
+ ); +} + +const getStyles = (theme: GrafanaTheme2, isConflicting: boolean) => { + return { + cardWrapper: css({ + alignItems: 'stretch', + }), + error: css({ + marginBottom: theme.spacing(1), + }), + card: css({ + background: theme.colors.background.primary, + border: `1px solid ${theme.colors.border.medium}`, + cursor: 'grab', + borderRadius: theme.shape.radius.default, + position: 'relative', + transition: 'all 0.5s ease-in 0s', + height: isConflicting ? 'auto' : '100%', + }), + cardError: css({ + boxShadow: `0px 0px 4px 0px ${theme.colors.warning.main}`, + border: `1px solid ${theme.colors.warning.main}`, + }), + cardHighlight: css({ + boxShadow: `0px 0px 4px 0px ${theme.colors.primary.border}`, + border: `1px solid ${theme.colors.primary.border}`, + }), + infoIcon: css({ + marginLeft: theme.spacing(0.5), + color: theme.colors.text.secondary, + ':hover': { + color: theme.colors.text.primary, + }, + }), + body: css({ + margin: theme.spacing(1, 1, 0.5, 1), + display: 'table', + }), + paramRow: css({ + label: 'paramRow', + display: 'table-row', + verticalAlign: 'middle', + }), + paramName: css({ + display: 'table-cell', + padding: theme.spacing(0, 1, 0, 0), + fontSize: theme.typography.bodySmall.fontSize, + fontWeight: theme.typography.fontWeightMedium, + verticalAlign: 'middle', + height: '32px', + }), + paramValue: css({ + label: 'paramValue', + display: 'table-cell', + verticalAlign: 'middle', + }), + restParam: css({ + padding: theme.spacing(0, 1, 1, 1), + }), + arrow: css({ + position: 'absolute', + top: '0', + right: '-18px', + display: 'flex', + }), + arrowLine: css({ + height: '2px', + width: '8px', + backgroundColor: theme.colors.border.strong, + position: 'relative', + top: '14px', + }), + arrowArrow: css({ + width: 0, + height: 0, + borderTop: `5px solid transparent`, + borderBottom: `5px solid transparent`, + borderLeft: `7px solid ${theme.colors.border.strong}`, + position: 'relative', + top: '10px', + }), + }; +}; + +/** + * When flash is switched on makes sure it is switched of right away, so we just flash the highlight and then fade + * out. + * @param flash + */ +function useFlash(flash?: boolean) { + const [keepFlash, setKeepFlash] = useState(true); + useEffect(() => { + let t: ReturnType; + if (flash) { + t = setTimeout(() => { + setKeepFlash(false); + }, 1000); + } else { + setKeepFlash(true); + } + + return () => clearTimeout(t); + }, [flash]); + + return keepFlash && flash; +} + + +function callParamChangedThenOnChange( + def: QueryBuilderOperationDefinition, + operation: QueryBuilderOperation, + operationIndex: number, + paramIndex: number, + onChange: (index: number, update: QueryBuilderOperation) => void +) { + if (def.paramChangedHandler) { + onChange(operationIndex, def.paramChangedHandler(paramIndex, operation, def)); + } else { + onChange(operationIndex, operation); + } +} + +function renderAddRestParamButton( + paramDef: QueryBuilderOperationParamDef, + onAddRestParam: () => void, + operationIndex: number, + paramIndex: number, + styles: OperationEditorStyles +) { + return ( +
+ +
+ ); +} + +type OperationEditorStyles = ReturnType; \ No newline at end of file From d6a0047abd3c4806cbf9ba3d0a98b4826ba91848 Mon Sep 17 00:00:00 2001 From: Ivana Huckova Date: Tue, 30 Jan 2024 15:54:16 +0100 Subject: [PATCH 2/3] Add newline -lint issue --- src/VisualQueryBuilder/components/OperationEditorBody.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualQueryBuilder/components/OperationEditorBody.tsx b/src/VisualQueryBuilder/components/OperationEditorBody.tsx index b24f1a0..f927452 100644 --- a/src/VisualQueryBuilder/components/OperationEditorBody.tsx +++ b/src/VisualQueryBuilder/components/OperationEditorBody.tsx @@ -285,4 +285,4 @@ function renderAddRestParamButton( ); } -type OperationEditorStyles = ReturnType; \ No newline at end of file +type OperationEditorStyles = ReturnType; From eadd757e3bf0f18bc6e6418139927eeb29a1c14a Mon Sep 17 00:00:00 2001 From: Ivana Huckova Date: Tue, 30 Jan 2024 17:52:52 +0100 Subject: [PATCH 3/3] Fix linting --- .../components/OperationEditorBody.tsx | 173 ++++++++++-------- 1 file changed, 94 insertions(+), 79 deletions(-) diff --git a/src/VisualQueryBuilder/components/OperationEditorBody.tsx b/src/VisualQueryBuilder/components/OperationEditorBody.tsx index f927452..5a5838c 100644 --- a/src/VisualQueryBuilder/components/OperationEditorBody.tsx +++ b/src/VisualQueryBuilder/components/OperationEditorBody.tsx @@ -4,29 +4,51 @@ import { Button, Icon, Tooltip, useTheme2 } from '@grafana/ui'; import { css, cx } from '@emotion/css'; import { DataSourceApi, GrafanaTheme2, TimeRange } from '@grafana/data'; import { OperationHeader } from './OperationHeader'; -import { QueryBuilderOperation, QueryBuilderOperationDefinition, QueryBuilderOperationParamDef, QueryBuilderOperationParamValue, VisualQuery, VisualQueryModeller } from '../types'; +import { + QueryBuilderOperation, + QueryBuilderOperationDefinition, + QueryBuilderOperationParamDef, + QueryBuilderOperationParamValue, + VisualQuery, + VisualQueryModeller, +} from '../types'; import { getOperationParamEditor, getOperationParamId } from './OperationParamEditor'; import { Stack } from '../../QueryEditor/Stack'; import { v4 } from 'uuid'; type Props = { - provided: DraggableProvided; - isConflicting: boolean, - index: number, - operation: QueryBuilderOperation, - definition: QueryBuilderOperationDefinition, - queryModeller: VisualQueryModeller; - query: VisualQuery; - onChange: (index: number, update: QueryBuilderOperation) => void; - onRemove: (index: number) => void; - onRunQuery: () => void; - datasource: DataSourceApi; - flash?: boolean, - highlight?: boolean, - timeRange?: TimeRange; - } - -export function OperationEditorBody({ provided, flash, isConflicting, highlight, index, queryModeller, onChange, onRemove, operation, definition, query, timeRange, onRunQuery, datasource }: Props) { + provided: DraggableProvided; + isConflicting: boolean; + index: number; + operation: QueryBuilderOperation; + definition: QueryBuilderOperationDefinition; + queryModeller: VisualQueryModeller; + query: VisualQuery; + onChange: (index: number, update: QueryBuilderOperation) => void; + onRemove: (index: number) => void; + onRunQuery: () => void; + datasource: DataSourceApi; + flash?: boolean; + highlight?: boolean; + timeRange?: TimeRange; +}; + +export function OperationEditorBody({ + provided, + flash, + isConflicting, + highlight, + index, + queryModeller, + onChange, + onRemove, + operation, + definition, + query, + timeRange, + onRunQuery, + datasource, +}: Props) { const theme = useTheme2(); const styles = getStyles(theme, isConflicting); const shouldFlash = useFlash(flash); @@ -60,13 +82,9 @@ export function OperationEditorBody({ provided, flash, isConflicting, highlight, } } - return ( -
- { - operation.params.map((param, paramIndex) => { - const paramDef = definition.params[Math.min(definition.params.length - 1, paramIndex)]; - const Editor = getOperationParamEditor(paramDef); + {operation.params.map((param, paramIndex) => { + const paramDef = definition.params[Math.min(definition.params.length - 1, paramIndex)]; + const Editor = getOperationParamEditor(paramDef); - return ( -
- {!paramDef.hideName && ( -
- - {paramDef.description && ( - - - + return ( +
+ {!paramDef.hideName && ( +
+ + {paramDef.description && ( + + + + )} +
)} +
+ + + {paramDef.restParam && (operation.params.length > definition.params.length || paramDef.optional) && ( +
- )} -
- - - {paramDef.restParam && (operation.params.length > definition.params.length || paramDef.optional) && ( -
+ ); + })} +
+ {restParam} + {index < query.operations.length - 1 && ( +
+
+
- ) - }) - } -
- {restParam} - {index < query.operations.length - 1 && ( -
-
-
+ )}
- )} -
); } @@ -247,7 +263,6 @@ function useFlash(flash?: boolean) { return keepFlash && flash; } - function callParamChangedThenOnChange( def: QueryBuilderOperationDefinition, operation: QueryBuilderOperation,