EuiColorStops change popover open from onClick to onMouseDown (#2505)

* parent component handles popover state; mousedown triggers popover toggle * CL
elastic · Oct 31, 2019 · 8f0de9f · 8f0de9f
1 parent 207a119
commit 8f0de9f
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 - Convert `EuiFlyout` to TypeScript ([#2500](https://github.com/elastic/eui/pull/2500))
 
+**Bug fixes**
+
+- Simplified `EuiColorStops` popover toggling ([#2505](https://github.com/elastic/eui/pull/2505))
+
 ## [`14.9.0`](https://github.com/elastic/eui/tree/v14.9.0)
 
 - Added new `euiTreeView` component for rendering recursive objects such as folder structures. ([#2409](https://github.com/elastic/eui/pull/2409))

diff --git a/src/components/color_picker/color_stops/color_stop_thumb.test.tsx b/src/components/color_picker/color_stops/color_stop_thumb.test.tsx
@@ -25,6 +25,9 @@ test('renders EuiColorStopThumb', () => {
       globalMin={0}
       globalMax={100}
       colorPickerMode="default"
+      isPopoverOpen={false}
+      openPopover={() => {}}
+      closePopover={() => {}}
       {...requiredProps}
     />
   );
@@ -42,6 +45,9 @@ test('renders swatch-only EuiColorStopThumb', () => {
       globalMin={0}
       globalMax={100}
       colorPickerMode="swatch"
+      isPopoverOpen={false}
+      openPopover={() => {}}
+      closePopover={() => {}}
       {...requiredProps}
     />
   );
@@ -59,6 +65,9 @@ test('renders picker-only EuiColorStopThumb', () => {
       globalMin={0}
       globalMax={100}
       colorPickerMode="picker"
+      isPopoverOpen={false}
+      openPopover={() => {}}
+      closePopover={() => {}}
       {...requiredProps}
     />
   );
@@ -77,6 +86,9 @@ test('renders disabled EuiColorStopThumb', () => {
       globalMax={100}
       colorPickerMode="default"
       disabled={true}
+      isPopoverOpen={false}
+      openPopover={() => {}}
+      closePopover={() => {}}
       {...requiredProps}
     />
   );
@@ -95,6 +107,9 @@ test('renders readOnly EuiColorStopThumb', () => {
       globalMax={100}
       colorPickerMode="default"
       readOnly={true}
+      isPopoverOpen={false}
+      openPopover={() => {}}
+      closePopover={() => {}}
       {...requiredProps}
     />
   );

diff --git a/src/components/color_picker/color_stops/color_stop_thumb.tsx b/src/components/color_picker/color_stops/color_stop_thumb.tsx
@@ -51,7 +51,9 @@ interface EuiColorStopThumbProps extends CommonProps, ColorStop {
   colorPickerSwatches?: EuiColorPickerProps['swatches'];
   disabled?: boolean;
   readOnly?: boolean;
-  isPopoverOpen?: boolean;
+  isPopoverOpen: boolean;
+  openPopover: () => void;
+  closePopover: () => void;
   'data-index'?: string;
   'aria-valuetext'?: string;
 }
@@ -76,12 +78,13 @@ export const EuiColorStopThumb: FunctionComponent<EuiColorStopThumbProps> = ({
   colorPickerSwatches,
   disabled,
   readOnly,
-  isPopoverOpen: beginOpen = false,
+  isPopoverOpen,
+  openPopover,
+  closePopover,
   'data-index': dataIndex,
   'aria-valuetext': ariaValueText,
 }) => {
-  const [isPopoverOpen, setIsPopoverOpen] = useState(beginOpen);
-  const [hasFocus, setHasFocus] = useState(beginOpen);
+  const [hasFocus, setHasFocus] = useState(isPopoverOpen);
   const [colorIsInvalid, setColorIsInvalid] = useState(isColorInvalid(color));
   const [stopIsInvalid, setStopIsInvalid] = useState(isStopInvalid(stop));
   const [numberInputRef, setNumberInputRef] = useState();
@@ -103,10 +106,6 @@ export const EuiColorStopThumb: FunctionComponent<EuiColorStopThumbProps> = ({
     return getPositionFromStop(stop, parentRef!, globalMin, globalMax);
   };
 
-  const openPopover = () => setIsPopoverOpen(true);
-
-  const closePopover = () => setIsPopoverOpen(false);
-
   const handleOnRemove = () => {
     if (onRemove) {
       closePopover();
@@ -171,13 +170,20 @@ export const EuiColorStopThumb: FunctionComponent<EuiColorStopThumbProps> = ({
 
   const handleKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => {
     switch (e.keyCode) {
+      case keyCodes.ENTER:
+        e.preventDefault();
+        openPopover();
+        break;
+
       case keyCodes.LEFT:
         e.preventDefault();
+        if (readOnly) return;
         handleStopChange(stop - 1);
         break;
 
       case keyCodes.RIGHT:
         e.preventDefault();
+        if (readOnly) return;
         handleStopChange(stop + 1);
         break;
     }
@@ -187,6 +193,20 @@ export const EuiColorStopThumb: FunctionComponent<EuiColorStopThumbProps> = ({
     handlePointerChange
   );
 
+  const handleOnMouseDown = (e: React.MouseEvent<HTMLButtonElement>) => {
+    if (!readOnly) {
+      handleMouseDown(e);
+    }
+    openPopover();
+  };
+
+  const handleTouchStart = (e: React.TouchEvent<HTMLButtonElement>) => {
+    if (!readOnly) {
+      handleInteraction(e);
+    }
+    openPopover();
+  };
+
   const classes = classNames(
     'euiColorStopPopover',
     {
@@ -231,14 +251,13 @@ export const EuiColorStopThumb: FunctionComponent<EuiColorStopThumbProps> = ({
                 min={localMin}
                 max={localMax}
                 value={stop}
-                onClick={openPopover}
                 onFocus={handleFocus}
                 onBlur={() => setHasFocus(false)}
                 onMouseOver={() => setHasFocus(true)}
                 onMouseOut={() => setHasFocus(false)}
-                onKeyDown={readOnly ? undefined : handleKeyDown}
-                onMouseDown={readOnly ? undefined : handleMouseDown}
-                onTouchStart={readOnly ? undefined : handleInteraction}
+                onKeyDown={handleKeyDown}
+                onMouseDown={handleOnMouseDown}
+                onTouchStart={handleTouchStart}
                 onTouchMove={readOnly ? undefined : handleInteraction}
                 aria-valuetext={ariaValueText}
                 aria-label={ariaLabel}

diff --git a/src/components/color_picker/color_stops/color_stops.test.tsx b/src/components/color_picker/color_stops/color_stops.test.tsx
@@ -182,7 +182,7 @@ test('popover color selector is shown when the thumb is clicked', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const colorSelector = findTestSubject(colorStops, 'euiColorStopPopover');
   expect(colorSelector.length).toBe(1);
 });
@@ -201,7 +201,7 @@ test('stop input updates stops', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const event = { target: { value: '10' } };
   const inputs = colorStops.find('input[type="number"]');
   expect(inputs.length).toBe(1);
@@ -231,7 +231,7 @@ test('stop input updates stops with error prevention (reset to bounds)', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const event = { target: { value: '1000' } };
   const inputs = colorStops.find('input[type="number"]');
   inputs.simulate('change', event);
@@ -260,7 +260,7 @@ test('hex input updates stops', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const event = { target: { value: '#FFFFFF' } };
   const inputs = colorStops.find('input[type="text"]');
   expect(inputs.length).toBe(1);
@@ -290,7 +290,7 @@ test('hex input updates stops with error', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const event = { target: { value: '#FFFFF' } };
   const inputs = colorStops.find('input[type="text"]');
   inputs.simulate('change', event);
@@ -319,7 +319,7 @@ test('picker updates stops', () => {
 
   findTestSubject(colorStops, 'euiColorStopThumb')
     .first()
-    .simulate('click');
+    .simulate('mousedown', { pageX: 0, pageY: 0 });
   const swatches = colorStops.find('button.euiColorPicker__swatchSelect');
   expect(swatches.length).toBe(VISUALIZATION_COLORS.length);
   swatches.first().simulate('click');

diff --git a/src/components/color_picker/color_stops/color_stops.tsx b/src/components/color_picker/color_stops/color_stops.tsx
@@ -124,6 +124,7 @@ export const EuiColorStops: FunctionComponent<EuiColorStopsProps> = ({
   }, [colorStops, min, rangeMax]);
   const [hasFocus, setHasFocus] = useState(false);
   const [focusedStopIndex, setFocusedStopIndex] = useState<number | null>(null);
+  const [openedStopId, setOpenedStopId] = useState<number | null>(null);
   const [wrapperRef, setWrapperRef] = useState<HTMLDivElement | null>(null);
   const [addTargetPosition, setAddTargetPosition] = useState<number>(0);
   const [isHoverDisabled, setIsHoverDisabled] = useState<boolean>(false);
@@ -133,8 +134,12 @@ export const EuiColorStops: FunctionComponent<EuiColorStopsProps> = ({
 
   useEffect(() => {
     if (focusStopOnUpdate !== null) {
-      const toFocus = sortedStops.map(el => el.stop).indexOf(focusStopOnUpdate);
-      onFocusStop(toFocus);
+      const toFocusIndex = sortedStops
+        .map(el => el.stop)
+        .indexOf(focusStopOnUpdate);
+      const toFocusId = toFocusIndex > -1 ? sortedStops[toFocusIndex].id : null;
+      onFocusStop(toFocusIndex);
+      setOpenedStopId(toFocusId);
       setFocusStopOnUpdate(null);
     }
   }, [sortedStops]);
@@ -324,7 +329,13 @@ export const EuiColorStops: FunctionComponent<EuiColorStopsProps> = ({
       aria-valuetext={`Stop: ${colorStop.stop}, Color: ${
         colorStop.color
       } (${index + 1} of ${colorStops.length})`}
-      isPopoverOpen={colorStop.stop === focusStopOnUpdate}
+      isPopoverOpen={colorStop.id === openedStopId}
+      openPopover={() => {
+        setOpenedStopId(colorStop.id);
+      }}
+      closePopover={() => {
+        setOpenedStopId(null);
+      }}
     />
   ));
 

diff --git a/src/components/form/range/range_thumb.tsx b/src/components/form/range/range_thumb.tsx
@@ -15,7 +15,7 @@ interface BaseProps extends CommonProps {
 interface ButtonLike extends BaseProps, HTMLAttributes<HTMLButtonElement> {}
 interface DivLike
   extends BaseProps,
-    Omit<HTMLAttributes<HTMLDivElement>, 'onClick'> {}
+    Omit<HTMLAttributes<HTMLDivElement>, 'onClick' | 'onMouseDown'> {}
 
 export type EuiRangeThumbProps = ExclusiveUnion<ButtonLike, DivLike>;
 
@@ -28,6 +28,7 @@ export const EuiRangeThumb: FunctionComponent<EuiRangeThumbProps> = ({
   showInput,
   showTicks,
   onClick,
+  onMouseDown,
   tabIndex,
   ...rest
 }) => {
@@ -47,10 +48,11 @@ export const EuiRangeThumb: FunctionComponent<EuiRangeThumbProps> = ({
     'aria-disabled': !!disabled,
     tabIndex: showInput || !!disabled ? -1 : tabIndex || 0,
   };
-  return onClick ? (
+  return onClick || onMouseDown ? (
     <button
       type="button"
       onClick={onClick}
+      onMouseDown={onMouseDown}
       disabled={disabled}
       {...commonAttrs}
       {...rest as HTMLAttributes<HTMLButtonElement>}