Skip to content

Conversation

@MitchellGoodwin
Copy link
Contributor

Adds a migration guide guide for MaterialState logic being marked deprecated by flutter/pull/142151

Presubmit checklist

@flutter-website-bot
Copy link
Collaborator

flutter-website-bot commented Feb 22, 2024

Visit the preview URL for this PR (updated for commit 8f9b821):

https://flutter-docs-prod--pr10194-material-state-migration-0x5o39j4.web.app

Copy link
Contributor

@sfshaza2 sfshaza2 left a 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MaterialState classes have been renamed to `WidgetState`. The
`MaterialState` classes have been renamed to `WidgetState`. The

Comment on lines 27 to 37
| MaterialState | WidgetState |
| MaterialStatePropertyResolver | WidgetStatePropertyResolver |
| MaterialStateColor | WidgetStateColor |
| MaterialStateMouseCursor | WidgetStateColorMouseCursor |
| MaterialStateBorderSide | WidgetStateBorderSide |
| MaterialStateOutlinedBorder | WidgetStateOutlinedBorder |
| MaterialStateTextStyle | WidgetStateTextStyle |
| MaterialStateProperty | WidgetStateProperty |
| MaterialStatePropertyAll | WidgetStatePropertyAll |
| MaterialStatesController | WidgetStatesController |
| MaterialStateMixin | WidgetStateMixin |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| 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` |

Comment on lines 41 to 42
Material library with no `WidgetState` equivilant, as
they are Material design specific.
Copy link
Contributor

@sfshaza2 sfshaza2 Feb 22, 2024

Choose a reason for hiding this comment

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

Suggested change
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>
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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!

Copy link
Member

@goderbauer goderbauer left a 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
Copy link
Member

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;
Copy link
Member

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;
Copy link
Member

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
Copy link
Member

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

@atsansone atsansone added the review.await-update Awaiting Updates after Edits label Feb 22, 2024
@sfshaza2
Copy link
Contributor

Have we lost you, @MitchellGoodwin?

@MitchellGoodwin
Copy link
Contributor Author

Apologies, this change ended up causing a lot of discussion. I'll make updates to this.

@MitchellGoodwin MitchellGoodwin force-pushed the material-state-migration branch from f316062 to d11cfc0 Compare March 19, 2024 21:08
@MitchellGoodwin MitchellGoodwin marked this pull request as ready for review March 22, 2024 20:25
@MitchellGoodwin MitchellGoodwin force-pushed the material-state-migration branch from df33ecb to 98b0757 Compare April 3, 2024 20:28
@MitchellGoodwin
Copy link
Contributor Author

Ready for review

Copy link
Member

@goderbauer goderbauer left a 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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

@parlough parlough added review.copy Awaiting Copy Review and removed review.await-update Awaiting Updates after Edits labels Apr 4, 2024
@parlough parlough assigned sfshaza2 and unassigned MitchellGoodwin Apr 4, 2024
Copy link
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

lgtm. Thx, @MitchellGoodwin!

@sfshaza2 sfshaza2 removed the review.copy Awaiting Copy Review label Apr 4, 2024
@sfshaza2 sfshaza2 merged commit 558e194 into flutter:main Apr 4, 2024
atsansone pushed a commit to atsansone/website that referenced this pull request Apr 5, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants