[TextField] Allow click event on spinner (stepper in number type) (#6719

) * [TextField] Do not stopPropagation on prop type number * Allow click events inside spinner arrows and alphabetize functions Co-authored-by: Weslley Araujo <weslley.araujo@shopify.com>
Shopify · Jul 27, 2022 · b75bc19 · b75bc19
1 parent 02663c0
commit b75bc19
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 127 deletions.
diff --git a/.changeset/late-turkeys-teach.md b/.changeset/late-turkeys-teach.md
@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': major
+---
+
+Allow click events for spinner in [TextField]
diff --git a/polaris-react/src/components/TextField/TextField.tsx b/polaris-react/src/components/TextField/TextField.tsx
@@ -239,6 +239,7 @@ export function TextField({
   const suffixRef = useRef<HTMLDivElement>(null);
   const verticalContentRef = useRef<HTMLDivElement>(null);
   const buttonPressTimer = useRef<number>();
+  const spinnerRef = useRef<HTMLDivElement>(null);
 
   useEffect(() => {
     const input = inputRef.current;
@@ -405,6 +406,7 @@ export function TextField({
         onChange={handleNumberChange}
         onMouseDown={handleButtonPress}
         onMouseUp={handleButtonRelease}
+        ref={spinnerRef}
       />
     ) : null;
 
@@ -571,46 +573,6 @@ export function TextField({
     </Labelled>
   );
 
-  function handleClearButtonPress() {
-    onClearButtonClick && onClearButtonClick(id);
-  }
-
-  function handleKeyPress(event: React.KeyboardEvent) {
-    const {key, which} = event;
-    const numbersSpec = /[\d.eE+-]$/;
-    if (type !== 'number' || which === Key.Enter || numbersSpec.test(key)) {
-      return;
-    }
-
-    event.preventDefault();
-  }
-
-  function isPrefixOrSuffix(target: Element | EventTarget) {
-    return (
-      target instanceof Element &&
-      ((prefixRef.current && prefixRef.current.contains(target)) ||
-        (suffixRef.current && suffixRef.current.contains(target)))
-    );
-  }
-
-  function isVerticalContent(target: Element | EventTarget) {
-    return (
-      target instanceof Element &&
-      verticalContentRef.current &&
-      (verticalContentRef.current.contains(target) ||
-        verticalContentRef.current.contains(document.activeElement))
-    );
-  }
-
-  function isInput(target: HTMLElement | EventTarget) {
-    return (
-      target instanceof HTMLElement &&
-      inputRef.current &&
-      (inputRef.current.contains(target) ||
-        inputRef.current.contains(document.activeElement))
-    );
-  }
-
   function handleChange(event: React.ChangeEvent<HTMLInputElement>) {
     onChange && onChange(event.currentTarget.value, id);
   }
@@ -620,6 +582,7 @@ export function TextField({
       isPrefixOrSuffix(target) ||
       isVerticalContent(target) ||
       isInput(target) ||
+      isSpinner(target) ||
       focus
     ) {
       return;
@@ -629,7 +592,7 @@ export function TextField({
   }
 
   function handleClickChild(event: React.MouseEvent) {
-    if (inputRef.current !== event.target) {
+    if (!isSpinner(event.target) && !isInput(event.target)) {
       event.stopPropagation();
     }
 
@@ -644,18 +607,66 @@ export function TextField({
 
     setFocus(true);
   }
-}
 
-function normalizeAriaMultiline(multiline?: boolean | number) {
-  if (!multiline) return undefined;
+  function handleClearButtonPress() {
+    onClearButtonClick && onClearButtonClick(id);
+  }
 
-  return Boolean(multiline) || multiline > 0
-    ? {'aria-multiline': true}
-    : undefined;
+  function handleKeyPress(event: React.KeyboardEvent) {
+    const {key, which} = event;
+    const numbersSpec = /[\d.eE+-]$/;
+    if (type !== 'number' || which === Key.Enter || numbersSpec.test(key)) {
+      return;
+    }
+
+    event.preventDefault();
+  }
+
+  function isInput(target: HTMLElement | EventTarget) {
+    return (
+      target instanceof HTMLElement &&
+      inputRef.current &&
+      (inputRef.current.contains(target) ||
+        inputRef.current.contains(document.activeElement))
+    );
+  }
+
+  function isPrefixOrSuffix(target: Element | EventTarget) {
+    return (
+      target instanceof Element &&
+      ((prefixRef.current && prefixRef.current.contains(target)) ||
+        (suffixRef.current && suffixRef.current.contains(target)))
+    );
+  }
+
+  function isSpinner(target: Element | EventTarget) {
+    return (
+      target instanceof Element &&
+      spinnerRef.current &&
+      spinnerRef.current.contains(target)
+    );
+  }
+
+  function isVerticalContent(target: Element | EventTarget) {
+    return (
+      target instanceof Element &&
+      verticalContentRef.current &&
+      (verticalContentRef.current.contains(target) ||
+        verticalContentRef.current.contains(document.activeElement))
+    );
+  }
 }
 
 function getRows(multiline?: boolean | number) {
   if (!multiline) return undefined;
 
   return typeof multiline === 'number' ? multiline : 1;
 }
+
+function normalizeAriaMultiline(multiline?: boolean | number) {
+  if (!multiline) return undefined;
+
+  return Boolean(multiline) || multiline > 0
+    ? {'aria-multiline': true}
+    : undefined;
+}
diff --git a/polaris-react/src/components/TextField/components/Spinner/Spinner.tsx b/polaris-react/src/components/TextField/components/Spinner/Spinner.tsx
@@ -13,50 +13,46 @@ export interface SpinnerProps {
   onMouseUp(): void;
 }
 
-export function Spinner({
-  onChange,
-  onClick,
-  onMouseDown,
-  onMouseUp,
-}: SpinnerProps) {
-  function handleStep(step: number) {
-    return () => onChange(step);
-  }
+export const Spinner = React.forwardRef<HTMLDivElement, SpinnerProps>(
+  function Spinner({onChange, onClick, onMouseDown, onMouseUp}, ref) {
+    function handleStep(step: number) {
+      return () => onChange(step);
+    }
 
-  function handleMouseDown(onChange: HandleStepFn) {
-    return (event: React.MouseEvent) => {
-      if (event.button !== 0) return;
-      onMouseDown(onChange);
-    };
-  }
+    function handleMouseDown(onChange: HandleStepFn) {
+      return (event: React.MouseEvent) => {
+        if (event.button !== 0) return;
+        onMouseDown(onChange);
+      };
+    }
 
-  return (
-    <div className={styles.Spinner} onClick={onClick} aria-hidden>
-      <div
-        role="button"
-        className={styles.Segment}
-        tabIndex={-1}
-        onClick={handleStep(1)}
-        onMouseDown={handleMouseDown(handleStep(1))}
-        onMouseUp={onMouseUp}
-      >
-        <div className={styles.SpinnerIcon}>
-          <Icon source={CaretUpMinor} />
+    return (
+      <div className={styles.Spinner} onClick={onClick} aria-hidden ref={ref}>
+        <div
+          role="button"
+          className={styles.Segment}
+          tabIndex={-1}
+          onClick={handleStep(1)}
+          onMouseDown={handleMouseDown(handleStep(1))}
+          onMouseUp={onMouseUp}
+        >
+          <div className={styles.SpinnerIcon}>
+            <Icon source={CaretUpMinor} />
+          </div>
         </div>
-      </div>
-
-      <div
-        role="button"
-        className={styles.Segment}
-        tabIndex={-1}
-        onClick={handleStep(-1)}
-        onMouseDown={handleMouseDown(handleStep(-1))}
-        onMouseUp={onMouseUp}
-      >
-        <div className={styles.SpinnerIcon}>
-          <Icon source={CaretDownMinor} />
+        <div
+          role="button"
+          className={styles.Segment}
+          tabIndex={-1}
+          onClick={handleStep(-1)}
+          onMouseDown={handleMouseDown(handleStep(-1))}
+          onMouseUp={onMouseUp}
+        >
+          <div className={styles.SpinnerIcon}>
+            <Icon source={CaretDownMinor} />
+          </div>
         </div>
       </div>
-    </div>
-  );
-}
+    );
+  },
+);
diff --git a/polaris-react/src/components/TextField/tests/TextField.test.tsx b/polaris-react/src/components/TextField/tests/TextField.test.tsx
@@ -85,49 +85,62 @@ describe('<TextField />', () => {
   });
 
   describe('click events', () => {
-    describe('when a click event occurs on the input', () => {
-      it('bubbles up to the parent element', () => {
-        const onClick = jest.fn();
-        const event = new MouseEvent('click', {
-          view: window,
-          bubbles: true,
-          cancelable: true,
-        });
-        const textField = mountWithApp(
-          <div onClick={onClick}>
-            <TextField type="text" label="TextField" autoComplete="off" />
-          </div>,
-        );
+    it('bubbles up to the parent element when it occurs in the input', () => {
+      const onClick = jest.fn();
+      const event = new MouseEvent('click', {
+        view: window,
+        bubbles: true,
+        cancelable: true,
+      });
+      const textField = mountWithApp(
+        <div onClick={onClick}>
+          <TextField type="text" label="TextField" autoComplete="off" />
+        </div>,
+      );
 
-        textField.find('input')!.domNode?.dispatchEvent(event);
-        expect(onClick).toHaveBeenCalled();
+      textField.find('input')!.domNode?.dispatchEvent(event);
+      expect(onClick).toHaveBeenCalled();
+    });
+
+    it('bubbles up to the parent element when it occurs in the spinner', () => {
+      const onClick = jest.fn();
+      const event = new MouseEvent('click', {
+        view: window,
+        bubbles: true,
+        cancelable: true,
       });
+      const textField = mountWithApp(
+        <div onClick={onClick}>
+          <TextField type="number" label="TextField" autoComplete="off" />
+        </div>,
+      );
 
-      describe('when a click event occurs in an element other than the input', () => {
-        it('does not bubble up to the parent element', () => {
-          const onClick = jest.fn();
-          const children = 'vertical-content-children';
-          const event = new MouseEvent('click', {
-            view: window,
-            bubbles: true,
-            cancelable: true,
-          });
-          const verticalContent = <span>{children}</span>;
-          const textField = mountWithApp(
-            <div onClick={onClick}>
-              <TextField
-                type="text"
-                label="TextField"
-                autoComplete="off"
-                verticalContent={verticalContent}
-              />
-            </div>,
-          );
+      textField.find(Spinner)!.domNode?.dispatchEvent(event);
+      expect(onClick).toHaveBeenCalled();
+    });
 
-          textField.find('span', {children})!.domNode?.dispatchEvent(event);
-          expect(onClick).not.toHaveBeenCalled();
-        });
+    it('does not bubble up to the parent element when it occurs in an element other than the input', () => {
+      const onClick = jest.fn();
+      const children = 'vertical-content-children';
+      const event = new MouseEvent('click', {
+        view: window,
+        bubbles: true,
+        cancelable: true,
       });
+      const verticalContent = <span>{children}</span>;
+      const textField = mountWithApp(
+        <div onClick={onClick}>
+          <TextField
+            type="text"
+            label="TextField"
+            autoComplete="off"
+            verticalContent={verticalContent}
+          />
+        </div>,
+      );
+
+      textField.find('span', {children})!.domNode?.dispatchEvent(event);
+      expect(onClick).not.toHaveBeenCalled();
     });
   });