-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Migration guide for https://github.com/flutter/flutter/pull/47177 #3430
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
Conversation
|
CI appears to be failing on unrelated files... |
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.
@dnfield, I'm a little surprised to see this PR against the flutter/flutter repo, as we are homing the breaking changes on flutter/website. (Under src/docs/release/breaking-changes)
| ## Description of change | ||
|
|
||
| The Flutter framework tracks when the editing state of an `EditableText` widget | ||
| changes. It keeps synchronizes this value to the engine, but only if the value |
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.
Delete "keeps"
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.
Done
| The Flutter framework tracks when the editing state of an `EditableText` widget | ||
| changes. It keeps synchronizes this value to the engine, but only if the value | ||
| has changed. The state of this value is tracked by the `TextInputClient`, and | ||
| logic exists to prevent sending duplicative values to the engine 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.
duplicative => duplicate
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.
Done
|
@sfshaza2 - this is against the website repo :) I've linked to the related flutter/flutter PR here. |
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.
That was fast, @dnfield!! LGTM. Waiting to merge for eng review (unless you tell me otherwise).
|
Thanks - let's give Hixie and goderbauer a chance to see it :) |
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
|
|
||
| ## Migration guide | ||
|
|
||
| Tests that relied on the dirty state will have to be updated. For example, this |
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 call out ".. dirty state from a previously run test..."?
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.
Done
flutter/flutter#47177 is a breaking change no matter how I try to slice it - even if I pull out the test related changes. Provides a guide for migrating tests that this breaks.