Tooltip Refactoring (#155)

* builds

* builds again

* linter is happy and tests passed

* created react types package

* saving progress

* attempt at proper aria labeling

* swap naming in the story

* clean up

* more clean up

* name flip

* add some tests

Author: MidnightCoder06

packages/@react-aria/tooltip/package.json

@@ -16,7 +16,10 @@
   },
   "dependencies": {
     "@babel/runtime": "^7.6.2",
-    "@react-aria/utils": "^3.0.0-alpha.2"
+    "@react-aria/utils": "^3.0.0-alpha.2",
+    "@react-aria/overlays": "^3.0.0-alpha.2",
+    "@react-aria/interactions": "^3.0.0-alpha.2",
+    "@react-stately/tooltip": "^3.0.0-alpha.2"
   },
   "peerDependencies": {
     "react": "^16.8.0"

packages/@react-aria/tooltip/src/index.ts

@@ -1 +1,2 @@
 export * from './useTooltip';
+export * from './useTooltipTrigger';

packages/@react-aria/tooltip/src/useTooltip.ts

@@ -12,13 +12,17 @@ interface TooltipAria {
 
 export function useTooltip(props: TooltipProps): TooltipAria {
   let tooltipId = useId(props.id);
+
   let {
     role = 'tooltip'
   } = props;
+
+  let tooltipProps = {
+    role,
+    id: tooltipId
+  };
+
   return {
-    tooltipProps: {
-      role,
-      id: tooltipId
-    }
+    tooltipProps
   };
 }

packages/@react-aria/tooltip/src/useTooltipTrigger.ts

@@ -0,0 +1,74 @@
+import {chain} from '@react-aria/utils';
+import {DOMProps} from '@react-types/shared';
+import {HTMLAttributes, RefObject} from 'react';
+import {PressProps} from '@react-aria/interactions';
+import {TooltipProps} from '@react-types/tooltip';
+import {TooltipTriggerState} from '@react-stately/tooltip';
+import {useId} from '@react-aria/utils';
+import {useOverlay} from '@react-aria/overlays';
+
+interface TriggerRefProps extends DOMProps, HTMLAttributes<HTMLElement> {
+  ref: RefObject<HTMLElement | null>,
+}
+
+interface TooltipTriggerProps {
+  tooltipProps: TooltipProps,
+  triggerProps: TriggerRefProps,
+  state: TooltipTriggerState,
+  type: string
+}
+
+interface TooltipTriggerAria {
+  triggerProps: HTMLAttributes<HTMLElement> & PressProps
+  tooltipProps: HTMLAttributes<HTMLElement>
+}
+
+export function useTooltipTrigger(props: TooltipTriggerProps): TooltipTriggerAria {
+  let tooltipId = useId();
+  let {
+    tooltipProps,
+    triggerProps,
+    state,
+    type
+  } = props;
+
+  let onClose = () => {
+    state.setOpen(false);
+  };
+
+  let {overlayProps} = useOverlay({
+    ref: triggerProps.ref,
+    onClose: onClose,
+    isOpen: state.open
+  });
+
+  let onKeyDownTrigger = (e) => {
+    if (triggerProps.ref && triggerProps.ref.current) {
+      // dismiss tooltip on esc key press
+      if (e.key === 'Escape') {
+        e.preventDefault();
+        e.stopPropagation();
+        state.setOpen(false);
+      }
+    }
+  };
+
+  let onPress = () => {
+    state.setOpen(!state.open);
+  };
+
+  let triggerType = type;
+
+  return {
+    triggerProps: {
+      ...tooltipProps,
+      ...overlayProps,
+      'aria-describedby': tooltipId,
+      onKeyDown: chain(triggerProps.onKeyDown, onKeyDownTrigger),
+      onPress: triggerType === 'click' ? onPress : undefined
+    },
+    tooltipProps: {
+      id: tooltipId
+    }
+  };
+}

packages/@react-aria/tooltip/test/useTooltip.test.js

@@ -0,0 +1,19 @@
+import {cleanup} from '@testing-library/react';
+import React from 'react';
+import {renderHook} from 'react-hooks-testing-library';
+import {useTooltip} from '../';
+
+describe('useTooltip', function () {
+  afterEach(cleanup);
+
+  let renderTooltipHook = (props) => {
+    let {result} = renderHook(() => useTooltip(props));
+    return result.current;
+  };
+
+  it('handles defaults', function () {
+    let {tooltipProps} = renderTooltipHook({children: 'Test Tooltip'});
+    expect(tooltipProps.role).toBe('tooltip');
+    expect(tooltipProps.id).toBeTruthy();
+  });
+});

packages/@react-spectrum/tooltip/package.json

@@ -34,6 +34,7 @@
     "@react-spectrum/overlays": "^3.0.0-alpha.2",
     "@react-spectrum/utils": "^3.0.0-alpha.2",
     "@react-stately/utils": "^3.0.0-alpha.2",
+    "@react-stately/tooltip": "^3.0.0-alpha.2",
     "@react-types/shared": "^3.0.0-rc.1"
   },
   "devDependencies": {

packages/@react-spectrum/tooltip/src/Tooltip.tsx

@@ -1,16 +1,10 @@
 import {classNames, filterDOMProps, useStyleProps} from '@react-spectrum/utils';
-import {DOMProps, StyleProps} from '@react-types/shared';
-import React, {ReactNode, RefObject, useRef} from 'react';
+import React, {RefObject, useRef} from 'react';
+import {SpectrumTooltipProps} from '@react-types/tooltip';
 import styles from '@adobe/spectrum-css-temp/components/tooltip/vars.css';
+import {useTooltip} from '@react-aria/tooltip';
 
-interface TooltipProps extends DOMProps, StyleProps {
-  children: ReactNode,
-  variant?: 'neutral' | 'positive' | 'negative' | 'info',
-  placement?: 'right' | 'left' | 'top' | 'bottom',
-  isOpen?: boolean
-}
-
-export const Tooltip = React.forwardRef((props: TooltipProps, ref: RefObject<HTMLDivElement>) => {
+export const Tooltip = React.forwardRef((props: SpectrumTooltipProps, ref: RefObject<HTMLDivElement>) => {
   ref = ref || useRef();
   let {
     variant = 'neutral',
@@ -19,11 +13,13 @@ export const Tooltip = React.forwardRef((props: TooltipProps, ref: RefObject<HTM
     ...otherProps
   } = props;
   let {styleProps} = useStyleProps(otherProps);
+  let {tooltipProps} = useTooltip(props);
 
   return (
     <div
       {...filterDOMProps(otherProps)}
       {...styleProps}
+      {...tooltipProps}
       className={classNames(
         styles,
         'spectrum-Tooltip',

packages/@react-spectrum/tooltip/src/TooltipTrigger.tsx

@@ -1,42 +1,40 @@
 import {DOMRefValue} from '@react-types/shared';
 import {Overlay} from '@react-spectrum/overlays';
-import {PositionProps, useOverlayPosition} from '@react-aria/overlays';
 import {PressResponder} from '@react-aria/interactions';
-import React, {Fragment, ReactElement, RefObject, useRef} from 'react';
+import React, {Fragment, useRef} from 'react';
+import {TooltipTriggerProps} from '@react-types/tooltip';
 import {unwrapDOMRef} from '@react-spectrum/utils';
-import {useControlledState} from '@react-stately/utils';
-
-interface TooltipTriggerProps extends PositionProps {
-  children: ReactElement[],
-  type?: 'click',
-  targetRef?: RefObject<HTMLElement>,
-  isOpen?: boolean,
-  defaultOpen?: boolean,
-  onOpenChange?: (isOpen: boolean) => void
-}
+import {useOverlayPosition} from '@react-aria/overlays';
+import {useTooltipTrigger} from '@react-aria/tooltip';
+import {useTooltipTriggerState} from '@react-stately/tooltip';
 
 export function TooltipTrigger(props: TooltipTriggerProps) {
   let {
     children,
     type,
     targetRef,
     isOpen,
-    defaultOpen,
-    onOpenChange
+    isDisabled
   } = props;
 
   let [trigger, content] = React.Children.toArray(children);
 
-  let [open, setOpen] = useControlledState(isOpen, defaultOpen || false, onOpenChange);
-
-  let onInteraction = () => {
-    setOpen(!open);
-  };
+  let state = useTooltipTriggerState(props);
 
   let containerRef = useRef<DOMRefValue<HTMLDivElement>>();
   let triggerRef = useRef<HTMLElement>();
   let overlayRef = useRef<HTMLDivElement>();
 
+  let {triggerProps, tooltipProps} = useTooltipTrigger({
+    tooltipProps: content.props,
+    triggerProps: {
+      ...trigger.props,
+      ref: triggerRef
+    },
+    state,
+    type
+  });
+
   let {overlayProps, placement, arrowProps} = useOverlayPosition({
     placement: props.placement,
     containerRef: unwrapDOMRef(containerRef),
@@ -47,23 +45,20 @@ export function TooltipTrigger(props: TooltipTriggerProps) {
 
   delete overlayProps.style.position;
 
-  let triggerPropsWithRef = {
-    ref: triggerRef
-  };
-
   let overlay = (
-    <Overlay isOpen={open} ref={containerRef}>
-      {React.cloneElement(content, {placement: placement, arrowProps: arrowProps, ref: overlayRef, ...overlayProps, isOpen: open})}
+    <Overlay isOpen={state.open} ref={containerRef}>
+      {React.cloneElement(content, {placement: placement, arrowProps: arrowProps, ref: overlayRef, UNSAFE_style: overlayProps.style, isOpen: open, ...tooltipProps})}
     </Overlay>
   );
 
   if (type === 'click') {
     return (
       <Fragment>
         <PressResponder
-          {...triggerPropsWithRef}
+          {...triggerProps}
+          ref={triggerRef}
           isPressed={isOpen}
-          onPress={onInteraction}>
+          isDisabled={isDisabled}>
           {trigger}
         </PressResponder>
         {overlay}

packages/@react-spectrum/tooltip/stories/Tooltip.stories.tsx

@@ -41,17 +41,20 @@ storiesOf('Tooltip', module)
     () => render(longMarkup)
   )
   .add(
-    'triggered by click, placement: right',
-    () => renderWithTrigger('This is a tooltip.', {placement: 'right', type: 'click'})
-  ).add(
     'triggered by click, placement: left',
-    () => renderWithTrigger('This is a tooltip.', {placement: 'left', type: 'click'})
+    () => renderWithTrigger('This is a tooltip.', {placement: 'start', type: 'click'})
+  ).add(
+    'triggered by click, placement: right',
+    () => renderWithTrigger('This is a tooltip.', {placement: 'end', type: 'click'})
   ).add(
     'triggered by click, placement: top',
     () => renderWithTrigger('This is a tooltip.', {placement: 'top', type: 'click'})
   ).add(
     'triggered by click, placement: bottom',
     () => renderWithTrigger('This is a tooltip.', {placement: 'bottom', type: 'click'})
+  ).add(
+    'isDisabled: true',
+    () => renderWithTrigger('This is a tooltip.', {placement: 'left', type: 'click', isDisabled: true})
   );
 
 function render(content, props = {}) {
@@ -69,7 +72,7 @@ function render(content, props = {}) {
 function renderWithTrigger(content, props = {}) {
   return (
     <TooltipTrigger {...props}>
-      <ActionButton>Click Me</ActionButton>
+      <ActionButton>Trigger Tooltip</ActionButton>
       <Tooltip>
         {content}
       </Tooltip>

packages/@react-spectrum/tooltip/test/Tooltip.test.js

@@ -0,0 +1,19 @@
+import {cleanup, render} from '@testing-library/react';
+import React from 'react';
+import {Tooltip} from '../';
+import V2Tooltip from '@react/react-spectrum/Tooltip';
+
+describe('Tooltip', function () {
+  afterEach(() => {
+    cleanup();
+  });
+
+  it.each`
+  Name           | Component
+  ${'Tooltip'}   | ${Tooltip}
+  ${'V2Tooltip'} | ${V2Tooltip}
+  `('$Name supports children', ({Component}) => {
+    let {getByText} = render(<Component>This is a tooltip</Component>);
+    expect(getByText('This is a tooltip')).toBeTruthy();
+  });
+});