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

[fields] Fix field editing after closing the picker #12675

Merged
merged 3 commits into from Apr 11, 2024

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Apr 4, 2024

Fixes #12652.

Clicking the open button causes the following event to be triggered and if default is not prevented, it executes the input DOM selection logic.

const handleInputClick = useEventCallback((event: React.MouseEvent, ...args) => {
// The click event on the clear button would propagate to the input, trigger this handler and result in a wrong section selection.
// We avoid this by checking if the call of `handleInputClick` is actually intended, or a side effect.
if (event.isDefaultPrevented()) {
return;
}
onClick?.(event, ...(args as []));
syncSelectionFromDOM();
});

Changed the onOpen and onClose actions to call event.preventDefault() to avoid such problems.

@LukasTy LukasTy added regression A bug, but worse component: pickers This is the name of the generic UI component, not the React module! feature: Keyboard editing Related to the pickers keyboard edition labels Apr 4, 2024
@LukasTy LukasTy self-assigned this Apr 4, 2024
@mui-bot
Copy link

mui-bot commented Apr 4, 2024

Deploy preview: https://deploy-preview-12675--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against bf0d93d

@flaviendelangle
Copy link
Member

@LukasTy the e2e tests are broken 👍

@LukasTy
Copy link
Member Author

LukasTy commented Apr 10, 2024

the e2e tests are broken 👍

@flaviendelangle This is a totally random problem that made no sense to me... 🙈 🤷
Simply renaming the test file helped to avoid this problem... 😆
This just makes my argument for going with the native playwright test writing style stronger. 👍

@flaviendelangle
Copy link
Member

Simply renaming the test file helped to avoid this problem... 😆

😭

onOpen: () => void;
onClose: () => void;
onOpen: (event: React.UIEvent) => void;
onClose: (event?: React.UIEvent) => void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have places where we pass the event to onClose?
This PR only passes them to onOpen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly nope, but implicitly it is called by clicking on the open picker button and this is basically the reason why our selection indexes get screwed up... 🙈
Without this, the fix doesn't work as the input selection logic gets called.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK make sense 👍

@LukasTy LukasTy merged commit aed8aeb into mui:master Apr 11, 2024
17 checks passed
@LukasTy LukasTy deleted the field-editing-after-picker-closing branch April 11, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! feature: Keyboard editing Related to the pickers keyboard edition regression A bug, but worse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fields] Can not type into date picker after dismissing calendar picker
3 participants