From 313718c1583a93cdb9277fd8f4ebe84deca28811 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 1 Jun 2022 16:46:10 +0100 Subject: [PATCH 1/3] only set drag handle props on the drag handle itself --- .../QueryOperationRow/QueryOperationRow.tsx | 20 +++++++++++-------- .../OrganizeFieldsTransformerEditor.tsx | 15 +++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx index 85f32fdfa08d..72087c71f04c 100644 --- a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx +++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx @@ -1,6 +1,6 @@ import { css, cx } from '@emotion/css'; import React, { useCallback, useState } from 'react'; -import { Draggable } from 'react-beautiful-dnd'; +import { Draggable, DraggableProvidedDragHandleProps } from 'react-beautiful-dnd'; import { useUpdateEffect } from 'react-use'; import { GrafanaTheme } from '@grafana/data'; @@ -93,7 +93,7 @@ export const QueryOperationRow: React.FC = ({ const actionsElement = actions && ReactUtils.renderOrCallToRender(actions, renderPropArgs); const headerElementRendered = headerElement && ReactUtils.renderOrCallToRender(headerElement, renderPropArgs); - const rowHeader = ( + const getRowHeader = (dragHandleProps?: DraggableProvidedDragHandleProps) => (
= ({
{actionsElement} {draggable && ( - + )}
@@ -122,13 +129,10 @@ export const QueryOperationRow: React.FC = ({ return ( {(provided) => { - const dragHandleProps = { ...provided.dragHandleProps, role: 'group' }; // replace the role="button" because it causes https://dequeuniversity.com/rules/axe/4.3/nested-interactive?application=msftAI return ( <>
-
- {rowHeader} -
+
{getRowHeader(provided.dragHandleProps)}
{isContentVisible &&
{children}
}
@@ -140,7 +144,7 @@ export const QueryOperationRow: React.FC = ({ return (
- {rowHeader} + {getRowHeader()} {isContentVisible &&
{children}
}
); diff --git a/public/app/features/transformers/editors/OrganizeFieldsTransformerEditor.tsx b/public/app/features/transformers/editors/OrganizeFieldsTransformerEditor.tsx index 1365aaf58dce..3fc97f52fa10 100644 --- a/public/app/features/transformers/editors/OrganizeFieldsTransformerEditor.tsx +++ b/public/app/features/transformers/editors/OrganizeFieldsTransformerEditor.tsx @@ -131,15 +131,16 @@ const DraggableFieldName: React.FC = ({ return ( {(provided) => ( -
+
- + Date: Wed, 1 Jun 2022 17:44:45 +0100 Subject: [PATCH 2/3] move into it's own component, add some margin to give some space around the drag handle --- .../QueryOperationRow/QueryOperationRow.tsx | 118 ++++------------- .../QueryOperationRowHeader.tsx | 123 ++++++++++++++++++ 2 files changed, 151 insertions(+), 90 deletions(-) create mode 100644 public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx index 72087c71f04c..331e189e9762 100644 --- a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx +++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx @@ -1,11 +1,13 @@ -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import React, { useCallback, useState } from 'react'; -import { Draggable, DraggableProvidedDragHandleProps } from 'react-beautiful-dnd'; +import { Draggable } from 'react-beautiful-dnd'; import { useUpdateEffect } from 'react-use'; import { GrafanaTheme } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; -import { Icon, ReactUtils, stylesFactory, useTheme } from '@grafana/ui'; +import { ReactUtils, stylesFactory, useTheme } from '@grafana/ui'; + +import { QueryOperationRowHeader } from './QueryOperationRowHeader'; interface QueryOperationRowProps { index: number; @@ -93,38 +95,6 @@ export const QueryOperationRow: React.FC = ({ const actionsElement = actions && ReactUtils.renderOrCallToRender(actions, renderPropArgs); const headerElementRendered = headerElement && ReactUtils.renderOrCallToRender(headerElement, renderPropArgs); - const getRowHeader = (dragHandleProps?: DraggableProvidedDragHandleProps) => ( -
-
- - {title && ( -
-
{titleElement}
-
- )} - {headerElementRendered} -
- -
- {actionsElement} - {draggable && ( - - )} -
-
- ); - if (draggable) { return ( @@ -132,7 +102,19 @@ export const QueryOperationRow: React.FC = ({ return ( <>
-
{getRowHeader(provided.dragHandleProps)}
+
+ +
{isContentVisible &&
{children}
}
@@ -144,7 +126,16 @@ export const QueryOperationRow: React.FC = ({ return (
- {getRowHeader()} + {isContentVisible &&
{children}
}
); @@ -155,63 +146,10 @@ const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => { wrapper: css` margin-bottom: ${theme.spacing.md}; `, - header: css` - label: Header; - padding: ${theme.spacing.xs} ${theme.spacing.sm}; - border-radius: ${theme.border.radius.sm}; - background: ${theme.colors.bg2}; - min-height: ${theme.spacing.formInputHeight}px; - display: grid; - grid-template-columns: minmax(100px, max-content) min-content; - align-items: center; - justify-content: space-between; - white-space: nowrap; - - &:focus { - outline: none; - } - `, - column: css` - label: Column; - display: flex; - align-items: center; - `, - dragIcon: css` - cursor: grab; - color: ${theme.colors.textWeak}; - &:hover { - color: ${theme.colors.text}; - } - `, - collapseIcon: css` - color: ${theme.colors.textWeak}; - cursor: pointer; - &:hover { - color: ${theme.colors.text}; - } - `, - titleWrapper: css` - display: flex; - align-items: center; - flex-grow: 1; - cursor: pointer; - overflow: hidden; - margin-right: ${theme.spacing.sm}; - `, - title: css` - font-weight: ${theme.typography.weight.semibold}; - color: ${theme.colors.textBlue}; - margin-left: ${theme.spacing.sm}; - overflow: hidden; - text-overflow: ellipsis; - `, content: css` margin-top: ${theme.spacing.inlineFormMargin}; margin-left: ${theme.spacing.lg}; `, - disabled: css` - color: ${theme.colors.textWeak}; - `, }; }); diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx new file mode 100644 index 000000000000..9b71a51a226e --- /dev/null +++ b/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx @@ -0,0 +1,123 @@ +import { css, cx } from '@emotion/css'; +import React, { MouseEventHandler } from 'react'; +import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Icon, useStyles2 } from '@grafana/ui'; + +interface QueryOperationRowHeaderProps { + actionsElement?: React.ReactNode; + disabled?: boolean; + draggable: boolean; + dragHandleProps?: DraggableProvidedDragHandleProps; + headerElement?: React.ReactNode; + isContentVisible: boolean; + onRowToggle: () => void; + reportDragMousePosition: MouseEventHandler; + titleElement?: React.ReactNode; +} + +export const QueryOperationRowHeader: React.FC = ({ + actionsElement, + disabled, + draggable, + dragHandleProps, + headerElement, + isContentVisible, + onRowToggle, + reportDragMousePosition, + titleElement, +}: QueryOperationRowHeaderProps) => { + const styles = useStyles2(getStyles); + + return ( +
+
+ + {titleElement && ( +
+
{titleElement}
+
+ )} + {headerElement} +
+ +
+ {actionsElement} + {draggable && ( + + )} +
+
+ ); +}; + +const getStyles = (theme: GrafanaTheme2) => ({ + header: css` + label: Header; + padding: ${theme.spacing(0.5, 0.5)}; + border-radius: ${theme.shape.borderRadius(1)}; + background: ${theme.colors.background.secondary}; + min-height: ${theme.spacing(4)}; + display: grid; + grid-template-columns: minmax(100px, max-content) min-content; + align-items: center; + justify-content: space-between; + white-space: nowrap; + + &:focus { + outline: none; + } + `, + column: css` + label: Column; + display: flex; + align-items: center; + `, + dragIcon: css` + cursor: grab; + color: ${theme.colors.text.disabled}; + margin: ${theme.spacing(0, 0.5)}; + &:hover { + color: ${theme.colors.text}; + } + `, + collapseIcon: css` + color: ${theme.colors.text.disabled}; + cursor: pointer; + &:hover { + color: ${theme.colors.text}; + } + `, + titleWrapper: css` + display: flex; + align-items: center; + flex-grow: 1; + cursor: pointer; + overflow: hidden; + margin-right: ${theme.spacing(0.5)}; + `, + title: css` + font-weight: ${theme.typography.fontWeightBold}; + color: ${theme.colors.text.link}; + margin-left: ${theme.spacing(0.5)}; + overflow: hidden; + text-overflow: ellipsis; + `, + disabled: css` + color: ${theme.colors.text.disabled}; + `, +}); + +QueryOperationRowHeader.displayName = 'QueryOperationRowHeader'; From 5ca8200f545eee0e53557cd29299e771d806b7d0 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 1 Jun 2022 17:52:27 +0100 Subject: [PATCH 3/3] fix unit tests --- .betterer.results | 2 +- .../QueryOperationRow/QueryOperationRow.test.tsx | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.betterer.results b/.betterer.results index 95c4515507ea..9f58a32036f0 100644 --- a/.betterer.results +++ b/.betterer.results @@ -110,7 +110,7 @@ exports[`no enzyme tests`] = { "public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx:3032694716": [ [0, 19, 13, "RegExp match", "2409514259"] ], - "public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx:2026575657": [ + "public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx:3743889097": [ [0, 26, 13, "RegExp match", "2409514259"] ], "public/app/core/components/Select/MetricSelect.test.tsx:3351544014": [ diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx index afcccbf93fbb..b046539d99f5 100644 --- a/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx +++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx @@ -60,7 +60,7 @@ describe('QueryOperationRow', () => { describe('headerElement rendering', () => { it('should render headerElement provided as element', () => { const title =
Test
; - const wrapper = shallow( + const wrapper = mount(
Test
@@ -72,7 +72,7 @@ describe('QueryOperationRow', () => { it('should render headerElement provided as function', () => { const title = () =>
Test
; - const wrapper = shallow( + const wrapper = mount(
Test
@@ -101,7 +101,7 @@ describe('QueryOperationRow', () => { describe('actions rendering', () => { it('should render actions provided as element', () => { const actions =
Test
; - const wrapper = shallow( + const wrapper = mount(
Test
@@ -112,7 +112,7 @@ describe('QueryOperationRow', () => { }); it('should render actions provided as function', () => { const actions = () =>
Test
; - const wrapper = shallow( + const wrapper = mount(
Test