From 46a59c20441aecdbb820d033c63152dbf8da7fd8 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 9 Nov 2022 17:01:42 +0530 Subject: [PATCH 01/12] [Chip] avoided focus if chip is disabled --- packages/mui-material/src/Chip/Chip.js | 1 + packages/mui-material/src/Chip/Chip.test.js | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index 44bbe36d98464a..a7f3e915fb9541 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -460,6 +460,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} + tabIndex={disabled ? ownerState.tabIndex || -1 : ownerState.tabIndex} ownerState={ownerState} {...moreProps} {...other} diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index 1e1c2fcb23ea85..d5db3f69299872 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -174,6 +174,18 @@ describe('', () => { expect(chip).to.have.class(classes.filledPrimary); }); + it('should not be focused when chip is disabled', () => { + const { getByRole } = render(); + + const chip = getByRole('button'); + + fireEvent.keyDown(chip, { key: 'Tab' }); + + expect(chip).to.have.class(classes.root); + expect(chip).to.have.property('tabIndex', -1); + expect(chip).to.not.have.class(classes.focusVisible); + }); + it('should render with the root and filled clickable secondary class', () => { const { getByRole } = render( {}} variant="filled" />, From 9d0e2f9b7249ddf698df7fed95b925792718196e Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 9 Nov 2022 17:46:53 +0530 Subject: [PATCH 02/12] [Chip] fix test --- packages/mui-material/src/Chip/Chip.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index d5db3f69299872..fd91a642811959 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -175,11 +175,11 @@ describe('', () => { }); it('should not be focused when chip is disabled', () => { - const { getByRole } = render(); + const { getByTestId } = render(); - const chip = getByRole('button'); + const chip = getByTestId('chip'); - fireEvent.keyDown(chip, { key: 'Tab' }); + fireEvent.keyDown(document.body, { key: 'Tab' }); expect(chip).to.have.class(classes.root); expect(chip).to.have.property('tabIndex', -1); From 20b1ee974e69439da4e863ee979abf8b648691ae Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 9 Nov 2022 20:37:57 +0530 Subject: [PATCH 03/12] destructrured tab index, fix test --- packages/mui-material/src/Chip/Chip.d.ts | 4 ++++ packages/mui-material/src/Chip/Chip.js | 7 ++++++- packages/mui-material/src/Chip/Chip.test.js | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/mui-material/src/Chip/Chip.d.ts b/packages/mui-material/src/Chip/Chip.d.ts index 9ff2c7a2767374..176869163728fe 100644 --- a/packages/mui-material/src/Chip/Chip.d.ts +++ b/packages/mui-material/src/Chip/Chip.d.ts @@ -76,6 +76,10 @@ export interface ChipTypeMap

{ * The system prop that allows defining system overrides as well as additional CSS styles. */ sx?: SxProps; + /** + * tab order of an element + */ + tabIndex?: number; /** * The variant to use. * @default 'filled' diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index a7f3e915fb9541..b41f131096680f 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -346,6 +346,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { onKeyUp, size = 'medium', variant = 'filled', + tabIndex, ...other } = props; @@ -460,7 +461,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} - tabIndex={disabled ? ownerState.tabIndex || -1 : ownerState.tabIndex} + tabIndex={disabled ? tabIndex ?? -1 : tabIndex} ownerState={ownerState} {...moreProps} {...other} @@ -570,6 +571,10 @@ Chip.propTypes /* remove-proptypes */ = { PropTypes.func, PropTypes.object, ]), + /** + * tab order of an element + */ + tabIndex: PropTypes.number, /** * The variant to use. * @default 'filled' diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index fd91a642811959..eafe568d82f3de 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -183,7 +183,7 @@ describe('', () => { expect(chip).to.have.class(classes.root); expect(chip).to.have.property('tabIndex', -1); - expect(chip).to.not.have.class(classes.focusVisible); + expect(chip).not.to.have.class(classes.focusVisible); }); it('should render with the root and filled clickable secondary class', () => { From 938ffc090cd0052a3a56eb8c8c88eeb1a12ba171 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 9 Nov 2022 20:57:41 +0530 Subject: [PATCH 04/12] [Chip] updated docs for chips --- docs/pages/material-ui/api/chip.json | 1 + docs/translations/api-docs/chip/chip.json | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/pages/material-ui/api/chip.json b/docs/pages/material-ui/api/chip.json index c0aa34f7971e66..863e18466df66d 100644 --- a/docs/pages/material-ui/api/chip.json +++ b/docs/pages/material-ui/api/chip.json @@ -30,6 +30,7 @@ "description": "Array<func
| object
| bool>
| func
| object" } }, + "tabIndex": { "type": { "name": "number" } }, "variant": { "type": { "name": "union", diff --git a/docs/translations/api-docs/chip/chip.json b/docs/translations/api-docs/chip/chip.json index 5012cbf664f9d0..e69e189d53994d 100644 --- a/docs/translations/api-docs/chip/chip.json +++ b/docs/translations/api-docs/chip/chip.json @@ -14,6 +14,7 @@ "onDelete": "Callback fired when the delete icon is clicked. If set, the delete icon will be shown.", "size": "The size of the component.", "sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the `sx` page for more details.", + "tabIndex": "tab order of an element", "variant": "The variant to use." }, "classDescriptions": { From 8e8dc3598c45066928b0929e55047f125e17e670 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 9 Nov 2022 21:17:09 +0530 Subject: [PATCH 05/12] [Chip] updated tabIndex prop description --- docs/pages/material-ui/api/chip.json | 1 - docs/translations/api-docs/chip/chip.json | 1 - packages/mui-material/src/Chip/Chip.d.ts | 2 +- packages/mui-material/src/Chip/Chip.js | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/pages/material-ui/api/chip.json b/docs/pages/material-ui/api/chip.json index 863e18466df66d..c0aa34f7971e66 100644 --- a/docs/pages/material-ui/api/chip.json +++ b/docs/pages/material-ui/api/chip.json @@ -30,7 +30,6 @@ "description": "Array<func
| object
| bool>
| func
| object" } }, - "tabIndex": { "type": { "name": "number" } }, "variant": { "type": { "name": "union", diff --git a/docs/translations/api-docs/chip/chip.json b/docs/translations/api-docs/chip/chip.json index e69e189d53994d..5012cbf664f9d0 100644 --- a/docs/translations/api-docs/chip/chip.json +++ b/docs/translations/api-docs/chip/chip.json @@ -14,7 +14,6 @@ "onDelete": "Callback fired when the delete icon is clicked. If set, the delete icon will be shown.", "size": "The size of the component.", "sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the `sx` page for more details.", - "tabIndex": "tab order of an element", "variant": "The variant to use." }, "classDescriptions": { diff --git a/packages/mui-material/src/Chip/Chip.d.ts b/packages/mui-material/src/Chip/Chip.d.ts index 176869163728fe..e8739089c9f124 100644 --- a/packages/mui-material/src/Chip/Chip.d.ts +++ b/packages/mui-material/src/Chip/Chip.d.ts @@ -77,7 +77,7 @@ export interface ChipTypeMap

{ */ sx?: SxProps; /** - * tab order of an element + * @ignore */ tabIndex?: number; /** diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index b41f131096680f..e069fb2a9ce7de 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -572,7 +572,7 @@ Chip.propTypes /* remove-proptypes */ = { PropTypes.object, ]), /** - * tab order of an element + * @ignore */ tabIndex: PropTypes.number, /** From 000a8a5ea90cd16d9921b9ab0ac44ef2e30172ce Mon Sep 17 00:00:00 2001 From: ZeeshanTamboli Date: Sat, 12 Nov 2022 13:12:54 +0530 Subject: [PATCH 06/12] improve test --- packages/mui-material/src/Chip/Chip.test.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index eafe568d82f3de..99971ab92170a6 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -174,12 +174,17 @@ describe('', () => { expect(chip).to.have.class(classes.filledPrimary); }); - it('should not be focused when chip is disabled', () => { - const { getByTestId } = render(); + it('should not be focused when a deletable chip is disabled', () => { + const { getByTestId } = render( + {}} />, + ); const chip = getByTestId('chip'); - fireEvent.keyDown(document.body, { key: 'Tab' }); + simulatePointerDevice(); + act(() => { + fireEvent.keyDown(document.body, { key: 'Tab' }); + }); expect(chip).to.have.class(classes.root); expect(chip).to.have.property('tabIndex', -1); From 4353049bddc05be29fe3c551cacd792fd851a781 Mon Sep 17 00:00:00 2001 From: Zeeshan Tamboli Date: Sat, 12 Nov 2022 13:13:23 +0530 Subject: [PATCH 07/12] Update packages/mui-material/src/Chip/Chip.js Signed-off-by: Zeeshan Tamboli --- packages/mui-material/src/Chip/Chip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index e069fb2a9ce7de..2baad5586017dd 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -461,7 +461,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} - tabIndex={disabled ? tabIndex ?? -1 : tabIndex} + tabIndex={disabled ? -1 : tabIndex} ownerState={ownerState} {...moreProps} {...other} From 0e1998e11dee19beea9ff4e76e1489001e1e1ab3 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:05:16 +0530 Subject: [PATCH 08/12] added skipFocusWhenDisabled prop --- packages/mui-material/src/Chip/Chip.d.ts | 6 ++++++ packages/mui-material/src/Chip/Chip.js | 9 ++++++++- packages/mui-material/src/Chip/Chip.test.js | 9 ++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/mui-material/src/Chip/Chip.d.ts b/packages/mui-material/src/Chip/Chip.d.ts index e8739089c9f124..9e8c0768feebb8 100644 --- a/packages/mui-material/src/Chip/Chip.d.ts +++ b/packages/mui-material/src/Chip/Chip.d.ts @@ -72,6 +72,12 @@ export interface ChipTypeMap

{ * @default 'medium' */ size?: OverridableStringUnion<'small' | 'medium', ChipPropsSizeOverrides>; + /** + * If `true`, allows the disabled chip to escape focus. + * If `false`, allows the disabled chip to receive focus. + * @default false + */ + skipFocusWhenDisabled?: boolean; /** * The system prop that allows defining system overrides as well as additional CSS styles. */ diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index 2baad5586017dd..6774de28d4dabe 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -347,6 +347,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { size = 'medium', variant = 'filled', tabIndex, + skipFocusWhenDisabled, ...other } = props; @@ -461,7 +462,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} - tabIndex={disabled ? -1 : tabIndex} + tabIndex={skipFocusWhenDisabled && disabled ? -1 : tabIndex} ownerState={ownerState} {...moreProps} {...other} @@ -563,6 +564,12 @@ Chip.propTypes /* remove-proptypes */ = { PropTypes.oneOf(['medium', 'small']), PropTypes.string, ]), + /** + * If `true`, allows the disabled chip to escape focus. + * If `false`, allows the disabled chip to receive focus + * @default false + */ + skipFocusWhenDisabled: PropTypes.bool, /** * The system prop that allows defining system overrides as well as additional CSS styles. */ diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index 99971ab92170a6..ee20d7fc50429f 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -174,9 +174,16 @@ describe('', () => { expect(chip).to.have.class(classes.filledPrimary); }); + // https://github.com/mui/material-ui/issues/35038 it('should not be focused when a deletable chip is disabled', () => { const { getByTestId } = render( - {}} />, + {}} + />, ); const chip = getByTestId('chip'); From e5226b2accaecb40dc2fb119c24104ca3d33b3e9 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:14:16 +0530 Subject: [PATCH 09/12] patch docs --- docs/pages/material-ui/api/chip.json | 1 + docs/translations/api-docs/chip/chip.json | 1 + packages/mui-material/src/Chip/Chip.js | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/pages/material-ui/api/chip.json b/docs/pages/material-ui/api/chip.json index c0aa34f7971e66..2d8251bf56d0f3 100644 --- a/docs/pages/material-ui/api/chip.json +++ b/docs/pages/material-ui/api/chip.json @@ -24,6 +24,7 @@ }, "default": "'medium'" }, + "skipFocusWhenDisabled": { "type": { "name": "bool" } }, "sx": { "type": { "name": "union", diff --git a/docs/translations/api-docs/chip/chip.json b/docs/translations/api-docs/chip/chip.json index 5012cbf664f9d0..2a39e6b8f90a2e 100644 --- a/docs/translations/api-docs/chip/chip.json +++ b/docs/translations/api-docs/chip/chip.json @@ -13,6 +13,7 @@ "label": "The content of the component.", "onDelete": "Callback fired when the delete icon is clicked. If set, the delete icon will be shown.", "size": "The size of the component.", + "skipFocusWhenDisabled": "If true, allows the disabled chip to escape focus. If false, allows the disabled chip to receive focus.", "sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the `sx` page for more details.", "variant": "The variant to use." }, diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index 6774de28d4dabe..78966cc3782130 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -566,7 +566,7 @@ Chip.propTypes /* remove-proptypes */ = { ]), /** * If `true`, allows the disabled chip to escape focus. - * If `false`, allows the disabled chip to receive focus + * If `false`, allows the disabled chip to receive focus. * @default false */ skipFocusWhenDisabled: PropTypes.bool, From dc6868469ba41f7129b2ff95a2489fbde8da60c8 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:19:56 +0530 Subject: [PATCH 10/12] prettier changes --- packages/mui-material/src/Chip/Chip.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/Chip/Chip.d.ts b/packages/mui-material/src/Chip/Chip.d.ts index 9e8c0768feebb8..4e8644064f1a2c 100644 --- a/packages/mui-material/src/Chip/Chip.d.ts +++ b/packages/mui-material/src/Chip/Chip.d.ts @@ -76,7 +76,7 @@ export interface ChipTypeMap

{ * If `true`, allows the disabled chip to escape focus. * If `false`, allows the disabled chip to receive focus. * @default false - */ + */ skipFocusWhenDisabled?: boolean; /** * The system prop that allows defining system overrides as well as additional CSS styles. From cbaaed6b8fb59b1c1610df302d7d82afb1aa8c8c Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:36:13 +0530 Subject: [PATCH 11/12] made default value false --- packages/mui-material/src/Chip/Chip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index 78966cc3782130..1d0075201ef4e2 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -347,7 +347,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { size = 'medium', variant = 'filled', tabIndex, - skipFocusWhenDisabled, + skipFocusWhenDisabled = false, ...other } = props; From bface646c050d70140f771ef98630f5372450d84 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Thu, 24 Nov 2022 19:37:56 +0530 Subject: [PATCH 12/12] updated test name and todo comment --- packages/mui-material/src/Chip/Chip.js | 2 +- packages/mui-material/src/Chip/Chip.test.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/mui-material/src/Chip/Chip.js b/packages/mui-material/src/Chip/Chip.js index 1d0075201ef4e2..87925389a8fc51 100644 --- a/packages/mui-material/src/Chip/Chip.js +++ b/packages/mui-material/src/Chip/Chip.js @@ -347,7 +347,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) { size = 'medium', variant = 'filled', tabIndex, - skipFocusWhenDisabled = false, + skipFocusWhenDisabled = false, // TODO v6: Rename to `focusableWhenDisabled`. ...other } = props; diff --git a/packages/mui-material/src/Chip/Chip.test.js b/packages/mui-material/src/Chip/Chip.test.js index ee20d7fc50429f..1cf81c596aa535 100644 --- a/packages/mui-material/src/Chip/Chip.test.js +++ b/packages/mui-material/src/Chip/Chip.test.js @@ -174,8 +174,7 @@ describe('', () => { expect(chip).to.have.class(classes.filledPrimary); }); - // https://github.com/mui/material-ui/issues/35038 - it('should not be focused when a deletable chip is disabled', () => { + it('should not be focused when a deletable chip is disabled and skipFocusWhenDisabled is true', () => { const { getByTestId } = render(