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

[material-ui][IconButton] Fix disableRipple behaviour when disableRipple is set in MuiButtonBase theme #43714

Merged
merged 18 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions packages/mui-material/src/IconButton/IconButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ const IconButtonRoot = styled(ButtonBase, {
}),
variants: [
{
props: { disableRipple: false },
props: (props) => !props.disableRipple,
style: {
'--IconButton-hoverBg': theme.vars
? `rgba(${theme.vars.palette.action.activeChannel} / ${theme.vars.palette.action.hoverOpacity})`
: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
'&:hover': {
backgroundColor: theme.vars
? `rgba(${theme.vars.palette.action.activeChannel} / ${theme.vars.palette.action.hoverOpacity})`
: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
backgroundColor: 'var(--IconButton-hoverBg)',
// Reset on touch devices, it doesn't add specificity
'@media (hover: none)': {
backgroundColor: 'transparent',
Expand Down Expand Up @@ -113,20 +114,11 @@ const IconButtonRoot = styled(ButtonBase, {
...Object.entries(theme.palette)
.filter(createSimplePaletteValueFilter()) // check all the used fields in the style below
.map(([color]) => ({
props: { color, disableRipple: false },
props: { color },
style: {
'&:hover': {
backgroundColor: theme.vars
? `rgba(${(theme.vars || theme).palette[color].mainChannel} / ${theme.vars.palette.action.hoverOpacity})`
: alpha(
(theme.vars || theme).palette[color].main,
theme.palette.action.hoverOpacity,
),
// Reset on touch devices, it doesn't add specificity
'@media (hover: none)': {
backgroundColor: 'transparent',
},
},
'--IconButton-hoverBg': theme.vars
? `rgba(${(theme.vars || theme).palette[color].mainChannel} / ${theme.vars.palette.action.hoverOpacity})`
: alpha((theme.vars || theme).palette[color].main, theme.palette.action.hoverOpacity),
},
})),
{
Expand Down Expand Up @@ -164,7 +156,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) {
color = 'default',
disabled = false,
disableFocusRipple = false,
disableRipple = false,
size = 'medium',
...other
} = props;
Expand All @@ -175,7 +166,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) {
color,
disabled,
disableFocusRipple,
disableRipple,
size,
};

Expand All @@ -187,7 +177,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) {
centerRipple
focusRipple={!disableFocusRipple}
disabled={disabled}
disableRipple={disableRipple}
ref={ref}
{...other}
ownerState={ownerState}
Expand Down
46 changes: 18 additions & 28 deletions packages/mui-material/src/IconButton/IconButton.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { expect } from 'chai';
import PropTypes from 'prop-types';
import { createRenderer, reactMajor, fireEvent } from '@mui/internal-test-utils';
import { createRenderer, reactMajor } from '@mui/internal-test-utils';
import capitalize from '@mui/utils/capitalize';
import { ThemeProvider, createTheme } from '@mui/material/styles';
import IconButton, { iconButtonClasses as classes } from '@mui/material/IconButton';
Expand Down Expand Up @@ -142,33 +142,23 @@ describe('<IconButton />', () => {
)).not.to.throw();
});

it('should apply the hover background by default', function test() {
if (!/jsdom/.test(window.navigator.userAgent)) {
this.skip();
}

const { container, getByTestId } = render(<IconButton data-testid="icon-button" />);

fireEvent.mouseMove(container.firstChild, {
clientX: 19,
});
expect(getByTestId('icon-button')).toHaveComputedStyle({
backgroundColor: 'rgba(0, 0, 0, 0.04)',
});
});

it('should not apply the hover background if disableRipple is true', function test() {
if (!/jsdom/.test(window.navigator.userAgent)) {
this.skip();
}

const { container, getByTestId } = render(
<IconButton disableRipple data-testid="icon-button" />,
it('should disable ripple if disableRipple:true is set in MuiButtonBase', async () => {
const { container, getByRole } = render(
<ThemeProvider
theme={createTheme({
components: {
MuiButtonBase: {
defaultProps: {
disableRipple: true,
},
},
},
})}
>
<IconButton TouchRippleProps={{ className: 'touch-ripple' }}>book</IconButton>,
</ThemeProvider>,
);

fireEvent.mouseMove(container.firstChild, {
clientX: 19,
});
expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' });
await ripple.startTouch(getByRole('button'));
expect(container.querySelector('.touch-ripple')).to.equal(null);
});
});