[8.1] [Controls] Improve controls management UX (#127524) (#128213)

* [Controls] Improve controls management UX (#127524) * Move control type selection in to flyout * Set default icon type if getIconType undefined * Fix create control functional tests * Fix factories for multiple types * Show only selected type icon when editing * Add optional tooltip support * Rename promise variable * Fix imports * Fix nits * Edit tooltip text for options list control (cherry picked from commit c9dfe16725) # Conflicts: # src/plugins/controls/public/control_group/embeddable/control_group_container.tsx * Fix merge conflicts
2025-04-23 17:28:26 -04:00 · 2022-03-21 18:27:05 -06:00 · 2022-03-21 18:27:05 -06:00 · 47da47faae
commit 47da47faae
parent e179504718
7 changed files with 162 additions and 155 deletions
--- a/src/plugins/controls/public/control_group/control_group_strings.ts
+++ b/src/plugins/controls/public/control_group/control_group_strings.ts
@ -48,6 +48,10 @@ export const ControlGroupStrings = {
      i18n.translate('controls.controlGroup.manageControl.titleInputTitle', {
        defaultMessage: 'Title',
      }),
+    getControlTypeTitle: () =>
+      i18n.translate('controls.controlGroup.manageControl.controlTypesTitle', {
+        defaultMessage: 'Control type',
+      }),
    getWidthInputTitle: () =>
      i18n.translate('controls.controlGroup.manageControl.widthInputTitle', {
        defaultMessage: 'Control size',
--- a/src/plugins/controls/public/control_group/editor/control_editor.tsx
+++ b/src/plugins/controls/public/control_group/editor/control_editor.tsx
@ -29,6 +29,10 @@ import {
  EuiForm,
  EuiButtonEmpty,
  EuiSpacer,
+  EuiKeyPadMenu,
+  EuiKeyPadMenuItem,
+  EuiIcon,
+  EuiToolTip,
 } from '@elastic/eui';

 import { ControlGroupStrings } from '../control_group_strings';
@ -39,14 +43,15 @@ import {
  IEditableControlFactory,
 } from '../../types';
 import { CONTROL_WIDTH_OPTIONS } from './editor_constants';
+import { pluginServices } from '../../services';
+import { EmbeddableFactoryDefinition } from '../../../../embeddable/public';

 interface EditControlProps {
-  factory: IEditableControlFactory;
  embeddable?: ControlEmbeddable;
-  width: ControlWidth;
  isCreate: boolean;
  title?: string;
-  onSave: () => void;
+  width: ControlWidth;
+  onSave: (type: string) => void;
  onCancel: () => void;
  removeControl?: () => void;
  updateTitle: (title?: string) => void;
@ -55,25 +60,75 @@ interface EditControlProps {
 }

 export const ControlEditor = ({
-  onTypeEditorChange,
+  embeddable,
+  isCreate,
+  title,
+  width,
+  onSave,
+  onCancel,
  removeControl,
  updateTitle,
  updateWidth,
-  embeddable,
-  isCreate,
-  onCancel,
-  factory,
-  onSave,
-  title,
-  width,
+  onTypeEditorChange,
 }: EditControlProps) => {
+  const { controls } = pluginServices.getServices();
+  const { getControlTypes, getControlFactory } = controls;
+
+  const [selectedType, setSelectedType] = useState(
+    !isCreate && embeddable ? embeddable.type : getControlTypes()[0]
+  );
+  const [defaultTitle, setDefaultTitle] = useState<string>();
  const [currentTitle, setCurrentTitle] = useState(title);
  const [currentWidth, setCurrentWidth] = useState(width);
-
  const [controlEditorValid, setControlEditorValid] = useState(false);
-  const [defaultTitle, setDefaultTitle] = useState<string>();

-  const ControlTypeEditor = factory.controlEditorComponent;
+  const getControlTypeEditor = (type: string) => {
+    const factory = getControlFactory(type);
+    const ControlTypeEditor = (factory as IEditableControlFactory).controlEditorComponent;
+    return ControlTypeEditor ? (
+      <ControlTypeEditor
+        onChange={onTypeEditorChange}
+        setValidState={setControlEditorValid}
+        initialInput={embeddable?.getInput()}
+        setDefaultTitle={(newDefaultTitle) => {
+          if (!currentTitle || currentTitle === defaultTitle) {
+            setCurrentTitle(newDefaultTitle);
+            updateTitle(newDefaultTitle);
+          }
+          setDefaultTitle(newDefaultTitle);
+        }}
+      />
+    ) : null;
+  };
+
+  const getTypeButtons = (controlTypes: string[]) => {
+    return controlTypes.map((type) => {
+      const factory = getControlFactory(type);
+      const icon = (factory as EmbeddableFactoryDefinition).getIconType?.();
+      const tooltip = (factory as EmbeddableFactoryDefinition).getDescription?.();
+      const menuPadItem = (
+        <EuiKeyPadMenuItem
+          id={`createControlButton_${type}`}
+          data-test-subj={`create-${type}-control`}
+          label={(factory as EmbeddableFactoryDefinition).getDisplayName()}
+          isSelected={selectedType === type}
+          onClick={() => {
+            setSelectedType(type);
+          }}
+        >
+          <EuiIcon type={!icon || icon === 'empty' ? 'controlsHorizontal' : icon} size="l" />
+        </EuiKeyPadMenuItem>
+      );
+
+      return tooltip ? (
+        <EuiToolTip content={tooltip} position="bottom">
+          {menuPadItem}
+        </EuiToolTip>
+      ) : (
+        menuPadItem
+      );
+    });
+  };

  return (
    <>
@ -88,58 +143,53 @@ export const ControlEditor = ({
      </EuiFlyoutHeader>
      <EuiFlyoutBody data-test-subj="control-editor-flyout">
        <EuiForm>
-          <EuiSpacer size="l" />
-          {ControlTypeEditor && (
-            <ControlTypeEditor
-              onChange={onTypeEditorChange}
-              setValidState={setControlEditorValid}
-              initialInput={embeddable?.getInput()}
-              setDefaultTitle={(newDefaultTitle) => {
-                if (!currentTitle || currentTitle === defaultTitle) {
-                  setCurrentTitle(newDefaultTitle);
-                  updateTitle(newDefaultTitle);
-                }
-                setDefaultTitle(newDefaultTitle);
-              }}
-            />
-          )}
-          <EuiFormRow label={ControlGroupStrings.manageControl.getTitleInputTitle()}>
-            <EuiFieldText
-              data-test-subj="control-editor-title-input"
-              placeholder={defaultTitle}
-              value={currentTitle}
-              onChange={(e) => {
-                updateTitle(e.target.value || defaultTitle);
-                setCurrentTitle(e.target.value);
-              }}
-            />
+          <EuiFormRow label={ControlGroupStrings.manageControl.getControlTypeTitle()}>
+            <EuiKeyPadMenu>
+              {isCreate ? getTypeButtons(getControlTypes()) : getTypeButtons([selectedType])}
+            </EuiKeyPadMenu>
          </EuiFormRow>
-          <EuiFormRow label={ControlGroupStrings.manageControl.getWidthInputTitle()}>
-            <EuiButtonGroup
-              color="primary"
-              legend={ControlGroupStrings.management.controlWidth.getWidthSwitchLegend()}
-              options={CONTROL_WIDTH_OPTIONS}
-              idSelected={currentWidth}
-              onChange={(newWidth: string) => {
-                setCurrentWidth(newWidth as ControlWidth);
-                updateWidth(newWidth as ControlWidth);
-              }}
-            />
-          </EuiFormRow>
-          <EuiSpacer size="l" />
-          {removeControl && (
-            <EuiButtonEmpty
-              aria-label={`delete-${title}`}
-              iconType="trash"
-              flush="left"
-              color="danger"
-              onClick={() => {
-                onCancel();
-                removeControl();
-              }}
-            >
-              {ControlGroupStrings.management.getDeleteButtonTitle()}
-            </EuiButtonEmpty>
+          {selectedType && (
+            <>
+              {getControlTypeEditor(selectedType)}
+              <EuiFormRow label={ControlGroupStrings.manageControl.getTitleInputTitle()}>
+                <EuiFieldText
+                  data-test-subj="control-editor-title-input"
+                  placeholder={defaultTitle}
+                  value={currentTitle}
+                  onChange={(e) => {
+                    updateTitle(e.target.value || defaultTitle);
+                    setCurrentTitle(e.target.value);
+                  }}
+                />
+              </EuiFormRow>
+              <EuiFormRow label={ControlGroupStrings.manageControl.getWidthInputTitle()}>
+                <EuiButtonGroup
+                  color="primary"
+                  legend={ControlGroupStrings.management.controlWidth.getWidthSwitchLegend()}
+                  options={CONTROL_WIDTH_OPTIONS}
+                  idSelected={currentWidth}
+                  onChange={(newWidth: string) => {
+                    setCurrentWidth(newWidth as ControlWidth);
+                    updateWidth(newWidth as ControlWidth);
+                  }}
+                />
+              </EuiFormRow>
+              <EuiSpacer size="l" />
+              {removeControl && (
+                <EuiButtonEmpty
+                  aria-label={`delete-${title}`}
+                  iconType="trash"
+                  flush="left"
+                  color="danger"
+                  onClick={() => {
+                    onCancel();
+                    removeControl();
+                  }}
+                >
+                  {ControlGroupStrings.management.getDeleteButtonTitle()}
+                </EuiButtonEmpty>
+              )}
+            </>
          )}
        </EuiForm>
      </EuiFlyoutBody>
@ -150,9 +200,7 @@ export const ControlEditor = ({
              aria-label={`cancel-${title}`}
              data-test-subj="control-editor-cancel"
              iconType="cross"
-              onClick={() => {
-                onCancel();
-              }}
+              onClick={() => onCancel()}
            >
              {ControlGroupStrings.manageControl.getCancelTitle()}
            </EuiButtonEmpty>
@ -164,7 +212,7 @@ export const ControlEditor = ({
              iconType="check"
              color="primary"
              disabled={!controlEditorValid}
-              onClick={() => onSave()}
+              onClick={() => onSave(selectedType)}
            >
              {ControlGroupStrings.manageControl.getSaveChangesTitle()}
            </EuiButton>
--- a/src/plugins/controls/public/control_group/editor/create_control.tsx
+++ b/src/plugins/controls/public/control_group/editor/create_control.tsx
@ -6,22 +6,15 @@
 * Side Public License, v 1.
 */

-import {
-  EuiButton,
-  EuiButtonIconColor,
-  EuiContextMenuItem,
-  EuiContextMenuPanel,
-  EuiPopover,
-} from '@elastic/eui';
-import React, { useState, ReactElement } from 'react';
+import { EuiButton, EuiContextMenuItem } from '@elastic/eui';
+import React from 'react';

 import { pluginServices } from '../../services';
 import { ControlEditor } from './control_editor';
 import { OverlayRef } from '../../../../../core/public';
 import { DEFAULT_CONTROL_WIDTH } from './editor_constants';
 import { ControlGroupStrings } from '../control_group_strings';
-import { EmbeddableFactoryNotFoundError } from '../../../../embeddable/public';
-import { ControlWidth, IEditableControlFactory, ControlInput } from '../../types';
+import { ControlWidth, ControlInput, IEditableControlFactory } from '../../types';
 import { toMountPoint } from '../../../../kibana_react/public';

 export type CreateControlButtonTypes = 'toolbar' | 'callout';
@ -33,6 +26,11 @@ export interface CreateControlButtonProps {
  closePopover?: () => void;
 }

+interface CreateControlResult {
+  type: string;
+  controlInput: Omit<ControlInput, 'id'>;
+}
+
 export const CreateControlButton = ({
  defaultControlWidth,
  updateDefaultWidth,
@ -44,14 +42,11 @@ export const CreateControlButton = ({
  const { overlays, controls } = pluginServices.getServices();
  const { getControlTypes, getControlFactory } = controls;
  const { openFlyout, openConfirm } = overlays;
-  const [isControlTypePopoverOpen, setIsControlTypePopoverOpen] = useState(false);

-  const createNewControl = async (type: string) => {
+  const createNewControl = async () => {
    const PresentationUtilProvider = pluginServices.getContextProvider();
-    const factory = getControlFactory(type);
-    if (!factory) throw new EmbeddableFactoryNotFoundError(type);

-    const initialInputPromise = new Promise<Omit<ControlInput, 'id'>>((resolve, reject) => {
+    const initialInputPromise = new Promise<CreateControlResult>((resolve, reject) => {
      let inputToReturn: Partial<ControlInput> = {};

      const onCancel = (ref: OverlayRef) => {
@ -73,28 +68,26 @@ export const CreateControlButton = ({
        });
      };

-      const editableFactory = factory as IEditableControlFactory;
-
      const flyoutInstance = openFlyout(
        toMountPoint(
          <PresentationUtilProvider>
            <ControlEditor
              isCreate={true}
-              factory={editableFactory}
              width={defaultControlWidth ?? DEFAULT_CONTROL_WIDTH}
              updateTitle={(newTitle) => (inputToReturn.title = newTitle)}
              updateWidth={updateDefaultWidth}
-              onTypeEditorChange={(partialInput) =>
-                (inputToReturn = { ...inputToReturn, ...partialInput })
-              }
-              onSave={() => {
-                if (editableFactory.presaveTransformFunction) {
-                  inputToReturn = editableFactory.presaveTransformFunction(inputToReturn);
+              onSave={(type: string) => {
+                const factory = getControlFactory(type) as IEditableControlFactory;
+                if (factory.presaveTransformFunction) {
+                  inputToReturn = factory.presaveTransformFunction(inputToReturn);
                }
-                resolve(inputToReturn);
+                resolve({ type, controlInput: inputToReturn });
                flyoutInstance.close();
              }}
              onCancel={() => onCancel(flyoutInstance)}
+              onTypeEditorChange={(partialInput) =>
+                (inputToReturn = { ...inputToReturn, ...partialInput })
+              }
            />
          </PresentationUtilProvider>
        ),
@ -105,8 +98,8 @@ export const CreateControlButton = ({
    });

    initialInputPromise.then(
-      async (explicitInput) => {
-        await addNewEmbeddable(type, explicitInput);
+      async (promise) => {
+        await addNewEmbeddable(promise.type, promise.controlInput);
      },
      () => {} // swallow promise rejection because it can be part of normal flow
    );
@ -115,60 +108,24 @@ export const CreateControlButton = ({
  if (getControlTypes().length === 0) return null;

  const commonButtonProps = {
-    color: 'primary' as EuiButtonIconColor,
+    key: 'addControl',
+    onClick: () => {
+      createNewControl();
+      if (closePopover) {
+        closePopover();
+      }
+    },
    'data-test-subj': 'controls-create-button',
    'aria-label': ControlGroupStrings.management.getManageButtonTitle(),
  };

-  const items: ReactElement[] = [];
-  getControlTypes().forEach((type) => {
-    const factory = getControlFactory(type);
-    items.push(
-      <EuiContextMenuItem
-        key={type}
-        icon={factory.getIconType?.()}
-        data-test-subj={`create-${type}-control`}
-        onClick={() => {
-          if (buttonType === 'callout' && isControlTypePopoverOpen) {
-            setIsControlTypePopoverOpen(false);
-          } else if (closePopover) {
-            closePopover();
-          }
-          createNewControl(type);
-        }}
-        toolTipContent={factory.getDescription()}
-      >
-        {factory.getDisplayName()}
-      </EuiContextMenuItem>
-    );
-  });
-
-  if (buttonType === 'callout') {
-    const onCreateButtonClick = () => {
-      if (getControlTypes().length > 1) {
-        setIsControlTypePopoverOpen(!isControlTypePopoverOpen);
-        return;
-      }
-      createNewControl(getControlTypes()[0]);
-    };
-
-    const createControlButton = (
-      <EuiButton {...commonButtonProps} onClick={onCreateButtonClick} size="s">
-        {ControlGroupStrings.emptyState.getAddControlButtonTitle()}
-      </EuiButton>
-    );
-    return (
-      <EuiPopover
-        button={createControlButton}
-        isOpen={isControlTypePopoverOpen}
-        panelPaddingSize="none"
-        anchorPosition="downLeft"
-        data-test-subj="control-type-picker"
-        closePopover={() => setIsControlTypePopoverOpen(false)}
-      >
-        <EuiContextMenuPanel items={items} />
-      </EuiPopover>
-    );
-  }
-  return <EuiContextMenuPanel items={items} />;
+  return buttonType === 'callout' ? (
+    <EuiButton {...commonButtonProps} color="primary" size="s">
+      {ControlGroupStrings.emptyState.getAddControlButtonTitle()}
+    </EuiButton>
+  ) : (
+    <EuiContextMenuItem {...commonButtonProps} icon="plusInCircle">
+      {ControlGroupStrings.emptyState.getAddControlButtonTitle()}
+    </EuiContextMenuItem>
+  );
 };
--- a/src/plugins/controls/public/control_group/editor/edit_control.tsx
+++ b/src/plugins/controls/public/control_group/editor/edit_control.tsx
@ -84,15 +84,12 @@ export const EditControlButton = ({ embeddableId }: { embeddableId: string }) =>
      });
    };

-    const editableFactory = factory as IEditableControlFactory;
-
    const flyoutInstance = openFlyout(
      forwardAllContext(
        <ControlEditor
          isCreate={false}
          width={panel.width}
          embeddable={embeddable}
-          factory={editableFactory}
          title={embeddable.getTitle()}
          onCancel={() => onCancel(flyoutInstance)}
          updateTitle={(newTitle) => (inputToReturn.title = newTitle)}
@ -101,6 +98,7 @@ export const EditControlButton = ({ embeddableId }: { embeddableId: string }) =>
            (inputToReturn = { ...inputToReturn, ...partialInput })
          }
          onSave={() => {
+            const editableFactory = factory as IEditableControlFactory;
            if (editableFactory.presaveTransformFunction) {
              inputToReturn = editableFactory.presaveTransformFunction(inputToReturn, embeddable);
            }
--- a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx
+++ b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx
@ -20,7 +20,7 @@ import {
  map,
  take,
 } from 'rxjs/operators';
-import { EuiContextMenuPanel, EuiHorizontalRule } from '@elastic/eui';
+import { EuiContextMenuPanel } from '@elastic/eui';

 import {
  ControlGroupInput,
@ -134,7 +134,6 @@ export class ControlGroupContainer extends Container<
          <EuiContextMenuPanel
            items={[
              this.getCreateControlButton('toolbar', closePopover),
-              <EuiHorizontalRule margin="none" />,
              this.getEditControlGroupButton(closePopover),
            ]}
          />
--- a/src/plugins/controls/public/control_types/options_list/options_list_strings.ts
+++ b/src/plugins/controls/public/control_types/options_list/options_list_strings.ts
@ -15,7 +15,7 @@ export const OptionsListStrings = {
    }),
  getDescription: () =>
    i18n.translate('controls.optionsList.description', {
-      defaultMessage: 'Add control that allows options to be selected from a dropdown.',
+      defaultMessage: 'Add a menu for selecting field values.',
    }),
  summary: {
    getSeparator: () =>
--- a/test/functional/page_objects/dashboard_page_controls.ts
+++ b/test/functional/page_objects/dashboard_page_controls.ts
@ -66,10 +66,11 @@ export class DashboardPageControls extends FtrService {
  public async openCreateControlFlyout(type: string) {
    this.log.debug(`Opening flyout for ${type} control`);
    await this.testSubjects.click('dashboard-controls-menu-button');
-    await this.testSubjects.click(`create-${type}-control`);
+    await this.testSubjects.click('controls-create-button');
    await this.retry.try(async () => {
      await this.testSubjects.existOrFail('control-editor-flyout');
    });
+    await this.testSubjects.click(`create-${type}-control`);
  }

  /* -----------------------------------------------------------