Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SpeedDial] Reset tooltip state when the speed dial is closed #25259

Merged
merged 11 commits into from
Mar 12, 2021
9 changes: 8 additions & 1 deletion packages/material-ui/src/SpeedDial/SpeedDial.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,13 @@ const SpeedDial = React.forwardRef(function SpeedDial(inProps, ref) {
});

const children = allItems.map((child, index) => {
const { FabProps: { ref: origButtonRef, ...ChildFabProps } = {} } = child.props;
const {
FabProps: { ref: origButtonRef, ...ChildFabProps } = {},
tooltipPlacement: tooltipPlacementProp,
} = child.props;

const tooltipPlacement =
tooltipPlacementProp || (getOrientation(direction) === 'vertical' ? 'left' : 'top');

return React.cloneElement(child, {
FabProps: {
Expand All @@ -369,6 +375,7 @@ const SpeedDial = React.forwardRef(function SpeedDial(inProps, ref) {
},
delay: 30 * (open ? index : allItems.length - index),
open,
tooltipPlacement,
id: `${id}-action-${index}`,
});
});
Expand Down
122 changes: 111 additions & 11 deletions packages/material-ui/src/SpeedDial/SpeedDial.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,26 @@ import {
createClientRender,
act,
fireEvent,
fireDiscreteEvent,
screen,
describeConformanceV5,
} from 'test/utils';
import Icon from '@material-ui/core/Icon';
import SpeedDial, { speedDialClasses as classes } from '@material-ui/core/SpeedDial';
import SpeedDialAction from '@material-ui/core/SpeedDialAction';
import { tooltipClasses } from '@material-ui/core/Tooltip';

describe('<SpeedDial />', () => {
let clock;

beforeEach(() => {
clock = useFakeTimers();
});

afterEach(() => {
clock.restore();
});

// StrictModeViolation: not using act(), prefer test/utils/createClientRender
const mount = createMount({ strict: false });
const render = createClientRender({ strict: false });
Expand Down Expand Up @@ -85,6 +97,42 @@ describe('<SpeedDial />', () => {
expect(actions.map((element) => element.className)).not.to.contain('is-closed');
});

it('should reset the state of the tooltip when the speed dial is closed while it is open', () => {
const { queryByRole, getByRole, getAllByRole } = render(
<SpeedDial icon={icon} ariaLabel="mySpeedDial">
<SpeedDialAction icon={icon} tooltipTitle="SpeedDialAction1" />
<SpeedDialAction icon={icon} tooltipTitle="SpeedDialAction2" />
</SpeedDial>,
);
const fab = getByRole('button');
const actions = getAllByRole('menuitem');

fireEvent.mouseEnter(fab);
act(() => {
clock.runAll();
});
expect(fab).to.have.attribute('aria-expanded', 'true');

fireEvent.mouseOver(actions[0]);
act(() => {
clock.runAll();
});
expect(queryByRole('tooltip')).not.to.equal(null);

fireEvent.mouseLeave(actions[0]);
act(() => {
clock.runAll();
});
expect(fab).to.have.attribute('aria-expanded', 'false');

fireEvent.mouseEnter(fab);
act(() => {
clock.runAll();
});
expect(queryByRole('tooltip')).to.equal(null);
expect(fab).to.have.attribute('aria-expanded', 'true');
});

describe('prop: onKeyDown', () => {
it('should be called when a key is pressed', () => {
const handleKeyDown = spy();
Expand Down Expand Up @@ -121,19 +169,31 @@ describe('<SpeedDial />', () => {
expect(getByRole('presentation')).to.have.class(classes[className]);
});
});
});

describe('keyboard', () => {
let clock;

beforeEach(() => {
clock = useFakeTimers();
});

afterEach(() => {
clock.restore();
[
['up', 'tooltipPlacementLeft'],
['down', 'tooltipPlacementLeft'],
['left', 'tooltipPlacementTop'],
['right', 'tooltipPlacementTop'],
].forEach(([direction, className]) => {
it(`should place the tooltip in the correct position when direction=${direction}`, () => {
const { getByRole, getAllByRole } = render(
<SpeedDial {...defaultProps} open direction={direction.toLowerCase()}>
<SpeedDialAction icon={icon} tooltipTitle="action1" />
<SpeedDialAction icon={icon} tooltipTitle="action2" />
</SpeedDial>,
);
const actions = getAllByRole('menuitem');
fireEvent.mouseOver(actions[0]);
act(() => {
clock.runAll();
});
expect(getByRole('tooltip').firstChild).to.have.class(tooltipClasses[className]);
});
});
});

describe('keyboard', () => {
it('should open the speed dial and move to the first action without closing', () => {
const handleOpen = spy();
const { getByRole, getAllByRole } = render(
Expand All @@ -154,6 +214,46 @@ describe('<SpeedDial />', () => {
expect(document.activeElement).to.equal(actions[0]);
expect(fab).to.have.attribute('aria-expanded', 'true');
});

it('should reset the state of the tooltip when the speed dial is closed while it is open', () => {
const handleOpen = spy();
const { queryByRole, getByRole, getAllByRole } = render(
<SpeedDial ariaLabel="mySpeedDial" onOpen={handleOpen}>
<SpeedDialAction tooltipTitle="action1" />
<SpeedDialAction tooltipTitle="action2" />
</SpeedDial>,
);
const fab = getByRole('button');
const actions = getAllByRole('menuitem');

fab.focus();
act(() => {
clock.runAll();
});

expect(fab).to.have.attribute('aria-expanded', 'true');

fireEvent.keyDown(fab, { key: 'ArrowUp' });
act(() => {
clock.runAll();
});
expect(queryByRole('tooltip')).not.to.equal(null);

fireDiscreteEvent.keyDown(actions[0], { key: 'Escape' });
act(() => {
clock.runAll();
});

expect(queryByRole('tooltip')).to.equal(null);
expect(fab).to.have.attribute('aria-expanded', 'false');

fab.focus();
act(() => {
clock.runAll();
});
expect(queryByRole('tooltip')).to.equal(null);
expect(fab).to.have.attribute('aria-expanded', 'true');
});
});

describe('dial focus', () => {
Expand Down Expand Up @@ -224,7 +324,7 @@ describe('<SpeedDial />', () => {
it('displays the actions on focus gain', () => {
renderSpeedDial();
expect(screen.getAllByRole('menuitem')).to.have.lengthOf(4);
expect(screen.getByRole('menu')).not.to.have.class(classes.actionsClosed);
expect(fabButton).to.have.attribute('aria-expanded', 'true');
});

it('considers arrow keys with the same initial orientation', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/material-ui/src/SpeedDialAction/SpeedDialAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ const SpeedDialAction = React.forwardRef(function SpeedDialAction(inProps, ref)
);
}

if (!open && tooltipOpen) {
setTooltipOpen(false);
}

return (
<Tooltip
id={id}
Expand Down
33 changes: 30 additions & 3 deletions test/utils/fireDiscreteEvent.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
import { configure, fireEvent, getConfig } from '@testing-library/react';

const noWrapper = (callback) => callback();

/**
* @param {() => void} callback
* @returns {void}
*/
function withMissingActWarningsIgnored(callback) {
const originalConsoleError = console.error;
console.error = function silenceMissingActWarnings(message, ...args) {
Expand All @@ -6,9 +14,16 @@ function withMissingActWarningsIgnored(callback) {
originalConsoleError.call(console, message, ...args);
}
};

const originalConfig = getConfig();
configure({
eventWrapper: noWrapper,
});

try {
callback();
} finally {
configure(originalConfig);
console.error = originalConsoleError;
}
}
Expand All @@ -21,10 +36,22 @@ function withMissingActWarningsIgnored(callback) {
// Be aware that "discrete events" are an implementation detail of React.
// To test discrete events we cannot use `fireEvent` from `@testing-library/react` because they are all wrapped in `act`.
// `act` overrides the "discrete event" semantics with "batching" semantics: https://github.com/facebook/react/blob/3fbd47b86285b6b7bdeab66d29c85951a84d4525/packages/react-reconciler/src/ReactFiberWorkLoop.old.js#L1061-L1064
// Note that using `fireEvent` from `@testing-library/dom` would not work since /react configures both `fireEvent` to use `act` as a wrapper.
// -----------------------------------------

// eslint-disable-next-line import/prefer-default-export -- there are more than one discrete events.
export function click(element) {
// TODO: Why are there different semantics between `element.click` and `dtlFireEvent.click`
return withMissingActWarningsIgnored(() => element.click());
return withMissingActWarningsIgnored(() => {
fireEvent.click(element);
});
}

/**
* @param {Element} element
* @param {{}} [options]
* @returns {void}
*/
export function keyDown(element, options) {
return withMissingActWarningsIgnored(() => {
fireEvent.keyDown(element, options);
});
}