-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] Enable the new field DOM structure by default #14651
[fields] Enable the new field DOM structure by default #14651
Conversation
Deploy preview: https://deploy-preview-14651--material-ui-x.netlify.app/ Updated pages: |
bce7e95
to
931a1a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we already want to remove the "v6" x "v7" terminology for good ?
It happens on:
-
DateField/tests/editing.DateField.test
- onChangeV7 (multiple declarations)
- onChangeV6 (multiple declarations)
-
TimeField/tests/editing.TimeField.test.tsx
- onChangeV7 (multiple declarations)
- onChangeV6 (multiple declarations)
Although I think it can make sense, I also see it as questionable since adding new suffixes could make it more verbose and hardly to read, plus we are already used to the v6 x v7 nomenclature. I wouldn't mind leaving the renaming for v9 once we remove the non-accessible-v6 input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👏
Nice to see so many removed lines, specially in our tests ! 🎉
I'm leaving the approve, I believe the v7xv6 nomenclature doesn't block this PR 🙂
Thanks for the deep dive on the v6/v7 nomenclature.
I would differenciate public usage of the nomenclature and internal one. All the rest is purely internal, I can rename it for consistency, but as you said everything will go away in v9 so it may not be worth the effort. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Closes #14477
If possible, this PR should be merged as soon as the
next
branch is prepared so that I can open the PR to #14796 which also impacts most of the interfaces in the package.Work
enableAccessibleFieldDOMStructure
when the prop is not provided and render the right text fieldTEnableAccessibleFieldDOMStructure
generic in every component and interface using itenableAccessibleFieldDOMStructure={true}
in doc and tests