-
-
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
v8.0.0-alpha.0 #15319
base: master
Are you sure you want to change the base?
v8.0.0-alpha.0 #15319
Conversation
Deploy preview: https://deploy-preview-15319--material-ui-x.netlify.app/ |
CHANGELOG.md
Outdated
```diff | ||
import * as React from 'react'; | ||
import { | ||
- TreeItem2, | ||
+ TreeItem, | ||
- TreeItem2Root, |
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.
@flaviendelangle is all this information necessary? It feels like we are not going straight to the point and that these should be kept in the PR or migration guide.
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.
So far AFAIK we always added the exact same content on the migration guide and on the changelog/release note.
If we want to stop doing it and instead add a summary + a link to the migration guide any time it's a longer section, I'm fine doing it.
WDYT @cherniavskii @alexfauquette @LukasTy @joserodolfofreitas?
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.
I have added just a summary on my BC PR assuming that it was the case, just saw you updated it @flaviendelangle . I think it would be more readable if we add a link to the migration guide, since it's the same content.
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.
I guess we could keep the "introduction text" as is and replace all the examples with a link to the migration guide.
I think it's important that the changelog is enough for people to know if the BC impact them or not (or at least if there is a high chance it does).
So I would keep a fairly detailed description and only remove the examples.
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.
I guess it's only for the alpha.0
that the BCs take a bit more than expected space, from next versions, since there'll be a release every week, it shouldn't be as long.
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.
Not that long but it can be a few hundreds lines 😆
But yeah alpha.0 is probably the worse.
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.
i've removed the examples, let me know if the wording is wrong or where we should keep the examples.
I tried to proofread where I removed them, but you know better than I what was necessary or not
Co-authored-by: Flavien DELANGLE <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>
Co-authored-by: Flavien DELANGLE <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>
Co-authored-by: Flavien DELANGLE <[email protected]> Co-authored-by: Bilal Shafi <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>
Signed-off-by: Jose C Quintas Jr <[email protected]>
Co-authored-by: Alexandre Fauquette <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>
CHANGELOG.md
Outdated
- The default DOM structure of the field has changed. | ||
Before version `v8.x`, the fields' DOM structure consisted of an `<input />`, which held the whole value for the component, but unfortunately presents a few limitations in terms of accessibility when managing multiple section values. | ||
Starting with version `v8.x`, all the field and picker components come with a new DOM structure that allows the field component to set aria attributes on individual sections, providing a far better experience with screen readers. | ||
**Fallback to the non-accessible DOM structure:** |
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.
This section is empty without the code snippet 😆
For the new DOM structure you can just remove everything under **Fallback to the non-accessible DOM structure:**
.
And I would add a link to the right section of the migration guide for each breaking change that was shortened.
Something like:
See Migration guide—New DOM structure for the field for more details.
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.
Added the migration guide to the changes: https://github.com/mui/mui-x/blob/186313072fd5714ccbadb95042f6a8ed7d5871a0/CHANGELOG.md
Had to manually add them though, and slightly reworked the text in some cases.
This is not the latest version from master, I'll do another pass when we have a set release date
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.
Had to manually add them though, and slightly reworked the text in some cases.
I'll add them on by PR descriptions for the next BCs 👍
- [pickers] Remove `TDate` generics in favor of `PickerValidDate` direct usage (#15001) @flaviendelangle | ||
- [pickers] Remove `utils` and `value` params from translations (#14986) @arthurbalduini | ||
- [pickers] Remove plural in "Pickers" on recently introduced APIs (#15297) @flaviendelangle | ||
- [pickers] Remove the re-export from `@mui/x-license` (#14487) @k-rajat19 |
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.
- [pickers] Remove the re-export from `@mui/x-license` (#14487) @k-rajat19 | |
- [pickers] Remove the re-export from `@mui/x-license` (#14487) @k-rajat19 | |
- [pickers] Unify JSDoc for all the disabled and readOnly props (#15304) @flaviendelangle |
|
||
#### Breaking changes | ||
|
||
- The default DOM structure of the field has changed. [Migration Guide - New DOM structure for the field](https://github.com/mui/mui-x/blob/master/docs/data/migration/migration-pickers-v7/migration-pickers-v7.md#new-dom-structure-for-the-field) |
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.
I think @samuelsycamore tried to unified the phrasing when we link to another doc section for more info:
- The default DOM structure of the field has changed. [Migration Guide - New DOM structure for the field](https://github.com/mui/mui-x/blob/master/docs/data/migration/migration-pickers-v7/migration-pickers-v7.md#new-dom-structure-for-the-field) | |
- The default DOM structure of the field has changed. | |
See the [Migration Guide — New DOM structure for the field](https://next.mui.com/x/migration/migration-pickers-v7/#new-dom-structure-for-the-field) for more details. |
And I replaced the link with the one of the deployed doc, not the Github file.
X others
We can get Sam's opinion here I think
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.
My issue with that is that it gets too big for no added value. Add it to each of the migration url and you have a lot of useless info.
It would be fine in the docs or in a blog post though
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.
I used the gh file because it is more static, the next.mui.com
will make this url useless very soon
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.
I used the gh file because it is more static, the next.mui.com will make this url useless very soon
Not sure to follow you here
For the wording around the link, if we think the full sentence is too lengthy, then I would just add:
- The default DOM structure of the field has changed. [Migration Guide - New DOM structure for the field](https://github.com/mui/mui-x/blob/master/docs/data/migration/migration-pickers-v7/migration-pickers-v7.md#new-dom-structure-for-the-field) | |
- The default DOM structure of the field has changed — [Learn more(https://next.mui.com/x/migration/migration-pickers-v7/#new-dom-structure-for-the-field). |
The current middleground feels weird to me.
By the way, we use a long dash "—" not "-" to separate the page title and the section title.
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.
I used the gh file because it is more static, the next.mui.com will make this url useless very soon
Not sure to follow you here
Wouldn't next.mui.com
be "stuck" after we release v8 until we release v9 next year? Or is next
always up-to-date? 🤔
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.
Wouldn't next.mui.com be "stuck" after we release v8 until we release v9 next year? Or is next always up-to-date? 🤔
AFAIK, we will replace all the next.mui
links with mui.com
links when releasing major.
No description provided.