[Controls] Move "clear selections" to hover action (#159526)

Closes #159395 Closes #153383 ## Summary This PR moves the "clear selections" button for all controls (options list, range slider, and time slider) from inside their respective popovers to a general hover action - this not only saves users a click for this common interaction (which has actually been brought in user feedback up as a downside of the current controls compared to the legacy controls), it also allows us to fully move forward with migrating the range slider control to the `EuiDualRange` component. This will be done in a follow up PR, which should both (1) clean up our range slider code significantly and (2) fix the [bug discussed here](#159271 (review)). The related issue can be tracked [here](#159724), since we might not be able to get to it right away. This "clear selections" action is available in both view and edit mode, like so: | | Edit mode | View mode | |--------|--------|--------| | **Range slider** | ![image](https://github.com/elastic/kibana/assets/8698078/83cb1e1a-0b20-43aa-a37b-14484b5f4945) | ![image](https://github.com/elastic/kibana/assets/8698078/0d28ce03-5242-4f3a-8a05-d447bca50ddb) | | **Options list** | ![image](https://github.com/elastic/kibana/assets/8698078/066257f6-c0ce-4e33-a193-5bbc62e341a6) | ![image](https://github.com/elastic/kibana/assets/8698078/d1ec124c-f5ee-4137-9eb9-33e06d522435) | | **Time slider** | ![image](https://github.com/elastic/kibana/assets/8698078/33b8bb80-fa0c-4281-ae81-f1e1b44086f3) | ![image](https://github.com/elastic/kibana/assets/8698078/bd7c41ae-706c-45f3-8b49-9bd4d259e5cf) | You may notice in the above screenshots that the "delete" action is now represented with a red trash icon rather than a red cross, and the tooltip text was also changed to use the word "Delete" rather than the word "Remove" - these changes were both made to be more consistent with the "Delete panel" action available on dashboards: | Delete control - Before | Delete control - After | Delete panel | |--------|--------|--------| | ![Screenshot 2023-06-13 at 5 32 22 PM](https://github.com/elastic/kibana/assets/8698078/2600b197-653b-43ea-a043-a50be7e6a796) | ![image](https://github.com/elastic/kibana/assets/8698078/5ef80380-2575-45fc-ba11-c59f3f252ac3) | <img src="https://github.com/elastic/kibana/assets/8698078/a7f65777-45cf-44f2-96a7-f1042cb25e02"/> | Beyond these changes, I also made a few quick changes to the time slider control, including: 1. Fixing the appearance so that the background is once again white, as described [here](#159526 (comment)) 2. Adding comparison logic so that clearing selections no longer causes unsaved changes unnecessarily, as described [here](#159526 (comment)) ### Videos **Before** https://github.com/elastic/kibana/assets/8698078/96365c85-748e-4fd7-ae5d-589aa11a23ef **After** https://github.com/elastic/kibana/assets/8698078/68352559-e71b-4b5e-8709-587016f0b35a ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Jun 20, 2023 · f1dc1e1 · f1dc1e1
1 parent 69849ee
commit f1dc1e1
Show file tree

Hide file tree

Showing 28 changed files with 207 additions and 148 deletions.
diff --git a/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts b/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts
@@ -11,6 +11,7 @@ import { omit, isEqual } from 'lodash';
 import { OPTIONS_LIST_DEFAULT_SORT } from '../options_list/suggestions_sorting';
 import { OptionsListEmbeddableInput, OPTIONS_LIST_CONTROL } from '../options_list/types';
 import { RangeSliderEmbeddableInput, RANGE_SLIDER_CONTROL } from '../range_slider/types';
+import { TimeSliderControlEmbeddableInput, TIME_SLIDER_CONTROL } from '../time_slider/types';
 
 import { ControlPanelState } from './types';
 
@@ -88,4 +89,29 @@ export const ControlPanelDiffSystems: {
       );
     },
   },
+  [TIME_SLIDER_CONTROL]: {
+    getPanelIsEqual: (initialInput, newInput) => {
+      if (!deepEqual(omit(initialInput, 'explicitInput'), omit(newInput, 'explicitInput'))) {
+        return false;
+      }
+
+      const {
+        isAnchored: isAnchoredA,
+        timesliceStartAsPercentageOfTimeRange: startA,
+        timesliceEndAsPercentageOfTimeRange: endA,
+      }: Partial<TimeSliderControlEmbeddableInput> = initialInput.explicitInput;
+      const {
+        isAnchored: isAnchoredB,
+        timesliceStartAsPercentageOfTimeRange: startB,
+        timesliceEndAsPercentageOfTimeRange: endB,
+      }: Partial<TimeSliderControlEmbeddableInput> = newInput.explicitInput;
+      return (
+        Boolean(isAnchoredA) === Boolean(isAnchoredB) &&
+        Boolean(startA) === Boolean(startB) &&
+        startA === startB &&
+        Boolean(endA) === Boolean(endB) &&
+        endA === endB
+      );
+    },
+  },
 };
diff --git a/src/plugins/controls/public/control_group/actions/clear_control_action.tsx b/src/plugins/controls/public/control_group/actions/clear_control_action.tsx
@@ -0,0 +1,78 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import React, { SyntheticEvent } from 'react';
+
+import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
+import { isErrorEmbeddable } from '@kbn/embeddable-plugin/public';
+import { Action, IncompatibleActionError } from '@kbn/ui-actions-plugin/public';
+
+import { ACTION_CLEAR_CONTROL } from '.';
+import { ControlGroupStrings } from '../control_group_strings';
+import { ControlEmbeddable, DataControlInput, isClearableControl } from '../../types';
+import { isControlGroup } from '../embeddable/control_group_helpers';
+
+export interface ClearControlActionContext {
+  embeddable: ControlEmbeddable<DataControlInput>;
+}
+
+export class ClearControlAction implements Action<ClearControlActionContext> {
+  public readonly type = ACTION_CLEAR_CONTROL;
+  public readonly id = ACTION_CLEAR_CONTROL;
+  public order = 1;
+
+  constructor() {}
+
+  public readonly MenuItem = ({ context }: { context: ClearControlActionContext }) => {
+    return (
+      <EuiToolTip content={this.getDisplayName(context)}>
+        <EuiButtonIcon
+          data-test-subj={`control-action-${context.embeddable.id}-erase`}
+          aria-label={this.getDisplayName(context)}
+          iconType={this.getIconType(context)}
+          onClick={(event: SyntheticEvent<HTMLButtonElement>) => {
+            (event.target as HTMLButtonElement).blur();
+            this.execute(context);
+          }}
+          color="text"
+        />
+      </EuiToolTip>
+    );
+  };
+
+  public getDisplayName({ embeddable }: ClearControlActionContext) {
+    if (!embeddable.parent || !isControlGroup(embeddable.parent)) {
+      throw new IncompatibleActionError();
+    }
+    return ControlGroupStrings.floatingActions.getClearButtonTitle();
+  }
+
+  public getIconType({ embeddable }: ClearControlActionContext) {
+    if (!embeddable.parent || !isControlGroup(embeddable.parent)) {
+      throw new IncompatibleActionError();
+    }
+    return 'eraser';
+  }
+
+  public async isCompatible({ embeddable }: ClearControlActionContext) {
+    if (isErrorEmbeddable(embeddable)) return false;
+    const controlGroup = embeddable.parent;
+    return Boolean(controlGroup && isControlGroup(controlGroup)) && isClearableControl(embeddable);
+  }
+
+  public async execute({ embeddable }: ClearControlActionContext) {
+    if (
+      !embeddable.parent ||
+      !isControlGroup(embeddable.parent) ||
+      !isClearableControl(embeddable)
+    ) {
+      throw new IncompatibleActionError();
+    }
+    embeddable.clearSelections();
+  }
+}
diff --git a/src/plugins/controls/public/control_group/actions/delete_control_action.tsx b/src/plugins/controls/public/control_group/actions/delete_control_action.tsx
@@ -25,7 +25,7 @@ export interface DeleteControlActionContext {
 export class DeleteControlAction implements Action<DeleteControlActionContext> {
   public readonly type = ACTION_DELETE_CONTROL;
   public readonly id = ACTION_DELETE_CONTROL;
-  public order = 2;
+  public order = 100; // should always be last
 
   private openConfirm;
 
@@ -60,7 +60,7 @@ export class DeleteControlAction implements Action<DeleteControlActionContext> {
     if (!embeddable.parent || !isControlGroup(embeddable.parent)) {
       throw new IncompatibleActionError();
     }
-    return 'cross';
+    return 'trash';
   }
 
   public async isCompatible({ embeddable }: DeleteControlActionContext) {

diff --git a/src/plugins/controls/public/control_group/actions/edit_control_action.tsx b/src/plugins/controls/public/control_group/actions/edit_control_action.tsx
@@ -29,7 +29,7 @@ export interface EditControlActionContext {
 export class EditControlAction implements Action<EditControlActionContext> {
   public readonly type = ACTION_EDIT_CONTROL;
   public readonly id = ACTION_EDIT_CONTROL;
-  public order = 1;
+  public order = 2;
 
   private getEmbeddableFactory;
   private openFlyout;

diff --git a/src/plugins/controls/public/control_group/actions/index.ts b/src/plugins/controls/public/control_group/actions/index.ts
@@ -7,4 +7,5 @@
  */
 
 export const ACTION_EDIT_CONTROL = 'editControl';
+export const ACTION_CLEAR_CONTROL = 'clearControl';
 export const ACTION_DELETE_CONTROL = 'deleteControl';
diff --git a/src/plugins/controls/public/control_group/component/control_group_component.tsx b/src/plugins/controls/public/control_group/component/control_group_component.tsx
@@ -82,6 +82,7 @@ export const ControlGroup = () => {
         });
       }
     }
+    (document.activeElement as HTMLElement)?.blur();
     setDraggingId(null);
   };
 

diff --git a/src/plugins/controls/public/control_group/control_group_strings.ts b/src/plugins/controls/public/control_group/control_group_strings.ts
@@ -285,11 +285,16 @@ export const ControlGroupStrings = {
   floatingActions: {
     getEditButtonTitle: () =>
       i18n.translate('controls.controlGroup.floatingActions.editTitle', {
-        defaultMessage: 'Edit control',
+        defaultMessage: 'Edit',
       }),
     getRemoveButtonTitle: () =>
       i18n.translate('controls.controlGroup.floatingActions.removeTitle', {
-        defaultMessage: 'Remove control',
+        defaultMessage: 'Delete',
+      }),
+
+    getClearButtonTitle: () =>
+      i18n.translate('controls.controlGroup.floatingActions.clearTitle', {
+        defaultMessage: 'Clear',
       }),
   },
   ariaActions: {

diff --git a/src/plugins/controls/public/options_list/components/options_list_popover_action_bar.tsx b/src/plugins/controls/public/options_list/components/options_list_popover_action_bar.tsx
@@ -134,21 +134,6 @@ export const OptionsListPopoverActionBar = ({
                   />
                 </EuiToolTip>
               </EuiFlexItem>
-              <EuiFlexItem grow={false}>
-                <EuiToolTip
-                  position="top"
-                  content={OptionsListStrings.popover.getClearAllSelectionsButtonTitle()}
-                >
-                  <EuiButtonIcon
-                    size="xs"
-                    color="danger"
-                    iconType="eraser"
-                    onClick={() => optionsList.dispatch.clearSelections({})}
-                    data-test-subj="optionsList-control-clear-all-selections"
-                    aria-label={OptionsListStrings.popover.getClearAllSelectionsButtonTitle()}
-                  />
-                </EuiToolTip>
-              </EuiFlexItem>
             </EuiFlexGroup>
           </EuiFlexItem>
         </EuiFlexGroup>

diff --git a/src/plugins/controls/public/options_list/components/options_list_strings.ts b/src/plugins/controls/public/options_list/components/options_list_strings.ts
@@ -129,10 +129,6 @@ export const OptionsListStrings = {
       i18n.translate('controls.optionsList.popover.selectedOptionsTitle', {
         defaultMessage: 'Show only selected options',
       }),
-    getClearAllSelectionsButtonTitle: () =>
-      i18n.translate('controls.optionsList.popover.clearAllSelectionsTitle', {
-        defaultMessage: 'Clear selections',
-      }),
     searchPlaceholder: {
       prefix: {
         getPlaceholderText: () =>

diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx
@@ -35,10 +35,11 @@ import {
   OptionsListEmbeddableInput,
 } from '../..';
 import { pluginServices } from '../../services';
-import { MIN_OPTIONS_LIST_REQUEST_SIZE, OptionsListReduxState } from '../types';
+import { IClearableControl } from '../../types';
 import { OptionsListControl } from '../components/options_list_control';
 import { ControlsDataViewsService } from '../../services/data_views/types';
 import { ControlsOptionsListService } from '../../services/options_list/types';
+import { MIN_OPTIONS_LIST_REQUEST_SIZE, OptionsListReduxState } from '../types';
 import { getDefaultComponentState, optionsListReducers } from '../options_list_reducers';
 
 const diffDataFetchProps = (
@@ -76,7 +77,10 @@ type OptionsListReduxEmbeddableTools = ReduxEmbeddableTools<
   typeof optionsListReducers
 >;
 
-export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput, ControlOutput> {
+export class OptionsListEmbeddable
+  extends Embeddable<OptionsListEmbeddableInput, ControlOutput>
+  implements IClearableControl
+{
   public readonly type = OPTIONS_LIST_CONTROL;
   public deferEmbeddableLoad = true;
 
@@ -411,6 +415,10 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
     return [newFilter];
   };
 
+  public clearSelections() {
+    this.dispatch.clearSelections({});
+  }
+
   reload = () => {
     // clear cache when reload is requested
     this.optionsListService.clearOptionsListCache();

diff --git a/src/plugins/controls/public/plugin.ts b/src/plugins/controls/public/plugin.ts
@@ -124,6 +124,11 @@ export class ControlsPlugin
       const editControlAction = new EditControlAction(deleteControlAction);
       uiActions.registerAction(editControlAction);
       uiActions.attachAction(PANEL_HOVER_TRIGGER, editControlAction.id);
+
+      const { ClearControlAction } = await import('./control_group/actions/clear_control_action');
+      const clearControlAction = new ClearControlAction();
+      uiActions.registerAction(clearControlAction);
+      uiActions.attachAction(PANEL_HOVER_TRIGGER, clearControlAction.id);
     });
 
     const { getControlFactory, getControlTypes } = controlsService;

diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx
@@ -9,15 +9,7 @@
 import React, { FC, ComponentProps, Ref, useEffect, useState, useMemo } from 'react';
 import useMount from 'react-use/lib/useMount';
 
-import {
-  EuiPopoverTitle,
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiDualRange,
-  EuiToolTip,
-  EuiButtonIcon,
-  EuiText,
-} from '@elastic/eui';
+import { EuiPopoverTitle, EuiDualRange, EuiText } from '@elastic/eui';
 import type { EuiDualRangeClass } from '@elastic/eui/src/components/form/range/dual_range';
 
 import { pluginServices } from '../../services';
@@ -101,55 +93,34 @@ export const RangeSliderPopover: FC<{
   }, [min, max]);
 
   return (
-    <>
+    <div data-test-subj="rangeSlider__popover">
       <EuiPopoverTitle paddingSize="s">{title}</EuiPopoverTitle>
-      <EuiFlexGroup
-        className="rangeSlider__actions"
-        gutterSize="none"
-        data-test-subj="rangeSlider-control-actions"
-        responsive={false}
-      >
-        <EuiFlexItem>
-          {min !== -Infinity && max !== Infinity ? (
-            <EuiDualRange
-              id={id}
-              min={rangeSliderMin}
-              max={rangeSliderMax}
-              onChange={([minSelection, maxSelection]) => {
-                onChange([String(minSelection), String(maxSelection)]);
-              }}
-              value={value}
-              ticks={ticks}
-              levels={levels}
-              showTicks
-              fullWidth
-              ref={rangeRef}
-              data-test-subj="rangeSlider__slider"
-            />
-          ) : isInvalid ? (
-            <EuiText size="s" data-test-subj="rangeSlider__helpText">
-              {RangeSliderStrings.popover.getNoDataHelpText()}
-            </EuiText>
-          ) : (
-            <EuiText size="s" data-test-subj="rangeSlider__helpText">
-              {RangeSliderStrings.popover.getNoAvailableDataHelpText()}
-            </EuiText>
-          )}
-        </EuiFlexItem>
-        <EuiFlexItem grow={false}>
-          <EuiToolTip content={RangeSliderStrings.popover.getClearRangeButtonTitle()}>
-            <EuiButtonIcon
-              iconType="eraser"
-              color="danger"
-              onClick={() => {
-                rangeSlider.dispatch.setSelectedRange(['', '']);
-              }}
-              aria-label={RangeSliderStrings.popover.getClearRangeButtonTitle()}
-              data-test-subj="rangeSlider__clearRangeButton"
-            />
-          </EuiToolTip>
-        </EuiFlexItem>
-      </EuiFlexGroup>
-    </>
+
+      {min !== -Infinity && max !== Infinity ? (
+        <EuiDualRange
+          id={id}
+          min={rangeSliderMin}
+          max={rangeSliderMax}
+          onChange={([minSelection, maxSelection]) => {
+            onChange([String(minSelection), String(maxSelection)]);
+          }}
+          value={value}
+          ticks={ticks}
+          levels={levels}
+          showTicks
+          fullWidth
+          ref={rangeRef}
+          data-test-subj="rangeSlider__slider"
+        />
+      ) : isInvalid ? (
+        <EuiText size="s" data-test-subj="rangeSlider__helpText">
+          {RangeSliderStrings.popover.getNoDataHelpText()}
+        </EuiText>
+      ) : (
+        <EuiText size="s" data-test-subj="rangeSlider__helpText">
+          {RangeSliderStrings.popover.getNoAvailableDataHelpText()}
+        </EuiText>
+      )}
+    </div>
   );
 };
diff --git a/src/plugins/controls/public/range_slider/components/range_slider_strings.ts b/src/plugins/controls/public/range_slider/components/range_slider_strings.ts
@@ -10,10 +10,6 @@ import { i18n } from '@kbn/i18n';
 
 export const RangeSliderStrings = {
   popover: {
-    getClearRangeButtonTitle: () =>
-      i18n.translate('controls.rangeSlider.popover.clearRangeTitle', {
-        defaultMessage: 'Clear range',
-      }),
     getNoDataHelpText: () =>
       i18n.translate('controls.rangeSlider.popover.noDataHelpText', {
         defaultMessage: 'Selected range resulted in no data. No filter was applied.',
-Original file line number
+Diff line change
@@ Expand Up / @@ -82,6 +82,7 @@ export const ControlGroup = () => { @@
             });
           }
         }
+        (document.activeElement as HTMLElement)?.blur();
         setDraggingId(null);
       };
@@ Expand Down @@