QueryEditorRow: Ability to change query name (#29779)

* QueryEditorRow: Ability to change query name

* Style tweaks

* Updated UX

* Fixed tests

* Added validation messages

* Fixed keyboard navigation

* Updated tests

packages/grafana-ui/src/components/Forms/FieldValidationMessage.story.tsx

@@ -19,8 +19,14 @@ export default {
   },
 };
 
-export const simple = () => {
+export const vertical = () => {
   const { message } = getKnobs();
 
   return <FieldValidationMessage>{message}</FieldValidationMessage>;
 };
+
+export const horizontal = () => {
+  const { message } = getKnobs();
+
+  return <FieldValidationMessage horizontal>{message}</FieldValidationMessage>;
+};

packages/grafana-ui/src/components/Forms/FieldValidationMessage.tsx

@@ -8,31 +8,52 @@ export interface FieldValidationMessageProps {
   children: string;
   /** Override component style */
   className?: string;
+  horizontal?: boolean;
 }
 
 export const getFieldValidationMessageStyles = stylesFactory((theme: GrafanaTheme) => {
-  return {
-    fieldValidationMessage: css`
+  const baseStyle = `
       font-size: ${theme.typography.size.sm};
       font-weight: ${theme.typography.weight.semibold};
-      margin: ${theme.spacing.formValidationMessageMargin};
       padding: ${theme.spacing.formValidationMessagePadding};
       color: ${theme.colors.formValidationMessageText};
       background: ${theme.colors.formValidationMessageBg};
       border-radius: ${theme.border.radius.sm};
       position: relative;
       display: inline-block;
+    `;
+
+  return {
+    vertical: css`
+      ${baseStyle}
+      margin: ${theme.spacing.formValidationMessageMargin};
 
       &:before {
         content: '';
         position: absolute;
         left: 9px;
-        top: -4px;
+        top: -5px;
         width: 0;
         height: 0;
-        border-left: 4px solid transparent;
-        border-right: 4px solid transparent;
-        border-bottom: 4px solid ${theme.colors.formValidationMessageBg};
+        border-width: 0 4px 5px 4px;
+        border-color: transparent transparent ${theme.colors.formValidationMessageBg} transparent;
+        border-style: solid;
+      }
+    `,
+    horizontal: css`
+      ${baseStyle}
+      margin-left: 10px;
+
+      &:before {
+        content: '';
+        position: absolute;
+        left: -5px;
+        top: 9px;
+        width: 0;
+        height: 0;
+        border-width: 4px 5px 4px 0;
+        border-color: transparent #e02f44 transparent transparent;
+        border-style: solid;
       }
     `,
     fieldValidationMessageIcon: css`
@@ -41,12 +62,13 @@ export const getFieldValidationMessageStyles = stylesFactory((theme: GrafanaThem
   };
 });
 
-export const FieldValidationMessage: React.FC<FieldValidationMessageProps> = ({ children, className }) => {
+export const FieldValidationMessage: React.FC<FieldValidationMessageProps> = ({ children, horizontal, className }) => {
   const theme = useTheme();
   const styles = getFieldValidationMessageStyles(theme);
+  const cssName = cx(horizontal ? styles.horizontal : styles.vertical, className);
 
   return (
-    <div role="alert" className={cx(styles.fieldValidationMessage, className)}>
+    <div role="alert" className={cssName}>
       <Icon className={styles.fieldValidationMessageIcon} name="exclamation-triangle" />
       {children}
     </div>

public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx

@@ -32,7 +32,7 @@ describe('QueryOperationRow', () => {
       const onOpenSpy = jest.fn();
       const onCloseSpy = jest.fn();
       const wrapper = mount(
-        <QueryOperationRow onOpen={onOpenSpy} onClose={onCloseSpy} isOpen={false} id="test-id" index={0}>
+        <QueryOperationRow title="title" onOpen={onOpenSpy} onClose={onCloseSpy} isOpen={false} id="test-id" index={0}>
           <div>Test</div>
         </QueryOperationRow>
       );
@@ -56,11 +56,11 @@ describe('QueryOperationRow', () => {
     });
   });
 
-  describe('title rendering', () => {
-    it('should render title provided as element', () => {
+  describe('headerElement rendering', () => {
+    it('should render headerElement provided as element', () => {
       const title = <div aria-label="test title">Test</div>;
       const wrapper = shallow(
-        <QueryOperationRow title={title} id="test-id" index={0}>
+        <QueryOperationRow headerElement={title} id="test-id" index={0}>
           <div>Test</div>
         </QueryOperationRow>
       );
@@ -68,10 +68,11 @@ describe('QueryOperationRow', () => {
       const titleEl = wrapper.find({ 'aria-label': 'test title' });
       expect(titleEl).toHaveLength(1);
     });
-    it('should render title provided as function', () => {
+
+    it('should render headerElement provided as function', () => {
       const title = () => <div aria-label="test title">Test</div>;
       const wrapper = shallow(
-        <QueryOperationRow title={title} id="test-id" index={0}>
+        <QueryOperationRow headerElement={title} id="test-id" index={0}>
           <div>Test</div>
         </QueryOperationRow>
       );
@@ -80,14 +81,14 @@ describe('QueryOperationRow', () => {
       expect(titleEl).toHaveLength(1);
     });
 
-    it('should expose api to title rendered as function', () => {
+    it('should expose api to headerElement rendered as function', () => {
       const propsSpy = jest.fn();
       const title = (props: any) => {
         propsSpy(props);
         return <div aria-label="test title">Test</div>;
       };
       shallow(
-        <QueryOperationRow title={title} id="test-id" index={0}>
+        <QueryOperationRow headerElement={title} id="test-id" index={0}>
           <div>Test</div>
         </QueryOperationRow>
       );
@@ -132,9 +133,7 @@ describe('QueryOperationRow', () => {
         </QueryOperationRow>
       );
 
-      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');
+      expect(Object.keys(propsSpy.mock.calls[0][0])).toEqual(['isOpen', 'onOpen', 'onClose']);
     });
   });
 });

public/app/core/components/QueryOperationRow/QueryOperationRow.tsx

@@ -8,11 +8,9 @@ import { Draggable } from 'react-beautiful-dnd';
 interface QueryOperationRowProps {
   index: number;
   id: string;
-  title?: ((props: { isOpen: boolean }) => React.ReactNode) | React.ReactNode;
-  headerElement?: React.ReactNode;
-  actions?:
-    | ((props: { isOpen: boolean; openRow: () => void; closeRow: () => void }) => React.ReactNode)
-    | React.ReactNode;
+  title?: string;
+  headerElement?: QueryOperationRowRenderProp;
+  actions?: QueryOperationRowRenderProp;
   onOpen?: () => void;
   onClose?: () => void;
   children: React.ReactNode;
@@ -20,6 +18,14 @@ interface QueryOperationRowProps {
   draggable?: boolean;
 }
 
+export type QueryOperationRowRenderProp = ((props: QueryOperationRowRenderProps) => React.ReactNode) | React.ReactNode;
+
+export interface QueryOperationRowRenderProps {
+  isOpen: boolean;
+  onOpen: () => void;
+  onClose: () => void;
+}
+
 export const QueryOperationRow: React.FC<QueryOperationRowProps> = ({
   children,
   actions,
@@ -51,26 +57,33 @@ export const QueryOperationRow: React.FC<QueryOperationRowProps> = ({
     }
   }, [isContentVisible]);
 
-  const titleElement = title && renderOrCallToRender(title, { isOpen: isContentVisible });
-  const actionsElement =
-    actions &&
-    renderOrCallToRender(actions, {
-      isOpen: isContentVisible,
-      openRow: () => {
-        setIsContentVisible(true);
-      },
-      closeRow: () => {
-        setIsContentVisible(false);
-      },
-    });
+  const renderPropArgs: QueryOperationRowRenderProps = {
+    isOpen: isContentVisible,
+    onOpen: () => {
+      setIsContentVisible(true);
+    },
+    onClose: () => {
+      setIsContentVisible(false);
+    },
+  };
+
+  const titleElement = title && renderOrCallToRender(title, renderPropArgs);
+  const actionsElement = actions && renderOrCallToRender(actions, renderPropArgs);
+  const headerElementRendered = headerElement && renderOrCallToRender(headerElement, renderPropArgs);
 
   const rowHeader = (
     <div className={styles.header}>
-      <div className={styles.titleWrapper} onClick={onRowToggle} aria-label="Query operation row title">
-        <Icon name={isContentVisible ? 'angle-down' : 'angle-right'} className={styles.collapseIcon} />
-        {title && <div className={styles.title}>{titleElement}</div>}
-        {headerElement}
-      </div>
+      <Icon
+        name={isContentVisible ? 'angle-down' : 'angle-right'}
+        className={styles.collapseIcon}
+        onClick={onRowToggle}
+      />
+      {title && (
+        <div className={styles.titleWrapper} onClick={onRowToggle} aria-label="Query operation row title">
+          <div className={styles.title}>{titleElement}</div>
+        </div>
+      )}
+      {headerElementRendered}
       {actions && <div>{actionsElement}</div>}
       {draggable && (
         <Icon title="Drag and drop to reorder" name="draggabledots" size="lg" className={styles.dragIcon} />
@@ -126,6 +139,7 @@ const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => {
     `,
     collapseIcon: css`
       color: ${theme.colors.textWeak};
+      cursor: pointer;
       &:hover {
         color: ${theme.colors.text};
       }

public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx

@@ -3,7 +3,10 @@ import { DataFrame, DataTransformerConfig, TransformerRegistyItem } from '@grafa
 import { HorizontalGroup } from '@grafana/ui';
 
 import { TransformationEditor } from './TransformationEditor';
-import { QueryOperationRow } from 'app/core/components/QueryOperationRow/QueryOperationRow';
+import {
+  QueryOperationRow,
+  QueryOperationRowRenderProps,
+} from 'app/core/components/QueryOperationRow/QueryOperationRow';
 import { QueryOperationAction } from 'app/core/components/QueryOperationRow/QueryOperationAction';
 import { TransformationsEditorTransformation } from './types';
 
@@ -28,7 +31,7 @@ export const TransformationOperationRow: React.FC<TransformationOperationRowProp
 }) => {
   const [showDebug, setShowDebug] = useState(false);
 
-  const renderActions = ({ isOpen }: { isOpen: boolean }) => {
+  const renderActions = ({ isOpen }: QueryOperationRowRenderProps) => {
     return (
       <HorizontalGroup align="center" width="auto">
         <QueryOperationAction

public/app/features/query/components/QueryEditorRow.tsx

@@ -19,7 +19,10 @@ import {
   DataSourceInstanceSettings,
 } from '@grafana/data';
 import { QueryEditorRowTitle } from './QueryEditorRowTitle';
-import { QueryOperationRow } from 'app/core/components/QueryOperationRow/QueryOperationRow';
+import {
+  QueryOperationRow,
+  QueryOperationRowRenderProps,
+} from 'app/core/components/QueryOperationRow/QueryOperationRow';
 import { QueryOperationAction } from 'app/core/components/QueryOperationRow/QueryOperationAction';
 import { DashboardModel } from '../../dashboard/state/DashboardModel';
 import { selectors } from '@grafana/e2e-selectors';
@@ -199,13 +202,13 @@ export class QueryEditorRow extends PureComponent<Props, State> {
     return <div>Data source plugin does not export any Query Editor component</div>;
   };
 
-  onToggleEditMode = (e: React.MouseEvent, { isOpen, openRow }: { isOpen: boolean; openRow: () => void }) => {
+  onToggleEditMode = (e: React.MouseEvent, props: QueryOperationRowRenderProps) => {
     e.stopPropagation();
     if (this.angularScope && this.angularScope.toggleEditorMode) {
       this.angularScope.toggleEditorMode();
       this.angularQueryEditor?.digest();
-      if (!isOpen) {
-        openRow();
+      if (!props.isOpen) {
+        props.onOpen();
       }
     }
   };
@@ -237,7 +240,7 @@ export class QueryEditorRow extends PureComponent<Props, State> {
     return null;
   }
 
-  renderActions = (props: { isOpen: boolean; openRow: () => void }) => {
+  renderActions = (props: QueryOperationRowRenderProps) => {
     const { query } = this.props;
     const { hasTextEditMode } = this.state;
     const isDisabled = query.hide;
@@ -264,18 +267,20 @@ export class QueryEditorRow extends PureComponent<Props, State> {
     );
   };
 
-  renderTitle = (props: { isOpen: boolean; openRow: () => void }) => {
-    const { query, dsSettings } = this.props;
+  renderTitle = (props: QueryOperationRowRenderProps) => {
+    const { query, dsSettings, onChange, queries } = this.props;
     const { datasource } = this.state;
     const isDisabled = query.hide;
 
     return (
       <QueryEditorRowTitle
         query={query}
+        queries={queries}
         inMixedMode={dsSettings.meta.mixed}
-        datasource={datasource!}
+        dataSourceName={datasource!.name}
         disabled={isDisabled}
         onClick={e => this.onToggleEditMode(e, props)}
+        onChange={onChange}
         collapsedText={!props.isOpen ? this.renderCollapsedText() : null}
       />
     );
@@ -303,7 +308,7 @@ export class QueryEditorRow extends PureComponent<Props, State> {
           id={id}
           draggable={true}
           index={index}
-          title={this.renderTitle}
+          headerElement={this.renderTitle}
           actions={this.renderActions}
           onOpen={this.onOpen}
         >

public/app/features/query/components/QueryEditorRowTitle.test.tsx

@@ -0,0 +1,67 @@
+import React from 'react';
+import { fireEvent, render, screen } from '@testing-library/react';
+import { QueryEditorRowTitle, Props } from './QueryEditorRowTitle';
+
+function renderScenario(overrides: Partial<Props>) {
+  const props: Props = {
+    query: {
+      refId: 'A',
+    },
+    queries: [
+      {
+        refId: 'A',
+      },
+      {
+        refId: 'B',
+      },
+    ],
+    dataSourceName: 'hello',
+    inMixedMode: false,
+    disabled: false,
+    onChange: jest.fn(),
+    onClick: jest.fn(),
+    collapsedText: '',
+  };
+
+  Object.assign(props, overrides);
+
+  return {
+    props,
+    renderResult: render(<QueryEditorRowTitle {...props} />),
+  };
+}
+
+describe('QueryEditorRowTitle', () => {
+  it('Can edit title', () => {
+    const scenario = renderScenario({});
+    screen.getByTestId('query-name-div').click();
+
+    const input = screen.getByTestId('query-name-input');
+    fireEvent.change(input, { target: { value: 'new name' } });
+    fireEvent.blur(input);
+
+    expect((scenario.props.onChange as any).mock.calls[0][0].refId).toBe('new name');
+  });
+
+  it('Show error when other query with same name exists', async () => {
+    renderScenario({});
+
+    screen.getByTestId('query-name-div').click();
+    const input = screen.getByTestId('query-name-input');
+    fireEvent.change(input, { target: { value: 'B' } });
+    const alert = await screen.findByRole('alert');
+
+    expect(alert.textContent).toBe('Query name already exists');
+  });
+
+  it('Show error when empty name is specified', async () => {
+    renderScenario({});
+
+    screen.getByTestId('query-name-div').click();
+    const input = screen.getByTestId('query-name-input');
+    fireEvent.change(input, { target: { value: '' } });
+    const alert = await screen.findByRole('alert');
+
+    expect(alert.textContent).toBe('An empty query name is not allowed');
+  });
+});

public/app/features/query/components/QueryEditorRowTitle.tsx

@@ -1,36 +1,125 @@
 import React from 'react';
-import { css } from 'emotion';
-import { DataQuery, DataSourceApi, GrafanaTheme } from '@grafana/data';
-import { stylesFactory, useTheme } from '@grafana/ui';
+import { css, cx } from 'emotion';
+import { DataQuery, GrafanaTheme } from '@grafana/data';
+import { Icon, Input, stylesFactory, useTheme, FieldValidationMessage } from '@grafana/ui';
 import { selectors } from '@grafana/e2e-selectors';
+import { useState } from 'react';
 
-interface QueryEditorRowTitleProps {
+export interface Props {
   query: DataQuery;
-  datasource: DataSourceApi;
+  queries: DataQuery[];
+  dataSourceName: string;
   inMixedMode?: boolean;
   disabled?: boolean;
+  onChange: (query: DataQuery) => void;
   onClick: (e: React.MouseEvent) => void;
   collapsedText: string | null;
 }
 
-export const QueryEditorRowTitle: React.FC<QueryEditorRowTitleProps> = ({
-  datasource,
+export const QueryEditorRowTitle: React.FC<Props> = ({
+  dataSourceName,
   inMixedMode,
   disabled,
   query,
+  queries,
   onClick,
+  onChange,
   collapsedText,
 }) => {
   const theme = useTheme();
   const styles = getQueryEditorRowTitleStyles(theme);
+  const [isEditing, setIsEditing] = useState<boolean>(false);
+  const [validationError, setValidationError] = useState<string | null>(null);
+
+  const onEditQuery = (event: React.SyntheticEvent) => {
+    setIsEditing(true);
+  };
+
+  const onEndEditName = (newName: string) => {
+    setIsEditing(false);
+
+    // Ignore change if invalid
+    if (validationError) {
+      setValidationError(null);
+      return;
+    }
+
+    if (query.refId !== newName) {
+      onChange({
+        ...query,
+        refId: newName,
+      });
+    }
+  };
+
+  const onInputChange = (event: React.SyntheticEvent<HTMLInputElement>) => {
+    const newName = event.currentTarget.value.trim();
+
+    if (newName.length === 0) {
+      setValidationError('An empty query name is not allowed');
+      return;
+    }
+
+    for (const otherQuery of queries) {
+      if (otherQuery !== query && newName === otherQuery.refId) {
+        setValidationError('Query name already exists');
+        return;
+      }
+    }
+
+    if (validationError) {
+      setValidationError(null);
+    }
+  };
+
+  const onEditQueryBlur = (event: React.SyntheticEvent<HTMLInputElement>) => {
+    onEndEditName(event.currentTarget.value.trim());
+  };
+
+  const onKeyDown = (event: React.KeyboardEvent) => {
+    if (event.key === 'Enter') {
+      onEndEditName((event.target as any).value);
+    }
+  };
+
+  const onFocus = (event: React.FocusEvent<HTMLInputElement>) => {
+    event.target.select();
+  };
 
   return (
     <div className={styles.wrapper}>
-      <div className={styles.refId} aria-label={selectors.components.QueryEditorRow.title(query.refId)}>
-        <span>{query.refId}</span>
-        {inMixedMode && <em className={styles.contextInfo}> ({datasource.name})</em>}
-        {disabled && <em className={styles.contextInfo}> Disabled</em>}
-      </div>
+      {!isEditing && (
+        <button
+          className={styles.queryNameWrapper}
+          aria-label={selectors.components.QueryEditorRow.title(query.refId)}
+          title="Edit query name"
+          onClick={onEditQuery}
+          data-testid="query-name-div"
+        >
+          <span className={styles.queryName}>{query.refId}</span>
+          <Icon name="pen" className={styles.queryEditIcon} size="sm" />
+        </button>
+      )}
+      {isEditing && (
+        <>
+          <Input
+            type="text"
+            defaultValue={query.refId}
+            onBlur={onEditQueryBlur}
+            autoFocus
+            onKeyDown={onKeyDown}
+            onFocus={onFocus}
+            invalid={validationError !== null}
+            onChange={onInputChange}
+            className={styles.queryNameInput}
+            data-testid="query-name-input"
+          />
+          {validationError && <FieldValidationMessage horizontal>{validationError}</FieldValidationMessage>}
+        </>
+      )}
+      {inMixedMode && <em className={styles.contextInfo}> ({dataSourceName})</em>}
+      {disabled && <em className={styles.contextInfo}> Disabled</em>}
+
       {collapsedText && (
         <div className={styles.collapsedText} onClick={onClick}>
           {collapsedText}
@@ -45,14 +134,58 @@ const getQueryEditorRowTitleStyles = stylesFactory((theme: GrafanaTheme) => {
     wrapper: css`
       display: flex;
       align-items: center;
-    `,
+      flex-grow: 1;
+      margin-left: ${theme.spacing.xs};
 
-    refId: css`
+      &:hover {
+        .query-name-wrapper {
+          background: ${theme.colors.bg3};
+          border: 1px dashed ${theme.colors.border3};
+        }
+
+        .query-name-edit-icon {
+          visibility: visible;
+        }
+      }
+    `,
+    queryNameWrapper: cx(
+      css`
+        display: flex;
+        cursor: pointer;
+        border: 1px solid transparent;
+        border-radius: ${theme.border.radius.md};
+        align-items: center;
+        padding: 0 0 0 ${theme.spacing.xs};
+        margin: 0;
+        background: transparent;
+
+        &:focus {
+          border: 2px solid ${theme.colors.formInputBorderActive};
+
+          .query-name-edit-icon {
+            visibility: visible;
+          }
+        }
+      `,
+      'query-name-wrapper'
+    ),
+    queryName: css`
       font-weight: ${theme.typography.weight.semibold};
       color: ${theme.colors.textBlue};
       cursor: pointer;
-      display: flex;
-      align-items: center;
+      overflow: hidden;
+      margin-left: ${theme.spacing.xs};
+    `,
+    queryEditIcon: cx(
+      css`
+        margin-left: ${theme.spacing.md};
+        visibility: hidden;
+      `,
+      'query-name-edit-icon'
+    ),
+    queryNameInput: css`
+      max-width: 300px;
+      margin: -4px 0;
     `,
     collapsedText: css`
       font-weight: ${theme.typography.weight.regular};

public/app/features/query/components/QueryEditorRows.tsx

@@ -30,8 +30,6 @@ export class QueryEditorRows extends PureComponent<Props> {
 
     const old = queries[index];
 
-    // ensure refId & datasource are maintained
-    query.refId = old.refId;
     if (old.datasource) {
       query.datasource = old.datasource;
     }