From 679a841baeafcad6ce3beb84208792164d196cc4 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Wed, 11 Sep 2024 21:59:20 +0530 Subject: [PATCH 01/14] Revert "[IconButton] Fix hover background color behavior (#43271)" This reverts commit d9d0a37d6e11b83b3fefa716915cb1161a7a91db. --- .../mui-material/src/IconButton/IconButton.js | 3 -- .../src/IconButton/IconButton.test.js | 32 +------------------ 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.js b/packages/mui-material/src/IconButton/IconButton.js index 05747642096d82..38237214ef8798 100644 --- a/packages/mui-material/src/IconButton/IconButton.js +++ b/packages/mui-material/src/IconButton/IconButton.js @@ -164,7 +164,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) { color = 'default', disabled = false, disableFocusRipple = false, - disableRipple = false, size = 'medium', ...other } = props; @@ -175,7 +174,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) { color, disabled, disableFocusRipple, - disableRipple, size, }; @@ -187,7 +185,6 @@ const IconButton = React.forwardRef(function IconButton(inProps, ref) { centerRipple focusRipple={!disableFocusRipple} disabled={disabled} - disableRipple={disableRipple} ref={ref} {...other} ownerState={ownerState} diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 76011f1d2dbf29..25ca0ca6b2b9d3 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -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'; @@ -141,34 +141,4 @@ describe('', () => { )).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(); - - 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( - , - ); - - fireEvent.mouseMove(container.firstChild, { - clientX: 19, - }); - expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); - }); }); From 7e677a4f787195db3046a298d24284fb808accd3 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Wed, 11 Sep 2024 22:06:38 +0530 Subject: [PATCH 02/14] add tests --- .../mui-material/src/IconButton/IconButton.js | 2 +- .../src/IconButton/IconButton.test.js | 52 ++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.js b/packages/mui-material/src/IconButton/IconButton.js index 38237214ef8798..20dd25ed775f28 100644 --- a/packages/mui-material/src/IconButton/IconButton.js +++ b/packages/mui-material/src/IconButton/IconButton.js @@ -55,7 +55,7 @@ const IconButtonRoot = styled(ButtonBase, { }), variants: [ { - props: { disableRipple: false }, + props: (props) => !props.disableRipple, style: { '&:hover': { backgroundColor: theme.vars diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 25ca0ca6b2b9d3..ed97499b8b61f1 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -1,7 +1,7 @@ import * as React from 'react'; import { expect } from 'chai'; import PropTypes from 'prop-types'; -import { createRenderer, reactMajor } from '@mui/internal-test-utils'; +import { createRenderer, reactMajor, fireEvent } 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'; @@ -124,6 +124,56 @@ describe('', () => { }).toErrorDev(['MUI: You are providing an onClick event listener']); }); + it('should apply the hover background by default', function test() { + if (!/jsdom/.test(window.navigator.userAgent)) { + this.skip(); + } + + const { container, getByTestId } = render(); + + 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( + , + ); + + fireEvent.mouseMove(container.firstChild, { + clientX: 19, + }); + expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); + }); + + it('should disableRipple if set in MuiButtonBase', async () => { + const { container, getByRole } = render( + + book, + , + ); + await ripple.startTouch(getByRole('button')); + expect(container.querySelector('.touch-ripple')).to.equal(null); + }); + it('should not throw error for a custom color', () => { expect(() => ( Date: Wed, 11 Sep 2024 22:13:31 +0530 Subject: [PATCH 03/14] fix test --- packages/mui-material/src/IconButton/IconButton.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index ed97499b8b61f1..888c62188afa57 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -167,7 +167,7 @@ describe('', () => { }, })} > - book, + book, , ); await ripple.startTouch(getByRole('button')); From afb666bfff55a7d7dd35f3e196d11479b72f8020 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Wed, 11 Sep 2024 22:20:49 +0530 Subject: [PATCH 04/14] fix diff --- .../src/IconButton/IconButton.test.js | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 888c62188afa57..c440562b166306 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -124,6 +124,24 @@ describe('', () => { }).toErrorDev(['MUI: You are providing an onClick event listener']); }); + it('should not throw error for a custom color', () => { + expect(() => ( + + + + )).not.to.throw(); + }); + it('should apply the hover background by default', function test() { if (!/jsdom/.test(window.navigator.userAgent)) { this.skip(); @@ -154,7 +172,7 @@ describe('', () => { expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); }); - it('should disableRipple if set in MuiButtonBase', async () => { + it('should disable ripple if disableRipple is set in MuiButtonBase', async () => { const { container, getByRole } = render( ', () => { await ripple.startTouch(getByRole('button')); expect(container.querySelector('.touch-ripple')).to.equal(null); }); - - it('should not throw error for a custom color', () => { - expect(() => ( - - - - )).not.to.throw(); - }); }); From 5e5c25873d8ea5e81c287b0ef42c2911546e702b Mon Sep 17 00:00:00 2001 From: sai6855 Date: Wed, 11 Sep 2024 22:22:00 +0530 Subject: [PATCH 05/14] fix diff --- packages/mui-material/src/IconButton/IconButton.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index c440562b166306..1aef852082a6f8 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -172,7 +172,7 @@ describe('', () => { expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); }); - it('should disable ripple if disableRipple is set in MuiButtonBase', async () => { + it('should disableRipple if set in MuiButtonBase', async () => { const { container, getByRole } = render( Date: Wed, 11 Sep 2024 22:23:31 +0530 Subject: [PATCH 06/14] Update IconButton.test.js --- packages/mui-material/src/IconButton/IconButton.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 1aef852082a6f8..3abf1a796ecaad 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -172,7 +172,7 @@ describe('', () => { expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); }); - it('should disableRipple if set in MuiButtonBase', async () => { + it('should disable ripple if disableRipple:true is set in MuiButtonBase', async () => { const { container, getByRole } = render( Date: Wed, 11 Sep 2024 17:54:23 -0300 Subject: [PATCH 07/14] Apply fix to the colored buttons as well --- packages/mui-material/src/IconButton/IconButton.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.js b/packages/mui-material/src/IconButton/IconButton.js index 20dd25ed775f28..af323bd55ff756 100644 --- a/packages/mui-material/src/IconButton/IconButton.js +++ b/packages/mui-material/src/IconButton/IconButton.js @@ -55,7 +55,7 @@ const IconButtonRoot = styled(ButtonBase, { }), variants: [ { - props: (props) => !props.disableRipple, + props: (props) => !props.color && !props.disableRipple, style: { '&:hover': { backgroundColor: theme.vars @@ -113,7 +113,7 @@ 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: (props) => props.color === color && !props.disableRipple, style: { '&:hover': { backgroundColor: theme.vars From a82d288aa346b113eef3595bb381efc4092aeb31 Mon Sep 17 00:00:00 2001 From: DiegoAndai Date: Wed, 11 Sep 2024 18:10:39 -0300 Subject: [PATCH 08/14] Remove incorrect style rule --- packages/mui-material/src/IconButton/IconButton.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/IconButton/IconButton.js b/packages/mui-material/src/IconButton/IconButton.js index af323bd55ff756..40e8640c762f19 100644 --- a/packages/mui-material/src/IconButton/IconButton.js +++ b/packages/mui-material/src/IconButton/IconButton.js @@ -55,7 +55,7 @@ const IconButtonRoot = styled(ButtonBase, { }), variants: [ { - props: (props) => !props.color && !props.disableRipple, + props: (props) => !props.disableRipple, style: { '&:hover': { backgroundColor: theme.vars From d04a8d468ee9eb15442d9509902fa5fa129361ed Mon Sep 17 00:00:00 2001 From: DiegoAndai Date: Wed, 11 Sep 2024 18:14:48 -0300 Subject: [PATCH 09/14] Add test for colored IconButton hover --- .../src/IconButton/IconButton.test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 3abf1a796ecaad..6de989264d46b4 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -157,6 +157,23 @@ describe('', () => { }); }); + it('should apply the colored hover background if color is provided', function test() { + if (!/jsdom/.test(window.navigator.userAgent)) { + this.skip(); + } + + const { container, getByTestId } = render( + , + ); + + fireEvent.mouseMove(container.firstChild, { + clientX: 19, + }); + expect(getByTestId('icon-button')).toHaveComputedStyle({ + backgroundColor: 'rgba(25, 118, 210, 0.04)', + }); + }); + it('should not apply the hover background if disableRipple is true', function test() { if (!/jsdom/.test(window.navigator.userAgent)) { this.skip(); From 02e1663fb216addf22a8644ac49249c846c96b0e Mon Sep 17 00:00:00 2001 From: siriwatknp Date: Thu, 12 Sep 2024 19:53:19 +0700 Subject: [PATCH 10/14] switch to CSS variable --- .../mui-material/src/IconButton/IconButton.js | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.js b/packages/mui-material/src/IconButton/IconButton.js index 40e8640c762f19..780a3ff72d3d19 100644 --- a/packages/mui-material/src/IconButton/IconButton.js +++ b/packages/mui-material/src/IconButton/IconButton.js @@ -57,10 +57,11 @@ const IconButtonRoot = styled(ButtonBase, { { 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', @@ -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: (props) => props.color === color && !props.disableRipple, + 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), }, })), { From 96e0f14e81e4603cedc6fbe108d65321476d1f6a Mon Sep 17 00:00:00 2001 From: DiegoAndai Date: Fri, 13 Sep 2024 13:06:42 -0300 Subject: [PATCH 11/14] Add experimental test --- .../IconButton/IconButtonDefaultHoverStyle.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js diff --git a/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js b/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js new file mode 100644 index 00000000000000..98f89c242e56b0 --- /dev/null +++ b/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js @@ -0,0 +1,34 @@ +import * as React from 'react'; +import { createTheme, ThemeProvider } from '@mui/material/styles'; +import IconButton from '@mui/material/IconButton'; +import DeleteIcon from '@mui/icons-material/Delete'; + +const theme = createTheme({}); + +const HOVER_MOCK_CLASS = 'hoverMock'; + +function copyHoverStyles() { + const styleSheet = document.styleSheets[0]; + const hoverRule = Array.from(styleSheet.cssRules) + .filter((rule) => rule instanceof CSSStyleRule) + .find((rule) => rule.selectorText.includes('MuiIconButton-root:hover')); + + if (!hoverRule) return; + + const newRule = `.${HOVER_MOCK_CLASS} { ${hoverRule.style.cssText} }`; + styleSheet.insertRule(newRule, styleSheet.cssRules.length); +} + +export default function IconButtons() { + React.useLayoutEffect(() => { + copyHoverStyles(); + }, []); + + return ( + + + + + + ); +} From f843e03c254359c8c4812b2eb965c044ce5be0e8 Mon Sep 17 00:00:00 2001 From: DiegoAndai Date: Fri, 13 Sep 2024 14:51:55 -0300 Subject: [PATCH 12/14] Remove experimental test --- .../IconButton/IconButtonDefaultHoverStyle.js | 34 ------------------- 1 file changed, 34 deletions(-) delete mode 100644 test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js diff --git a/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js b/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js deleted file mode 100644 index 98f89c242e56b0..00000000000000 --- a/test/regressions/fixtures/IconButton/IconButtonDefaultHoverStyle.js +++ /dev/null @@ -1,34 +0,0 @@ -import * as React from 'react'; -import { createTheme, ThemeProvider } from '@mui/material/styles'; -import IconButton from '@mui/material/IconButton'; -import DeleteIcon from '@mui/icons-material/Delete'; - -const theme = createTheme({}); - -const HOVER_MOCK_CLASS = 'hoverMock'; - -function copyHoverStyles() { - const styleSheet = document.styleSheets[0]; - const hoverRule = Array.from(styleSheet.cssRules) - .filter((rule) => rule instanceof CSSStyleRule) - .find((rule) => rule.selectorText.includes('MuiIconButton-root:hover')); - - if (!hoverRule) return; - - const newRule = `.${HOVER_MOCK_CLASS} { ${hoverRule.style.cssText} }`; - styleSheet.insertRule(newRule, styleSheet.cssRules.length); -} - -export default function IconButtons() { - React.useLayoutEffect(() => { - copyHoverStyles(); - }, []); - - return ( - - - - - - ); -} From 4eaffa28c1310d5ed4b713312ed1dd8997db51c7 Mon Sep 17 00:00:00 2001 From: DiegoAndai Date: Tue, 8 Oct 2024 15:52:39 -0300 Subject: [PATCH 13/14] Remove hover tests --- .../src/IconButton/IconButton.test.js | 47 ------------------- 1 file changed, 47 deletions(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index 6de989264d46b4..de3528c4f05347 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -142,53 +142,6 @@ describe('', () => { )).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(); - - fireEvent.mouseMove(container.firstChild, { - clientX: 19, - }); - expect(getByTestId('icon-button')).toHaveComputedStyle({ - backgroundColor: 'rgba(0, 0, 0, 0.04)', - }); - }); - - it('should apply the colored hover background if color is provided', function test() { - if (!/jsdom/.test(window.navigator.userAgent)) { - this.skip(); - } - - const { container, getByTestId } = render( - , - ); - - fireEvent.mouseMove(container.firstChild, { - clientX: 19, - }); - expect(getByTestId('icon-button')).toHaveComputedStyle({ - backgroundColor: 'rgba(25, 118, 210, 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( - , - ); - - fireEvent.mouseMove(container.firstChild, { - clientX: 19, - }); - expect(getByTestId('icon-button')).toHaveComputedStyle({ backgroundColor: 'rgba(0, 0, 0, 0)' }); - }); - it('should disable ripple if disableRipple:true is set in MuiButtonBase', async () => { const { container, getByRole } = render( Date: Tue, 8 Oct 2024 16:06:43 -0300 Subject: [PATCH 14/14] Remove unused import --- packages/mui-material/src/IconButton/IconButton.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/IconButton/IconButton.test.js b/packages/mui-material/src/IconButton/IconButton.test.js index de3528c4f05347..827d2c91fb560d 100644 --- a/packages/mui-material/src/IconButton/IconButton.test.js +++ b/packages/mui-material/src/IconButton/IconButton.test.js @@ -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';