Graphite: Migrate to React (part 2B: migrate FunctionEditor) (#37070) (#37254)

* Add UMLs

* Add rendered diagrams

* Move QueryCtrl to flux

* Remove redundant param in the reducer

* Use named imports for lodash and fix typing for GraphiteTagOperator

* Add missing async/await

* Extract providers to a separate file

* Clean up async await

* Rename controller functions back to main

* Simplify creating actions

* Re-order controller functions

* Separate helpers from actions

* Rename vars

* Simplify helpers

* Move controller methods to state reducers

* Remove docs (they are added in design doc)

* Move actions.ts to state folder

* Add docs

* Add old methods stubs for easier review

* Check how state dependencies will be mapped

* Rename state to store

* Rename state to store

* Rewrite spec tests for Graphite Query Controller

* Update docs

* Update docs

* Add GraphiteTextEditor

* Add play button

* Add AddGraphiteFunction

* Use Segment to simplify AddGraphiteFunction

* Memoize function defs

* Fix useCallback deps

* Update public/app/plugins/datasource/graphite/state/helpers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/helpers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/helpers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Update public/app/plugins/datasource/graphite/state/providers.ts

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* Add more type definitions

* Remove submitOnClickAwayOption

This behavior is actually needed to remove parameters in functions

* Load function definitions before parsing the target on initial load

* Add button padding

* Fix loading function definitions

* Change targetChanged to updateQuery to avoid mutating state directly

It's also needed for extra refresh/runQuery execution as handleTargetChanged doesn't handle changing the raw query

* Fix updating query after adding a function

* Simplify updating function params

* Migrate function editor to react

* Simplify setting Segment Select min width

* Remove unnecessary changes to SegmentInput

* Extract view logic to a helper and update types definitions

* Clean up types

* Update FuncDef types and add tests

* Show red border for unknown functions

* Autofocus on new params

* Extract params mapping to a helper

* Split code between params and function editor

* Focus on the first param when a function is added even if it's an optional argument

* Add function editor tests

* Remove todo marker

* Fix adding new functions

* Allow empty value in selects for removing function params

* Add placeholders and fix styling

* Add more docs

* Rename .tsx -> .ts

* Update types

* Use ?? instead of || + add test for mapping options

* Use const (let is not needed here)

* Revert test name change

* Allow removing only optional params and mark additional params as optional (only the first one is required)

* Use theme.typography.bodySmall.fontSize

Co-authored-by: Giordano Ricci <me@giordanoricci.com>
(cherry picked from commit 8d7e22e1bb)

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>

packages/grafana-ui/src/components/Segment/Segment.tsx

@@ -11,6 +11,7 @@ export interface SegmentSyncProps<T> extends SegmentProps<T>, Omit<HTMLProps<HTM
   value?: T | SelectableValue<T>;
   onChange: (item: SelectableValue<T>) => void;
   options: Array<SelectableValue<T>>;
+  inputMinWidth?: number;
 }
 
 export function Segment<T>({
@@ -20,12 +21,16 @@ export function Segment<T>({
   Component,
   className,
   allowCustomValue,
+  allowEmptyValue,
   placeholder,
   disabled,
   inputMinWidth,
+  inputPlaceholder,
+  onExpandedChange,
+  autofocus = false,
   ...rest
 }: React.PropsWithChildren<SegmentSyncProps<T>>) {
-  const [Label, labelWidth, expanded, setExpanded] = useExpandableLabel(false);
+  const [Label, labelWidth, expanded, setExpanded] = useExpandableLabel(autofocus, onExpandedChange);
   const width = inputMinWidth ? Math.max(inputMinWidth, labelWidth) : labelWidth;
   const styles = useStyles(getSegmentStyles);
 
@@ -59,10 +64,12 @@ export function Segment<T>({
     <SegmentSelect
       {...rest}
       value={value && !isObject(value) ? { value } : value}
+      placeholder={inputPlaceholder}
       options={options}
       width={width}
       onClickOutside={() => setExpanded(false)}
       allowCustomValue={allowCustomValue}
+      allowEmptyValue={allowEmptyValue}
       onChange={(item) => {
         setExpanded(false);
         onChange(item);

packages/grafana-ui/src/components/Segment/SegmentAsync.tsx

@@ -15,6 +15,7 @@ export interface SegmentAsyncProps<T> extends SegmentProps<T>, Omit<HTMLProps<HT
   loadOptions: (query?: string) => Promise<Array<SelectableValue<T>>>;
   onChange: (item: SelectableValue<T>) => void;
   noOptionMessageHandler?: (state: AsyncState<Array<SelectableValue<T>>>) => string;
+  inputMinWidth?: number;
 }
 
 export function SegmentAsync<T>({
@@ -24,14 +25,18 @@ export function SegmentAsync<T>({
   Component,
   className,
   allowCustomValue,
+  allowEmptyValue,
   disabled,
   placeholder,
   inputMinWidth,
+  inputPlaceholder,
+  autofocus = false,
+  onExpandedChange,
   noOptionMessageHandler = mapStateToNoOptionsMessage,
   ...rest
 }: React.PropsWithChildren<SegmentAsyncProps<T>>) {
   const [state, fetchOptions] = useAsyncFn(loadOptions, [loadOptions]);
-  const [Label, labelWidth, expanded, setExpanded] = useExpandableLabel(false);
+  const [Label, labelWidth, expanded, setExpanded] = useExpandableLabel(autofocus, onExpandedChange);
   const width = inputMinWidth ? Math.max(inputMinWidth, labelWidth) : labelWidth;
   const styles = useStyles(getSegmentStyles);
 
@@ -66,10 +71,12 @@ export function SegmentAsync<T>({
     <SegmentSelect
       {...rest}
       value={value && !isObject(value) ? { value } : value}
+      placeholder={inputPlaceholder}
       options={state.value ?? []}
       width={width}
       noOptionsMessage={noOptionMessageHandler(state)}
       allowCustomValue={allowCustomValue}
+      allowEmptyValue={allowEmptyValue}
       onClickOutside={() => {
         setExpanded(false);
       }}

packages/grafana-ui/src/components/Segment/SegmentInput.tsx

@@ -10,7 +10,6 @@ import { useStyles } from '../../themes';
 export interface SegmentInputProps<T> extends SegmentProps<T>, Omit<HTMLProps<HTMLInputElement>, 'value' | 'onChange'> {
   value: string | number;
   onChange: (text: string | number) => void;
-  autofocus?: boolean;
 }
 
 const FONT_SIZE = 14;
@@ -21,14 +20,16 @@ export function SegmentInput<T>({
   Component,
   className,
   placeholder,
+  inputPlaceholder,
   disabled,
   autofocus = false,
+  onExpandedChange,
   ...rest
 }: React.PropsWithChildren<SegmentInputProps<T>>) {
   const ref = useRef<HTMLInputElement>(null);
   const [value, setValue] = useState<number | string>(initialValue);
   const [inputWidth, setInputWidth] = useState<number>(measureText((initialValue || '').toString(), FONT_SIZE).width);
-  const [Label, , expanded, setExpanded] = useExpandableLabel(autofocus);
+  const [Label, , expanded, setExpanded] = useExpandableLabel(autofocus, onExpandedChange);
   const styles = useStyles(getSegmentStyles);
 
   useClickAway(ref, () => {
@@ -71,6 +72,7 @@ export function SegmentInput<T>({
       autoFocus
       className={cx(`gf-form gf-form-input`, inputWidthStyle)}
       value={value}
+      placeholder={inputPlaceholder}
       onChange={(item) => {
         const { width } = measureText(item.target.value, FONT_SIZE);
         setInputWidth(width);

packages/grafana-ui/src/components/Segment/SegmentSelect.tsx

@@ -15,17 +15,25 @@ export interface Props<T> extends Omit<HTMLProps<HTMLDivElement>, 'value' | 'onC
   width: number;
   noOptionsMessage?: string;
   allowCustomValue?: boolean;
+  /**
+   * If true, empty value will be passed to onChange callback otherwise using empty value
+   * will work as canceling and using the previous value
+   */
+  allowEmptyValue?: boolean;
+  placeholder?: string;
 }
 
 /** @internal */
 export function SegmentSelect<T>({
   value,
+  placeholder = '',
   options = [],
   onChange,
   onClickOutside,
   width: widthPixels,
   noOptionsMessage = '',
   allowCustomValue = false,
+  allowEmptyValue = false,
   ...rest
 }: React.PropsWithChildren<Props<T>>) {
   const ref = useRef<HTMLDivElement>(null);
@@ -38,7 +46,7 @@ export function SegmentSelect<T>({
       <Select
         width={width}
         noOptionsMessage={noOptionsMessage}
-        placeholder=""
+        placeholder={placeholder}
         autoFocus={true}
         isOpen={true}
         onChange={onChange}
@@ -53,7 +61,7 @@ export function SegmentSelect<T>({
             // https://github.com/JedWatson/react-select/issues/188#issuecomment-279240292
             // Unfortunately there's no other way of retrieving the value (not yet) created new option
             const input = ref.current.querySelector('input[id^="react-select-"]') as HTMLInputElement;
-            if (input && input.value) {
+            if (input && (input.value || allowEmptyValue)) {
               onChange({ value: input.value as any, label: input.value });
             } else {
               onClickOutside();

packages/grafana-ui/src/components/Segment/types.ts

@@ -6,5 +6,8 @@ export interface SegmentProps<T> {
   allowCustomValue?: boolean;
   placeholder?: string;
   disabled?: boolean;
-  inputMinWidth?: number;
+  onExpandedChange?: (expanded: boolean) => void;
+  autofocus?: boolean;
+  allowEmptyValue?: boolean;
+  inputPlaceholder?: string;
 }

packages/grafana-ui/src/components/Segment/useExpandableLabel.tsx

@@ -7,12 +7,20 @@ interface LabelProps {
 }
 
 export const useExpandableLabel = (
-  initialExpanded: boolean
+  initialExpanded: boolean,
+  onExpandedChange?: (expanded: boolean) => void
 ): [React.ComponentType<LabelProps>, number, boolean, (expanded: boolean) => void] => {
   const ref = useRef<HTMLDivElement>(null);
   const [expanded, setExpanded] = useState<boolean>(initialExpanded);
   const [width, setWidth] = useState(0);
 
+  const setExpandedWrapper = (expanded: boolean) => {
+    setExpanded(expanded);
+    if (onExpandedChange) {
+      onExpandedChange(expanded);
+    }
+  };
+
   const Label: React.FC<LabelProps> = ({ Component, onClick, disabled }) => (
     <div
       ref={ref}
@@ -20,7 +28,7 @@ export const useExpandableLabel = (
         disabled
           ? undefined
           : () => {
-              setExpanded(true);
+              setExpandedWrapper(true);
               if (ref && ref.current) {
                 setWidth(ref.current.clientWidth * 1.25);
               }
@@ -34,5 +42,5 @@ export const useExpandableLabel = (
     </div>
   );
 
-  return [Label, width, expanded, setExpanded];
+  return [Label, width, expanded, setExpandedWrapper];
 };

public/app/core/angular_wrappers.ts

@@ -28,6 +28,7 @@ import QueryEditor from 'app/plugins/datasource/grafana-azure-monitor-datasource
 import { GraphiteTextEditor } from '../plugins/datasource/graphite/components/GraphiteTextEditor';
 import { PlayButton } from '../plugins/datasource/graphite/components/PlayButton';
 import { AddGraphiteFunction } from '../plugins/datasource/graphite/components/AddGraphiteFunction';
+import { GraphiteFunctionEditor } from '../plugins/datasource/graphite/components/GraphiteFunctionEditor';
 
 const { SecretFormField } = LegacyForms;
 
@@ -209,4 +210,5 @@ export function registerAngularDirectives() {
   react2AngularDirective('graphiteTextEditor', GraphiteTextEditor, ['rawQuery', 'dispatch']);
   react2AngularDirective('playButton', PlayButton, ['dispatch']);
   react2AngularDirective('addGraphiteFunction', AddGraphiteFunction, ['funcDefs', 'dispatch']);
+  react2AngularDirective('graphiteFunctionEditor', GraphiteFunctionEditor, ['func', 'dispatch']);
 }

public/app/plugins/datasource/graphite/FunctionEditor.tsx

@@ -1,16 +1,32 @@
 import React, { useRef } from 'react';
-import { PopoverController, Popover, ClickOutsideWrapper, Icon, Tooltip, useTheme } from '@grafana/ui';
+import { PopoverController, Popover, ClickOutsideWrapper, Icon, Tooltip, useStyles2 } from '@grafana/ui';
 import { FunctionEditorControls, FunctionEditorControlsProps } from './FunctionEditorControls';
 import { FuncInstance } from './gfunc';
 import { css } from '@emotion/css';
+import { GrafanaTheme2 } from '@grafana/data';
 
 interface FunctionEditorProps extends FunctionEditorControlsProps {
   func: FuncInstance;
 }
 
+const getStyles = (theme: GrafanaTheme2) => {
+  return {
+    icon: css`
+      margin-right: ${theme.spacing(0.5)};
+    `,
+    label: css({
+      fontWeight: theme.typography.fontWeightMedium,
+      fontSize: theme.typography.bodySmall.fontSize, // to match .gf-form-label
+      cursor: 'pointer',
+      display: 'inline-block',
+      paddingBottom: '2px',
+    }),
+  };
+};
+
 const FunctionEditor: React.FC<FunctionEditorProps> = ({ onMoveLeft, onMoveRight, func, ...props }) => {
   const triggerRef = useRef<HTMLSpanElement>(null);
-  const theme = useTheme();
+  const styles = useStyles2(getStyles);
 
   const renderContent = ({ updatePopperPosition }: any) => (
     <FunctionEditorControls
@@ -50,17 +66,10 @@ const FunctionEditor: React.FC<FunctionEditorProps> = ({ onMoveLeft, onMoveRight
                 }
               }}
             >
-              <span ref={triggerRef} onClick={popperProps.show ? hidePopper : showPopper} style={{ cursor: 'pointer' }}>
+              <span ref={triggerRef} onClick={popperProps.show ? hidePopper : showPopper} className={styles.label}>
                 {func.def.unknown && (
                   <Tooltip content={<TooltipContent />} placement="bottom">
-                    <Icon
+                    <Icon data-testid="warning-icon" name="exclamation-triangle" size="xs" className={styles.icon} />
-                      data-testid="warning-icon"
-                      name="exclamation-triangle"
-                      size="xs"
-                      className={css`
-                        margin-right: ${theme.spacing.xxs};
-                      `}
-                    />
                   </Tooltip>
                 )}
                 {func.def.name}

public/app/plugins/datasource/graphite/components/AddGraphiteFunction.tsx

@@ -1,8 +1,8 @@
-import React, { useCallback, useMemo } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { Button, Segment, useStyles2 } from '@grafana/ui';
 import { FuncDefs } from '../gfunc';
 import { actions } from '../state/actions';
-import { GrafanaTheme2 } from '@grafana/data';
+import { GrafanaTheme2, SelectableValue } from '@grafana/data';
 import { css, cx } from '@emotion/css';
 import { mapFuncDefsToSelectables } from './helpers';
 import { Dispatch } from 'redux';
@@ -13,21 +13,29 @@ type Props = {
 };
 
 export function AddGraphiteFunction({ dispatch, funcDefs }: Props) {
-  const onChange = useCallback(
+  const [value, setValue] = useState<SelectableValue<string> | undefined>(undefined);
-    ({ value }) => {
-      dispatch(actions.addFunction({ name: value }));
-    },
-    [dispatch]
-  );
   const styles = useStyles2(getStyles);
 
   const options = useMemo(() => mapFuncDefsToSelectables(funcDefs), [funcDefs]);
 
+  // Note: actions.addFunction will add a component that will have a dropdown or input in auto-focus
+  // (the first param of the function). This auto-focus will cause onBlur() on AddGraphiteFunction's
+  // Segment component and trigger onChange once again. (why? we call onChange if the user dismissed
+  // the dropdown, see: SegmentSelect.onCloseMenu for more details). To avoid it we need to wait for
+  // the Segment to disappear first (hence useEffect) and then dispatch the action that will add new
+  // components.
+  useEffect(() => {
+    if (value?.value !== undefined) {
+      dispatch(actions.addFunction({ name: value.value }));
+      setValue(undefined);
+    }
+  }, [value, dispatch]);
+
   return (
     <Segment
       Component={<Button icon="plus" variant="secondary" className={cx(styles.button)} />}
       options={options}
-      onChange={onChange}
+      onChange={setValue}
       inputMinWidth={150}
     ></Segment>
   );

public/app/plugins/datasource/graphite/components/FunctionParamEditor.tsx

@@ -0,0 +1,76 @@
+import React from 'react';
+import { Segment, SegmentInput, useStyles2 } from '@grafana/ui';
+import { GrafanaTheme2, SelectableValue } from '@grafana/data';
+import { css } from '@emotion/css';
+
+export type EditableParam = {
+  name: string;
+  value: string;
+  optional: boolean;
+  multiple: boolean;
+  options: Array<SelectableValue<string>>;
+};
+
+type FieldEditorProps = {
+  editableParam: EditableParam;
+  onChange: (value: string) => void;
+  onExpandedChange: (expanded: boolean) => void;
+  autofocus: boolean;
+};
+
+/**
+ * Render a function parameter with a segment dropdown for multiple options or simple input.
+ */
+export function FunctionParamEditor({ editableParam, onChange, onExpandedChange, autofocus }: FieldEditorProps) {
+  const styles = useStyles2(getStyles);
+
+  if (editableParam.options?.length > 0) {
+    return (
+      <Segment
+        autofocus={autofocus}
+        value={editableParam.value}
+        inputPlaceholder={editableParam.name}
+        className={styles.segment}
+        options={editableParam.options}
+        placeholder={' +' + editableParam.name}
+        onChange={(value) => {
+          onChange(value.value || '');
+        }}
+        onExpandedChange={onExpandedChange}
+        inputMinWidth={150}
+        allowCustomValue={true}
+        allowEmptyValue={true}
+      ></Segment>
+    );
+  } else {
+    return (
+      <SegmentInput
+        autofocus={autofocus}
+        className={styles.input}
+        value={editableParam.value || ''}
+        placeholder={' +' + editableParam.name}
+        inputPlaceholder={editableParam.name}
+        onChange={(value) => {
+          onChange(value.toString());
+        }}
+        onExpandedChange={onExpandedChange}
+        // input style
+        style={{ height: '25px', paddingTop: '2px', marginTop: '2px', paddingLeft: '4px', minWidth: '100px' }}
+      ></SegmentInput>
+    );
+  }
+}
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  segment: css({
+    margin: 0,
+    padding: 0,
+  }),
+  input: css`
+    margin: 0;
+    padding: 0;
+    input {
+      height: 25px;
+    },
+  `,
+});

public/app/plugins/datasource/graphite/components/GraphiteFunctionEditor.tsx

@@ -0,0 +1,95 @@
+import React, { useState } from 'react';
+import { HorizontalGroup, InlineLabel, useStyles2 } from '@grafana/ui';
+import { GrafanaTheme2 } from '@grafana/data';
+import { css, cx } from '@emotion/css';
+import { FuncInstance } from '../gfunc';
+import { EditableParam, FunctionParamEditor } from './FunctionParamEditor';
+import { actions } from '../state/actions';
+import { FunctionEditor } from '../FunctionEditor';
+import { mapFuncInstanceToParams } from './helpers';
+
+export type FunctionEditorProps = {
+  func: FuncInstance;
+  dispatch: (action: any) => void;
+};
+
+/**
+ * Allows editing function params and removing/moving a function (note: editing function name is not supported)
+ */
+export function GraphiteFunctionEditor({ func, dispatch }: FunctionEditorProps) {
+  const styles = useStyles2(getStyles);
+
+  // keep track of mouse over and isExpanded state to display buttons for adding optional/multiple params
+  // only when the user mouse over over the function editor OR any param editor is expanded.
+  const [mouseOver, setIsMouseOver] = useState(false);
+  const [expanded, setIsExpanded] = useState(false);
+
+  let params = mapFuncInstanceToParams(func);
+  params = params.filter((p: EditableParam, index: number) => {
+    // func.added is set for newly added functions - see autofocus below
+    return (index < func.def.params.length && !p.optional) || func.added || p.value || expanded || mouseOver;
+  });
+
+  return (
+    <div
+      className={cx(styles.container, { [styles.error]: func.def.unknown })}
+      onMouseOver={() => setIsMouseOver(true)}
+      onMouseLeave={() => setIsMouseOver(false)}
+    >
+      <HorizontalGroup spacing="none">
+        <FunctionEditor
+          func={func}
+          onMoveLeft={() => {
+            dispatch(actions.moveFunction({ func, offset: -1 }));
+          }}
+          onMoveRight={() => {
+            dispatch(actions.moveFunction({ func, offset: 1 }));
+          }}
+          onRemove={() => {
+            dispatch(actions.removeFunction({ func }));
+          }}
+        />
+        <InlineLabel className={styles.label}>(</InlineLabel>
+        {params.map((editableParam: EditableParam, index: number) => {
+          return (
+            <React.Fragment key={index}>
+              <FunctionParamEditor
+                autofocus={index === 0 && func.added}
+                editableParam={editableParam}
+                onChange={(value) => {
+                  if (value !== '' || editableParam.optional) {
+                    dispatch(actions.updateFunctionParam({ func, index, value }));
+                  }
+                  setIsExpanded(false);
+                  setIsMouseOver(false);
+                }}
+                onExpandedChange={setIsExpanded}
+              />
+              {index !== params.length - 1 ? ',' : ''}
+            </React.Fragment>
+          );
+        })}
+        <InlineLabel className={styles.label}>)</InlineLabel>
+      </HorizontalGroup>
+    </div>
+  );
+}
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  container: css({
+    backgroundColor: theme.colors.background.secondary,
+    borderRadius: theme.shape.borderRadius(),
+    marginRight: theme.spacing(0.5),
+    padding: `0 ${theme.spacing(1)}`,
+  }),
+  error: css`
+    border: 1px solid ${theme.colors.error.main};
+  `,
+  label: css({
+    padding: 0,
+    margin: 0,
+  }),
+  button: css({
+    padding: theme.spacing(0.5),
+  }),
+});

public/app/plugins/datasource/graphite/components/helpers.test.ts

@@ -0,0 +1,252 @@
+import { mapFuncDefsToSelectables, mapFuncInstanceToParams } from './helpers';
+import { FuncDef, FuncDefs, FuncInstance } from '../gfunc';
+import { EditableParam } from './FunctionParamEditor';
+
+function createFunctionInstance(funcDef: FuncDef, currentParams: string[]): FuncInstance {
+  let funcInstance: FuncInstance = new FuncInstance(funcDef);
+  funcInstance.params = currentParams;
+  return funcInstance;
+}
+
+describe('Graphite components helpers', () => {
+  it('converts function definitions to selectable options', function () {
+    const functionDefs: FuncDefs = {
+      functionA1: { name: 'functionA1', category: 'A', params: [], defaultParams: [] },
+      functionB1: { name: 'functionB1', category: 'B', params: [], defaultParams: [] },
+      functionA2: { name: 'functionA2', category: 'A', params: [], defaultParams: [] },
+      functionB2: { name: 'functionB2', category: 'B', params: [], defaultParams: [] },
+    };
+    const options = mapFuncDefsToSelectables(functionDefs);
+
+    expect(options).toMatchObject([
+      {
+        label: 'A',
+        options: [
+          { label: 'functionA1', value: 'functionA1' },
+          { label: 'functionA2', value: 'functionA2' },
+        ],
+      },
+      {
+        label: 'B',
+        options: [
+          { label: 'functionB1', value: 'functionB1' },
+          { label: 'functionB2', value: 'functionB2' },
+        ],
+      },
+    ]);
+  });
+
+  describe('mapFuncInstanceToParams', () => {
+    let funcDef: FuncDef;
+
+    function assertFunctionInstance(definition: FuncDef, params: string[], expected: EditableParam[]): void {
+      expect(mapFuncInstanceToParams(createFunctionInstance(definition, params))).toMatchObject(expected);
+    }
+
+    it('converts param options to selectable options', () => {
+      const funcDef = {
+        name: 'functionA1',
+        category: 'A',
+        params: [{ name: 'foo', type: 'any', optional: false, multiple: false, options: ['foo', 2] }],
+        defaultParams: [],
+      };
+
+      assertFunctionInstance(
+        funcDef,
+        [],
+        [
+          {
+            name: 'foo',
+            multiple: false,
+            optional: false,
+            options: [
+              { label: 'foo', value: 'foo' },
+              { label: '2', value: '2' },
+            ],
+            value: '',
+          },
+        ]
+      );
+    });
+
+    describe('when all parameters are required and no multiple values are allowed', () => {
+      beforeEach(() => {
+        funcDef = {
+          name: 'allRequiredNoMultiple',
+          category: 'A',
+          params: [
+            { name: 'a', type: 'any', optional: false, multiple: false },
+            { name: 'b', type: 'any', optional: false, multiple: false },
+          ],
+          defaultParams: [],
+        };
+      });
+
+      it('creates required params', () => {
+        assertFunctionInstance(
+          funcDef,
+          [],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: '' },
+            { name: 'b', multiple: false, optional: false, options: [], value: '' },
+          ]
+        );
+      });
+
+      it('fills in provided parameters', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: false, optional: false, options: [], value: 'b' },
+          ]
+        );
+      });
+    });
+
+    describe('when all parameters are required and multiple values are allowed', () => {
+      beforeEach(() => {
+        funcDef = {
+          name: 'allRequiredWithMultiple',
+          category: 'A',
+          params: [
+            { name: 'a', type: 'any', optional: false, multiple: false },
+            { name: 'b', type: 'any', optional: false, multiple: true },
+          ],
+          defaultParams: [],
+        };
+      });
+
+      it('does not add extra param to add multiple values if not all params are filled in', () => {
+        assertFunctionInstance(
+          funcDef,
+          [],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: '' },
+            { name: 'b', multiple: true, optional: false, options: [], value: '' },
+          ]
+        );
+
+        assertFunctionInstance(
+          funcDef,
+          ['a'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: true, optional: false, options: [], value: '' },
+          ]
+        );
+      });
+
+      it('marks additional params as optional (only first one is required)', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b', 'b2'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: true, optional: false, options: [], value: 'b' },
+            { name: 'b', multiple: true, optional: true, options: [], value: 'b2' },
+            { name: 'b', multiple: true, optional: true, options: [], value: '' },
+          ]
+        );
+      });
+
+      it('adds an extra param to allo adding multiple values if all params are filled in', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: true, optional: false, options: [], value: 'b' },
+            { name: 'b', multiple: true, optional: true, options: [], value: '' },
+          ]
+        );
+      });
+    });
+
+    describe('when there are optional parameters but no multiple values are allowed', () => {
+      beforeEach(() => {
+        funcDef = {
+          name: 'twoOptionalNoMultiple',
+          category: 'A',
+          params: [
+            { name: 'a', type: 'any', optional: false, multiple: false },
+            { name: 'b', type: 'any', optional: false, multiple: false },
+            { name: 'c', type: 'any', optional: true, multiple: false },
+            { name: 'd', type: 'any', optional: true, multiple: false },
+          ],
+          defaultParams: [],
+        };
+      });
+
+      it('creates non-required parameters', () => {
+        assertFunctionInstance(
+          funcDef,
+          [],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: '' },
+            { name: 'b', multiple: false, optional: false, options: [], value: '' },
+            { name: 'c', multiple: false, optional: true, options: [], value: '' },
+            { name: 'd', multiple: false, optional: true, options: [], value: '' },
+          ]
+        );
+      });
+
+      it('fills in provided parameters', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b', 'c', 'd'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: false, optional: false, options: [], value: 'b' },
+            { name: 'c', multiple: false, optional: true, options: [], value: 'c' },
+            { name: 'd', multiple: false, optional: true, options: [], value: 'd' },
+          ]
+        );
+      });
+    });
+
+    describe('when there are optional parameters and multiple values are allowed', () => {
+      beforeEach(() => {
+        funcDef = {
+          name: 'twoOptionalWithMultiple',
+          category: 'A',
+          params: [
+            { name: 'a', type: 'any', optional: false, multiple: false },
+            { name: 'b', type: 'any', optional: false, multiple: false },
+            { name: 'c', type: 'any', optional: true, multiple: false },
+            { name: 'd', type: 'any', optional: true, multiple: true },
+          ],
+          defaultParams: [],
+        };
+      });
+
+      it('does not add extra param to add multiple values if not all params are filled in', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b', 'c'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: false, optional: false, options: [], value: 'b' },
+            { name: 'c', multiple: false, optional: true, options: [], value: 'c' },
+            { name: 'd', multiple: true, optional: true, options: [], value: '' },
+          ]
+        );
+      });
+
+      it('adds an extra param to add multiple values if all params are filled in', () => {
+        assertFunctionInstance(
+          funcDef,
+          ['a', 'b', 'c', 'd'],
+          [
+            { name: 'a', multiple: false, optional: false, options: [], value: 'a' },
+            { name: 'b', multiple: false, optional: false, options: [], value: 'b' },
+            { name: 'c', multiple: false, optional: true, options: [], value: 'c' },
+            { name: 'd', multiple: true, optional: true, options: [], value: 'd' },
+            { name: 'd', multiple: true, optional: true, options: [], value: '' },
+          ]
+        );
+      });
+    });
+  });
+});

public/app/plugins/datasource/graphite/components/helpers.test.tsx

@@ -1,31 +0,0 @@
-import { mapFuncDefsToSelectables } from './helpers';
-import { FuncDefs } from '../gfunc';
-
-describe('Graphite components helpers', () => {
-  it('converts function definitions to selectable options', function () {
-    const functionDefs: FuncDefs = {
-      functionA1: { name: 'functionA1', category: 'A', params: [], defaultParams: [] },
-      functionB1: { name: 'functionB1', category: 'B', params: [], defaultParams: [] },
-      functionA2: { name: 'functionA2', category: 'A', params: [], defaultParams: [] },
-      functionB2: { name: 'functionB2', category: 'B', params: [], defaultParams: [] },
-    };
-    const options = mapFuncDefsToSelectables(functionDefs);
-
-    expect(options).toMatchObject([
-      {
-        label: 'A',
-        options: [
-          { label: 'functionA1', value: 'functionA1' },
-          { label: 'functionA2', value: 'functionA2' },
-        ],
-      },
-      {
-        label: 'B',
-        options: [
-          { label: 'functionB1', value: 'functionB1' },
-          { label: 'functionB2', value: 'functionB2' },
-        ],
-      },
-    ]);
-  });
-});

public/app/plugins/datasource/graphite/components/helpers.ts

@@ -0,0 +1,62 @@
+import { FuncDefs, FuncInstance, ParamDef } from '../gfunc';
+import { forEach, sortBy } from 'lodash';
+import { SelectableValue } from '@grafana/data';
+import { EditableParam } from './FunctionParamEditor';
+
+export function mapFuncDefsToSelectables(funcDefs: FuncDefs): Array<SelectableValue<string>> {
+  const categories: any = {};
+
+  forEach(funcDefs, (funcDef) => {
+    if (!funcDef.category) {
+      return;
+    }
+    if (!categories[funcDef.category]) {
+      categories[funcDef.category] = { label: funcDef.category, value: funcDef.category, options: [] };
+    }
+    categories[funcDef.category].options.push({
+      label: funcDef.name,
+      value: funcDef.name,
+    });
+  });
+
+  return sortBy(categories, 'label');
+}
+
+function createEditableParam(paramDef: ParamDef, additional: boolean, value?: string | number): EditableParam {
+  return {
+    name: paramDef.name,
+    value: value?.toString() || '',
+    optional: !!paramDef.optional || additional, // only first param is required when multiple are allowed
+    multiple: !!paramDef.multiple,
+    options:
+      paramDef.options?.map((option: string | number) => ({
+        value: option.toString(),
+        label: option.toString(),
+      })) ?? [],
+  };
+}
+
+/**
+ * Create a list of params that can be edited in the function editor.
+ */
+export function mapFuncInstanceToParams(func: FuncInstance): EditableParam[] {
+  // list of required parameters (from func.def)
+  const params: EditableParam[] = func.def.params.map((paramDef: ParamDef, index: number) =>
+    createEditableParam(paramDef, false, func.params[index])
+  );
+
+  // list of additional (multiple or optional) params entered by the user
+  while (params.length < func.params.length) {
+    const paramDef = func.def.params[func.def.params.length - 1];
+    const value = func.params[params.length];
+    params.push(createEditableParam(paramDef, true, value));
+  }
+
+  // extra "fake" param to allow adding more multiple values at the end
+  if (params.length && params[params.length - 1].value && params[params.length - 1]?.multiple) {
+    const paramDef = func.def.params[func.def.params.length - 1];
+    params.push(createEditableParam(paramDef, true, ''));
+  }
+
+  return params;
+}

public/app/plugins/datasource/graphite/components/helpers.tsx

@@ -1,22 +0,0 @@
-import { FuncDefs } from '../gfunc';
-import { forEach, sortBy } from 'lodash';
-import { SelectableValue } from '@grafana/data';
-
-export function mapFuncDefsToSelectables(funcDefs: FuncDefs): Array<SelectableValue<string>> {
-  const categories: any = {};
-
-  forEach(funcDefs, (funcDef) => {
-    if (!funcDef.category) {
-      return;
-    }
-    if (!categories[funcDef.category]) {
-      categories[funcDef.category] = { label: funcDef.category, value: funcDef.category, options: [] };
-    }
-    categories[funcDef.category].options.push({
-      label: funcDef.name,
-      value: funcDef.name,
-    });
-  });
-
-  return sortBy(categories, 'label');
-}

public/app/plugins/datasource/graphite/func_editor.ts

@@ -1,256 +0,0 @@
-import { assign, clone, each, last, map, partial } from 'lodash';
-import $ from 'jquery';
-import coreModule from 'app/core/core_module';
-import { TemplateSrv } from 'app/features/templating/template_srv';
-import { actions } from './state/actions';
-
-/** @ngInject */
-export function graphiteFuncEditor($compile: any, templateSrv: TemplateSrv) {
-  const funcSpanTemplate = `
-    <function-editor
-      func="func"
-      onRemove="ctrl.handleRemoveFunction"
-      onMoveLeft="ctrl.handleMoveLeft"
-      onMoveRight="ctrl.handleMoveRight">
-    </function-editor>
-    <span>(</span>
-  `;
-  const paramTemplate =
-    '<input type="text" style="display:none"' + ' class="input-small tight-form-func-param"></input>';
-
-  return {
-    restrict: 'A',
-    link: function postLink($scope: any, elem: JQuery) {
-      const $funcLink = $(funcSpanTemplate);
-      const ctrl = $scope.ctrl;
-      const func = $scope.func;
-      let scheduledRelink = false;
-      let paramCountAtLink = 0;
-      let cancelBlur: any = null;
-
-      ctrl.handleRemoveFunction = (func: any) => {
-        ctrl.dispatch(actions.removeFunction({ func }));
-      };
-
-      ctrl.handleMoveLeft = (func: any) => {
-        ctrl.dispatch(actions.moveFunction({ func, offset: -1 }));
-      };
-
-      ctrl.handleMoveRight = (func: any) => {
-        ctrl.dispatch(actions.moveFunction({ func, offset: 1 }));
-      };
-
-      function clickFuncParam(this: any, paramIndex: any) {
-        const $link = $(this);
-        const $comma = $link.prev('.comma');
-        const $input = $link.next();
-
-        $input.val(func.params[paramIndex]);
-
-        $comma.removeClass('query-part__last');
-        $link.hide();
-        $input.show();
-        $input.focus();
-        $input.select();
-
-        const typeahead = $input.data('typeahead');
-        if (typeahead) {
-          $input.val('');
-          typeahead.lookup();
-        }
-      }
-
-      function scheduledRelinkIfNeeded() {
-        if (paramCountAtLink === func.params.length) {
-          return;
-        }
-
-        if (!scheduledRelink) {
-          scheduledRelink = true;
-          setTimeout(() => {
-            relink();
-            scheduledRelink = false;
-          }, 200);
-        }
-      }
-
-      function paramDef(index: number) {
-        if (index < func.def.params.length) {
-          return func.def.params[index];
-        }
-        if ((last(func.def.params) as any).multiple) {
-          return assign({}, last(func.def.params), { optional: true });
-        }
-        return {};
-      }
-
-      function switchToLink(inputElem: HTMLElement, paramIndex: any) {
-        const $input = $(inputElem);
-
-        clearTimeout(cancelBlur);
-        cancelBlur = null;
-
-        const $link = $input.prev();
-        const $comma = $link.prev('.comma');
-        const newValue = $input.val() as any;
-
-        // remove optional empty params
-        if (newValue !== '' || paramDef(paramIndex).optional) {
-          func.updateParam(newValue, paramIndex);
-          $link.html(newValue ? templateSrv.highlightVariablesAsHtml(newValue) : '&nbsp;');
-        }
-
-        scheduledRelinkIfNeeded();
-
-        $scope.$apply(() => {
-          // WIP: at the moment function params are mutated directly by func_editor
-          // after migrating to react it will be done by passing param value to
-          // updateFunctionParam action
-          ctrl.dispatch(actions.updateFunctionParam({ func }));
-        });
-
-        if ($link.hasClass('query-part__last') && newValue === '') {
-          $comma.addClass('query-part__last');
-        } else {
-          $link.removeClass('query-part__last');
-        }
-
-        $input.hide();
-        $link.show();
-      }
-
-      // this = input element
-      function inputBlur(this: any, paramIndex: any) {
-        const inputElem = this;
-        // happens long before the click event on the typeahead options
-        // need to have long delay because the blur
-        cancelBlur = setTimeout(() => {
-          switchToLink(inputElem, paramIndex);
-        }, 200);
-      }
-
-      function inputKeyPress(this: any, paramIndex: any, e: any) {
-        if (e.which === 13) {
-          $(this).blur();
-        }
-      }
-
-      function inputKeyDown(this: any) {
-        this.style.width = (3 + this.value.length) * 8 + 'px';
-      }
-
-      function addTypeahead($input: any, paramIndex: any) {
-        $input.attr('data-provide', 'typeahead');
-
-        let options = paramDef(paramIndex).options;
-        if (paramDef(paramIndex).type === 'int') {
-          options = map(options, (val) => {
-            return val.toString();
-          });
-        }
-
-        $input.typeahead({
-          source: options,
-          minLength: 0,
-          items: 20,
-          updater: (value: any) => {
-            $input.val(value);
-            switchToLink($input[0], paramIndex);
-            return value;
-          },
-        });
-
-        const typeahead = $input.data('typeahead');
-        typeahead.lookup = function () {
-          this.query = this.$element.val() || '';
-          return this.process(this.source);
-        };
-      }
-
-      function addElementsAndCompile() {
-        $funcLink.appendTo(elem);
-
-        if (func.def.unknown) {
-          elem.addClass('unknown-function');
-        }
-
-        const defParams: any = clone(func.def.params);
-        const lastParam: any = last(func.def.params);
-
-        while (func.params.length >= defParams.length && lastParam && lastParam.multiple) {
-          defParams.push(assign({}, lastParam, { optional: true }));
-        }
-
-        each(defParams, (param: any, index: number) => {
-          if (param.optional && func.params.length < index) {
-            return false;
-          }
-
-          let paramValue = templateSrv.highlightVariablesAsHtml(func.params[index]);
-          const hasValue = paramValue !== null && paramValue !== undefined && paramValue !== '';
-          const last = index >= func.params.length - 1 && param.optional && !hasValue;
-          let linkClass = 'query-part__link';
-
-          if (last) {
-            linkClass += ' query-part__last';
-          }
-
-          if (last && param.multiple) {
-            paramValue = '+';
-          } else if (!hasValue) {
-            // for params with no value default to param name
-            paramValue = param.name;
-            linkClass += ' query-part__link--no-value';
-          }
-
-          if (index > 0) {
-            $('<span class="comma' + (last ? ' query-part__last' : '') + '">, </span>').appendTo(elem);
-          }
-
-          const $paramLink = $(`<a ng-click="" class="${linkClass}">${paramValue}</a>`);
-          const $input = $(paramTemplate);
-          $input.attr('placeholder', param.name);
-
-          paramCountAtLink++;
-
-          $paramLink.appendTo(elem);
-          $input.appendTo(elem);
-
-          $input.blur(partial(inputBlur, index));
-          $input.keyup(inputKeyDown);
-          $input.keypress(partial(inputKeyPress, index));
-          $paramLink.click(partial(clickFuncParam, index));
-
-          if (param.options) {
-            addTypeahead($input, index);
-          }
-
-          return true;
-        });
-
-        $('<span>)</span>').appendTo(elem);
-
-        $compile(elem.contents())($scope);
-      }
-
-      function ifJustAddedFocusFirstParam() {
-        if ($scope.func.added) {
-          $scope.func.added = false;
-          setTimeout(() => {
-            elem.find('.query-part__link').first().click();
-          }, 10);
-        }
-      }
-
-      function relink() {
-        elem.children().remove();
-        addElementsAndCompile();
-        ifJustAddedFocusFirstParam();
-      }
-
-      relink();
-    },
-  };
-}
-
-coreModule.directive('graphiteFuncEditor', graphiteFuncEditor);

public/app/plugins/datasource/graphite/gfunc.ts

@@ -2,7 +2,7 @@ import { assign, each, filter, forEach, get, includes, isString, last, map, toSt
 import { isVersionGtOrEq } from 'app/core/utils/version';
 import { InterpolateFunction } from '@grafana/data';
 
-type ParamDef = {
+export type ParamDef = {
   name: string;
   type: string;
   options?: Array<string | number>;
@@ -990,6 +990,10 @@ export class FuncInstance {
   def: FuncDef;
   params: Array<string | number>;
   text: any;
+  /**
+   * True if this function was just added and not edited yet. It's used to focus on first
+   * function param to edit it straight away after adding a function.
+   */
   declare added: boolean;
   /**
    * Hidden functions are not displayed in UI but available in text editor

public/app/plugins/datasource/graphite/graphite_query.ts

@@ -192,6 +192,9 @@ export default class GraphiteQuery {
         this.updateRenderedTarget(target, targets);
       }
     }
+
+    // clean-up added param
+    this.functions.forEach((func) => (func.added = false));
   }
 
   updateRenderedTarget(target: { refId: string | number; target: any; targetFull: any }, targets: any) {

public/app/plugins/datasource/graphite/partials/query.editor.html

@@ -66,7 +66,7 @@
       </div>
 
       <div ng-repeat="func in ctrl.state.queryModel.functions" class="gf-form">
-        <span graphite-func-editor class="gf-form-label query-part" ng-hide="func.hidden"></span>
+        <graphite-function-editor func="func" dispatch="ctrl.dispatch" ng-hide="func.hidden"></graphite-function-editor>
       </div>
 
       <div class="gf-form dropdown">

public/app/plugins/datasource/graphite/query_ctrl.ts

@@ -1,5 +1,3 @@
-import './func_editor';
-
 import GraphiteQuery from './graphite_query';
 import { QueryCtrl } from 'app/plugins/sdk';
 import { auto } from 'angular';

public/app/plugins/datasource/graphite/state/actions.ts

@@ -23,8 +23,7 @@ const unpause = createAction('unpause');
 const addFunction = createAction<{ name: string }>('add-function');
 const removeFunction = createAction<{ func: FuncInstance }>('remove-function');
 const moveFunction = createAction<{ func: FuncInstance; offset: number }>('move-function');
-// TODO: at the moment parameters are modified directly, new value of the param will be passed in the action
+const updateFunctionParam = createAction<{ func: FuncInstance; index: number; value: string }>('change-function-param');
-const updateFunctionParam = createAction<{ func: FuncInstance }>('update-function-param');
 
 // Text editor
 const updateQuery = createAction<{ query: string }>('update-query');

public/app/plugins/datasource/graphite/state/store.ts

@@ -146,6 +146,8 @@ const reducer = async (action: Action, state: GraphiteQueryEditorState): Promise
     handleTargetChanged(state);
   }
   if (actions.updateFunctionParam.match(action)) {
+    const { func, index, value } = action.payload;
+    func.updateParam(value, index);
     handleTargetChanged(state);
   }
   if (actions.updateQuery.match(action)) {

public/sass/components/_query_part.scss

@@ -1,10 +1,6 @@
 .query-part {
   background-color: $tight-form-func-bg;
 
-  &.unknown-function {
-    border: 1px solid $error-text-color;
-  }
-
   &.show-function-controls {
     padding-top: 5px;
     min-width: 100px;