Skip to content

Commit

Permalink
Revert "stop propagation of clicks on tooltips (#4479)" (#4535)
Browse files Browse the repository at this point in the history
This reverts commit b677593.
  • Loading branch information
snowystinger authored May 16, 2023
1 parent a85ddb2 commit 86988dc
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 62 deletions.
5 changes: 2 additions & 3 deletions packages/@react-aria/tooltip/src/useTooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {AriaTooltipProps} from '@react-types/tooltip';
import {DOMAttributes} from '@react-types/shared';
import {filterDOMProps, mergeProps} from '@react-aria/utils';
import {TooltipTriggerState} from '@react-stately/tooltip';
import {useHover, usePress} from '@react-aria/interactions';
import {useHover} from '@react-aria/interactions';

export interface TooltipAria {
/**
Expand All @@ -28,15 +28,14 @@ export interface TooltipAria {
*/
export function useTooltip(props: AriaTooltipProps, state?: TooltipTriggerState): TooltipAria {
let domProps = filterDOMProps(props, {labelable: true});
let {pressProps} = usePress({});

let {hoverProps} = useHover({
onHoverStart: () => state?.open(true),
onHoverEnd: () => state?.close()
});

return {
tooltipProps: mergeProps(domProps, hoverProps, pressProps, {
tooltipProps: mergeProps(domProps, hoverProps, {
role: 'tooltip'
})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Delete from '@spectrum-icons/workflow/Delete';
import Edit from '@spectrum-icons/workflow/Edit';
import {Flex} from '@react-spectrum/layout';
import {Link} from '@react-spectrum/link';
import React, {CSSProperties, useState} from 'react';
import React, {useState} from 'react';
import SaveTo from '@spectrum-icons/workflow/SaveTo';
import {SpectrumTooltipTriggerProps} from '@react-types/tooltip';
import {Tooltip, TooltipTrigger} from '../src';
Expand Down Expand Up @@ -176,25 +176,6 @@ export const TooltripTriggerInsideActionGroup: TooltipTriggerStory = {
)
};

const TooltipDivRender = (props) => {
const [isDisabled, setIsDisabled] = useState(false);
const wrapperStyle: CSSProperties = {width: '400px', height: '400px', backgroundColor: 'red', position: 'absolute'};
return (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
<div onClick={() => setIsDisabled(!isDisabled)} style={wrapperStyle}>
<TooltipTrigger {...props} isOpen>
<ActionButton isDisabled={isDisabled} aria-label="Edit" UNSAFE_style={{top: '50px'}}>click red to disable</ActionButton>
<Tooltip>Click on tooltip doesn't propagate to parent</Tooltip>
</TooltipTrigger>
</div>
);
};
export const TooltripTriggerInsideDiv: TooltipTriggerStory = {
args: {delay: 0},
render: (args) => <TooltipDivRender {...args} />,
parameters: {description: {data: 'Event handlers are attached to the parent of the tooltip trigger. They should not be called when the tooltip itself is clicked.'}}
};

export const ArrowPositioningAtEdge: TooltipTriggerStory = {
args: {
children: [
Expand Down
11 changes: 1 addition & 10 deletions packages/@react-spectrum/tooltip/test/Tooltip.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import React from 'react';
import {render, triggerPress} from '@react-spectrum/test-utils';
import {render} from '@react-spectrum/test-utils';
import {Tooltip} from '../';

describe('Tooltip', function () {
Expand Down Expand Up @@ -40,13 +40,4 @@ describe('Tooltip', function () {
let tooltip = getByRole('tooltip');
expect(ref.current.UNSAFE_getDOMNode()).toBe(tooltip);
});

it('click does not propagate to parent', () => {
let mockClick = jest.fn();
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
let {getByRole} = render(<div onClick={mockClick}><Tooltip>This is a tooltip</Tooltip></div>);
let tooltip = getByRole('tooltip');
triggerPress(tooltip);
expect(mockClick).not.toHaveBeenCalled();
});
});
29 changes: 0 additions & 29 deletions packages/@react-spectrum/tooltip/test/TooltipTrigger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -976,33 +976,4 @@ describe('TooltipTrigger', function () {
expect(queryByRole('tooltip')).toBeNull();
});
});

it('does not propagate pointer or click through the portal', () => {
let onPointerDown = jest.fn();
let onPointerUp = jest.fn();
let onClick = jest.fn();

let {getByRole} = render(
<Provider theme={theme}>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */}
<div onPointerDown={onPointerDown} onPointerUp={onPointerUp} onClick={onClick}>
<TooltipTrigger>
<ActionButton>Trigger</ActionButton>
<Tooltip>Helpful information.</Tooltip>
</TooltipTrigger>
</div>
</Provider>
);

let button = getByRole('button');
act(() => {
button.focus();
});

let tooltip = getByRole('tooltip');
triggerPress(tooltip);
expect(onPointerDown).not.toHaveBeenCalled();
expect(onPointerUp).not.toHaveBeenCalled();
expect(onClick).not.toHaveBeenCalled();
});
});

1 comment on commit 86988dc

@rspbot
Copy link

@rspbot rspbot commented on 86988dc May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.