-
Notifications
You must be signed in to change notification settings - Fork 3.4k
MaterialState migration guide #10194
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
MaterialState migration guide #10194
Conversation
|
Visit the preview URL for this PR (updated for commit 8f9b821): https://flutter-docs-prod--pr10194-material-state-migration-0x5o39j4.web.app |
sfshaza2
left a comment
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.
A few minor edits. This feature sounds great. Thanks, @MitchellGoodwin!
|
|
||
| ## Summary | ||
|
|
||
| `MaterialState`, and its related APIs have been moved out |
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.
| `MaterialState`, and its related APIs have been moved out | |
| `MaterialState`, and its related APIs, have been moved out |
|
|
||
| ## Background | ||
|
|
||
| `MaterialState` provided logic for handeling multiple different |
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.
| `MaterialState` provided logic for handeling multiple different | |
| Previously, `MaterialState` provided logic for handling multiple different |
| `MaterialState` provided logic for handeling multiple different | ||
| states a widget could have, like "hovered", "focused", and | ||
| "disabled". Because this functionality could be useful outside | ||
| of the Material library, namely the base Widgets layer and |
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.
| of the Material library, namely the base Widgets layer and | |
| of the Material library, namely for the base Widgets layer and |
| of the Material library, namely the base Widgets layer and | ||
| Cupertino, it was decided to move it outside of Material. As | ||
| part of the move, and to avoid future confusion, the different | ||
| MaterialState classes have been renamed to `WidgetState`. The |
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.
| MaterialState classes have been renamed to `WidgetState`. The | |
| `MaterialState` classes have been renamed to `WidgetState`. The |
| | MaterialState | WidgetState | | ||
| | MaterialStatePropertyResolver | WidgetStatePropertyResolver | | ||
| | MaterialStateColor | WidgetStateColor | | ||
| | MaterialStateMouseCursor | WidgetStateColorMouseCursor | | ||
| | MaterialStateBorderSide | WidgetStateBorderSide | | ||
| | MaterialStateOutlinedBorder | WidgetStateOutlinedBorder | | ||
| | MaterialStateTextStyle | WidgetStateTextStyle | | ||
| | MaterialStateProperty | WidgetStateProperty | | ||
| | MaterialStatePropertyAll | WidgetStatePropertyAll | | ||
| | MaterialStatesController | WidgetStatesController | | ||
| | MaterialStateMixin | WidgetStateMixin | |
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.
| | MaterialState | WidgetState | | |
| | MaterialStatePropertyResolver | WidgetStatePropertyResolver | | |
| | MaterialStateColor | WidgetStateColor | | |
| | MaterialStateMouseCursor | WidgetStateColorMouseCursor | | |
| | MaterialStateBorderSide | WidgetStateBorderSide | | |
| | MaterialStateOutlinedBorder | WidgetStateOutlinedBorder | | |
| | MaterialStateTextStyle | WidgetStateTextStyle | | |
| | MaterialStateProperty | WidgetStateProperty | | |
| | MaterialStatePropertyAll | WidgetStatePropertyAll | | |
| | MaterialStatesController | WidgetStatesController | | |
| | MaterialStateMixin | WidgetStateMixin | | |
| | `MaterialState` | `WidgetState` | | |
| | `MaterialStatePropertyResolver` | `WidgetStatePropertyResolver` | | |
| | `MaterialStateColor` | `WidgetStateColor` | | |
| | `MaterialStateMouseCursor` | `WidgetStateColorMouseCursor` | | |
| | `MaterialStateBorderSide` | `WidgetStateBorderSide` | | |
| | `MaterialStateOutlinedBorder` | `WidgetStateOutlinedBorder` | | |
| | `MaterialStateTextStyle` | `WidgetStateTextStyle` | | |
| | `MaterialStateProperty` | `WidgetStateProperty` | | |
| | `MaterialStatePropertyAll` | `WidgetStatePropertyAll` | | |
| | `MaterialStatesController` | `WidgetStatesController` | | |
| | `MaterialStateMixin` | `WidgetStateMixin` | |
| Material library with no `WidgetState` equivilant, as | ||
| they are Material design specific. |
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.
| Material library with no `WidgetState` equivilant, as | |
| they are Material design specific. | |
| Material library with no `WidgetState` equivalent, as | |
| they are specific to Material design. |
|
|
||
| ## Timeline | ||
|
|
||
| Landed in version: xxx<br> |
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.
@MitchellGoodwin, do you have the info on which main or beta release has this change? Put that here.
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.
It's not landed yet, we are trying to make sure our communication is ready to go before landing it. Should I put what release it's likely to be in for now? Or leave it as a placeholder.
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.
Leave it for now, but once you know, we should update the page. thx!
goderbauer
left a comment
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
|
|
||
| Previously, `MaterialState` provided logic for handling multiple different | ||
| states a widget could have, like "hovered", "focused", and | ||
| "disabled". Because this functionality could be useful outside |
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.
could be useful -> is useful?
| final MaterialPropertyResolver<MouseCursor?> resolveCallback; | ||
|
|
||
| @override | ||
| MouseCursor resolve(Set<MaterialState> states) => resolveCallback(states) ?? MouseCursor.uncontrolled; |
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.
nit here and below: I think usually we indent by only two spaces.
| MouseCursor resolve(Set<MaterialState> states) => resolveCallback(states) ?? MouseCursor.uncontrolled; | ||
| } | ||
|
|
||
| MaterialStateBorderSide? get side; |
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 top-level getter without a body looks a little strange. Is that even legal in dart? Maybe just have it return null?
|
|
||
| Code after migration: | ||
|
|
||
| ```dart |
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.
Same comments about this sample
|
Have we lost you, @MitchellGoodwin? |
|
Apologies, this change ended up causing a lot of discussion. I'll make updates to this. |
f316062 to
d11cfc0
Compare
df33ecb to
98b0757
Compare
|
Ready for review |
goderbauer
left a comment
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
| --- | ||
| title: Rename MaterialState to WidgetState | ||
| description: >- | ||
| MaterialState and it's related APIs have been moved |
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.
| MaterialState and it's related APIs have been moved | |
| MaterialState and its related APIs have been moved |
| states a widget could have, like "hovered", "focused", and | ||
| "disabled". Because this functionality is useful outside of the | ||
| Material library, namely for the base Widgets layer and Cupertino, | ||
| it was decided to move it outside of Material. Aspart of the move, and to |
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.
| it was decided to move it outside of Material. Aspart of the move, and to | |
| it was decided to move it outside of Material. As part of the move, and to |
sfshaza2
left a comment
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. Thx, @MitchellGoodwin!
Adds a migration guide guide for MaterialState logic being marked deprecated by [flutter/pull/142151](flutter/flutter#142151) ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.
Adds a migration guide guide for MaterialState logic being marked deprecated by flutter/pull/142151
Presubmit checklist