From ef6c93479260f25ca479dde14a01df7f5d969cce Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 20 Aug 2023 19:30:51 +0530 Subject: [PATCH 1/8] fix --- .../mui-material/src/Select/SelectInput.js | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index ba51fc1925035a..8c6275b1e386dd 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -258,6 +258,21 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { } }; + const clonedOnChange = (event, newValue, child) => { + // Redefine target to allow name and value to be read. + // This allows seamless integration with the most popular form libraries. + // https://github.com/mui/material-ui/issues/13485#issuecomment-676048492 + // Clone the event to not override `target` of the original event. + const nativeEvent = event.nativeEvent || event; + const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent); + + Object.defineProperty(clonedEvent, 'target', { + writable: true, + value: { value: newValue, name }, + }); + onChange(clonedEvent, child); + }; + const handleItemClick = (child) => (event) => { let newValue; @@ -286,18 +301,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { setValueState(newValue); if (onChange) { - // Redefine target to allow name and value to be read. - // This allows seamless integration with the most popular form libraries. - // https://github.com/mui/material-ui/issues/13485#issuecomment-676048492 - // Clone the event to not override `target` of the original event. - const nativeEvent = event.nativeEvent || event; - const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent); - - Object.defineProperty(clonedEvent, 'target', { - writable: true, - value: { value: newValue, name }, - }); - onChange(clonedEvent, child); + clonedOnChange(event, newValue, child); } } @@ -324,6 +328,28 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { } }; + const handleItemKeyDown = (event) => { + if (event.key === ' ') { + const child = childrenArray.find( + (childItem) => childItem.props.value === event.target.dataset.value, + ); + + if (child === undefined) { + return; + } + + const newValue = child.props.value; + if (newValue !== value) { + setValueState(child.props.value); + + if (onChange) { + clonedOnChange(event, newValue, child); + } + } + update(false, event); + } + }; + const open = displayNode !== null && openState; const handleBlur = (event) => { @@ -408,6 +434,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { child.props.onKeyUp(event); } }, + onKeyDown: handleItemKeyDown, role: 'option', selected, value: undefined, // The value is most likely not a valid HTML attribute. From 89ef4ce7df6c581923c81f186aa1871cbf55c7e1 Mon Sep 17 00:00:00 2001 From: sai6855 <60743144+sai6855@users.noreply.github.com> Date: Sun, 20 Aug 2023 21:35:01 +0530 Subject: [PATCH 2/8] fix --- packages/mui-material/src/Select/SelectInput.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 8c6275b1e386dd..1db8a65e7767aa 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -338,9 +338,13 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { return; } + if (child.props.onKeyDown) { + child.props.onKeyDown(event); + } + const newValue = child.props.value; if (newValue !== value) { - setValueState(child.props.value); + setValueState(newValue); if (onChange) { clonedOnChange(event, newValue, child); From 9e01896f595f6b8ea76d8b0bda54c17e806a9749 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Thu, 3 Oct 2024 12:06:03 +0530 Subject: [PATCH 3/8] fix logic --- packages/mui-material/src/Select/SelectInput.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index dec13b68ead402..546a9ad0d28599 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -328,16 +328,8 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { } }; - const handleItemKeyDown = (event) => { + const handleItemKeyDown = (child) => (event) => { if (event.key === ' ') { - const child = childrenArray.find( - (childItem) => childItem.props.value === event.target.dataset.value, - ); - - if (child === undefined) { - return; - } - if (child.props.onKeyDown) { child.props.onKeyDown(event); } @@ -438,7 +430,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { child.props.onKeyUp(event); } }, - onKeyDown: handleItemKeyDown, + onKeyDown: handleItemKeyDown(child), role: 'option', selected, value: undefined, // The value is most likely not a valid HTML attribute. From 7da690bcb8eaceb95073020372d504ff9808e7f1 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Thu, 3 Oct 2024 12:37:53 +0530 Subject: [PATCH 4/8] test --- .../mui-material/src/Select/Select.test.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/mui-material/src/Select/Select.test.js b/packages/mui-material/src/Select/Select.test.js index 4d6817f9ad0c13..0af30f316ed6da 100644 --- a/packages/mui-material/src/Select/Select.test.js +++ b/packages/mui-material/src/Select/Select.test.js @@ -152,6 +152,26 @@ describe(' + + + + , + ); + fireEvent.mouseDown(getByRole('combobox')); + const options = getAllByRole('option'); + await act(async () => { + fireEvent.keyDown(options[0], { key: 'ArrowDown' }); + fireEvent.keyDown(options[1], { key: 'ArrowDown' }); + fireEvent.keyDown(options[2], { key: ' ' }); + }); + + expect(onChangeHandler.args[0][0].target.value).to.equal('2'); + }); + [' ', 'ArrowUp', 'ArrowDown', 'Enter'].forEach((key) => { it(`should open menu when pressed ${key} key on select`, async () => { render( From c20fd25cfe15b05e3f014531db052f13cbed5a1f Mon Sep 17 00:00:00 2001 From: sai6855 Date: Thu, 3 Oct 2024 12:41:17 +0530 Subject: [PATCH 5/8] modify test --- .../mui-material/src/Select/Select.test.js | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/mui-material/src/Select/Select.test.js b/packages/mui-material/src/Select/Select.test.js index 0af30f316ed6da..a88d5993bd72d9 100644 --- a/packages/mui-material/src/Select/Select.test.js +++ b/packages/mui-material/src/Select/Select.test.js @@ -152,24 +152,26 @@ describe(' - - - + , ); - fireEvent.mouseDown(getByRole('combobox')); + + const trigger = getByRole('combobox'); + fireEvent.mouseDown(trigger); + const options = getAllByRole('option'); - await act(async () => { - fireEvent.keyDown(options[0], { key: 'ArrowDown' }); - fireEvent.keyDown(options[1], { key: 'ArrowDown' }); - fireEvent.keyDown(options[2], { key: ' ' }); - }); + fireEvent.keyDown(options[0], { key: 'ArrowDown' }); + fireEvent.keyDown(options[1], { key: 'ArrowDown' }); + fireEvent.keyDown(options[2], { key: ' ' }); - expect(onChangeHandler.args[0][0].target.value).to.equal('2'); + expect(handleChange.callCount).to.equal(1); + expect(handleChange.firstCall.args[0].target.value).to.equal('2'); }); [' ', 'ArrowUp', 'ArrowDown', 'Enter'].forEach((key) => { From 469f350908ae8bd22a8de07ad1ce28116f641dd4 Mon Sep 17 00:00:00 2001 From: sai6855 Date: Thu, 3 Oct 2024 12:43:40 +0530 Subject: [PATCH 6/8] modify test --- packages/mui-material/src/Select/Select.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/mui-material/src/Select/Select.test.js b/packages/mui-material/src/Select/Select.test.js index a88d5993bd72d9..95b778fce1cefe 100644 --- a/packages/mui-material/src/Select/Select.test.js +++ b/packages/mui-material/src/Select/Select.test.js @@ -154,11 +154,14 @@ describe(' Zero One - Two + + Two + , ); @@ -171,6 +174,7 @@ describe('', () => { const handleKeyDown = spy(); const { getAllByRole, getByRole } = render( ', () => { fireEvent.keyDown(options[2], { key: ' ' }); expect(handleChange.callCount).to.equal(1); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(3); expect(handleChange.firstCall.args[0].target.value).to.equal('2'); }); diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 546a9ad0d28599..588622e2a60e7c 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -330,10 +330,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { const handleItemKeyDown = (child) => (event) => { if (event.key === ' ') { - if (child.props.onKeyDown) { - child.props.onKeyDown(event); - } - const newValue = child.props.value; if (newValue !== value) { setValueState(newValue); @@ -344,6 +340,10 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { } update(false, event); } + + if (child.props.onKeyDown) { + child.props.onKeyDown(event); + } }; const open = displayNode !== null && openState; From 002a319b90279c5647a446ae2df194227f7bfef0 Mon Sep 17 00:00:00 2001 From: sai chand <60743144+sai6855@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:53:10 +0530 Subject: [PATCH 8/8] Update packages/mui-material/src/Select/SelectInput.js Co-authored-by: Marija Najdova Signed-off-by: sai chand <60743144+sai6855@users.noreply.github.com> --- packages/mui-material/src/Select/SelectInput.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 588622e2a60e7c..fd9f28f69b51cd 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -341,9 +341,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { update(false, event); } - if (child.props.onKeyDown) { - child.props.onKeyDown(event); - } + child?.props?.onKeyDown?.(event); }; const open = displayNode !== null && openState;