From a33fcdf5a5d0fcfee4d91447930b354702c4c775 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 14 Aug 2023 23:38:30 +0800 Subject: [PATCH 1/7] Revert "[TextField] fix running click event on disabled (#36892)" This reverts commit 9118505019c3f9c2b3a837d78e582a101b135ffa. --- .../mui-material/src/InputBase/InputBase.js | 3 ++- .../mui-material/src/TextField/TextField.d.ts | 4 ---- .../mui-material/src/TextField/TextField.js | 6 ------ .../src/TextField/TextField.test.js | 19 +------------------ 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/packages/mui-material/src/InputBase/InputBase.js b/packages/mui-material/src/InputBase/InputBase.js index 6f948212ec8afc..90d0b3a40851ea 100644 --- a/packages/mui-material/src/InputBase/InputBase.js +++ b/packages/mui-material/src/InputBase/InputBase.js @@ -440,7 +440,8 @@ const InputBase = React.forwardRef(function InputBase(inProps, ref) { if (inputRef.current && event.currentTarget === event.target) { inputRef.current.focus(); } - if (onClick && !fcs.disabled) { + + if (onClick) { onClick(event); } }; diff --git a/packages/mui-material/src/TextField/TextField.d.ts b/packages/mui-material/src/TextField/TextField.d.ts index 468affd862e276..33b83fb1c04450 100644 --- a/packages/mui-material/src/TextField/TextField.d.ts +++ b/packages/mui-material/src/TextField/TextField.d.ts @@ -111,10 +111,6 @@ export interface BaseTextFieldProps name?: string; onBlur?: InputBaseProps['onBlur']; onFocus?: StandardInputProps['onFocus']; - /** - * @ignore - */ - onClick?: InputBaseProps['onClick']; /** * The short hint displayed in the `input` before the user enters a value. */ diff --git a/packages/mui-material/src/TextField/TextField.js b/packages/mui-material/src/TextField/TextField.js index 2448ce22274d89..c41caa67fe3020 100644 --- a/packages/mui-material/src/TextField/TextField.js +++ b/packages/mui-material/src/TextField/TextField.js @@ -95,7 +95,6 @@ const TextField = React.forwardRef(function TextField(inProps, ref) { name, onBlur, onChange, - onClick, onFocus, placeholder, required = false, @@ -179,7 +178,6 @@ const TextField = React.forwardRef(function TextField(inProps, ref) { onBlur={onBlur} onChange={onChange} onFocus={onFocus} - onClick={handleClick} placeholder={placeholder} inputProps={inputProps} {...InputMore} @@ -358,10 +356,6 @@ TextField.propTypes /* remove-proptypes */ = { * You can pull out the new value by accessing `event.target.value` (string). */ onChange: PropTypes.func, - /** - * @ignore - */ - onClick: PropTypes.func, /** * @ignore */ diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index 18b3b0e5d61829..49c1c48fd63af7 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -1,7 +1,6 @@ import * as React from 'react'; import { expect } from 'chai'; -import { spy } from 'sinon'; -import { createRenderer, describeConformance, fireEvent } from 'test/utils'; +import { createRenderer, describeConformance } from 'test/utils'; import FormControl from '@mui/material/FormControl'; import { inputBaseClasses } from '@mui/material/InputBase'; import MenuItem from '@mui/material/MenuItem'; @@ -155,22 +154,6 @@ describe('', () => { }); }); - describe('prop: disabled', () => { - it('should not run click event when disabled', () => { - const handleClick = spy(); - const { getByRole } = render(); - fireEvent.click(getByRole('textbox')); - expect(handleClick.callCount).to.equal(0); - }); - - it('should not run click event when disabled and when onClick prop is set through InputProps', () => { - const handleClick = spy(); - const { getByRole } = render(); - fireEvent.click(getByRole('textbox')); - expect(handleClick.callCount).to.equal(0); - }); - }); - describe('prop: select', () => { it('can render a is disabled', () => { + const handleClick = spy(); + const { getByRole } = render(); + const input = getByRole('textbox'); + fireEvent.click(input); + expect(handleClick.callCount).to.equal(1); + }); }); describe('prop: readonly', () => { diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index 8987dcd335089c..9f127c1c2dcc8d 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -1,6 +1,7 @@ import * as React from 'react'; import { expect } from 'chai'; -import { createRenderer, describeConformance } from 'test/utils'; +import { spy } from 'sinon'; +import { createRenderer, describeConformance, fireEvent } from 'test/utils'; import FormControl from '@mui/material/FormControl'; import { inputBaseClasses } from '@mui/material/InputBase'; import MenuItem from '@mui/material/MenuItem'; @@ -226,4 +227,15 @@ describe('', () => { expect(getByRole('button')).toHaveAccessibleDescription('Foo bar'); }); }); + + describe('click handling', () => { + it('should trigger `onClick` only once', () => { + const handleClick = spy(); + const { getByRole } = render( + , + ); + fireEvent.click(getByRole('textbox')); + expect(handleClick.callCount).to.equal(1); + }); + }); }); From 68dc36146385387d2b3ec19b365bb856812a23d4 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Thu, 17 Aug 2023 14:59:07 +0800 Subject: [PATCH 5/7] Test onClick is registered on TextField root --- .../src/TextField/TextField.test.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index 9f127c1c2dcc8d..9cebf9b7526b65 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -228,7 +228,7 @@ describe('', () => { }); }); - describe('click handling', () => { + describe('event: click', () => { it('should trigger `onClick` only once', () => { const handleClick = spy(); const { getByRole } = render( @@ -237,5 +237,23 @@ describe('', () => { fireEvent.click(getByRole('textbox')); expect(handleClick.callCount).to.equal(1); }); + + it('registers `onClick` on the root slot', () => { + const handleClick = spy(); + const { getByTestId } = render( + , + ); + + const root = getByTestId('root'); + + fireEvent.click(root); + expect(handleClick.callCount).to.equal(1); + }); }); }); From 2b2c35b97555122a33fa370c8e216c469e27ce08 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 21 Aug 2023 16:21:20 +0800 Subject: [PATCH 6/7] Update test --- .../src/TextField/TextField.test.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index 9cebf9b7526b65..0b229d8e96fe4d 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -239,21 +239,20 @@ describe('', () => { }); it('registers `onClick` on the root slot', () => { - const handleClick = spy(); - const { getByTestId } = render( - , + const handleClick = spy((event) => event.currentTarget); + const { getByTestId, getByRole } = render( + , ); + const input = getByRole('textbox'); + const root = getByTestId('root'); - fireEvent.click(root); + fireEvent.click(input); + expect(handleClick.callCount).to.equal(1); + // return value is event.currentTarget + expect(handleClick.returned(root)); }); }); }); From 886d3d87c598b8f291a7a5d1fa05ff8c90d906eb Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Tue, 22 Aug 2023 14:26:44 +0800 Subject: [PATCH 7/7] Remove redundant test --- packages/mui-material/src/TextField/TextField.test.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index 0b229d8e96fe4d..b793e8f0b01f2d 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -229,15 +229,6 @@ describe('', () => { }); describe('event: click', () => { - it('should trigger `onClick` only once', () => { - const handleClick = spy(); - const { getByRole } = render( - , - ); - fireEvent.click(getByRole('textbox')); - expect(handleClick.callCount).to.equal(1); - }); - it('registers `onClick` on the root slot', () => { const handleClick = spy((event) => event.currentTarget); const { getByTestId, getByRole } = render(