Skip to content

Reland "Refactor ThemeData (#91497)" (part 1) - #92822

Merged
guidezpl merged 1 commit into
flutter:masterfrom
guidezpl:reland-themedata
Nov 1, 2021
Merged

guidezpl merged 1 commit into
flutter:masterfrom
guidezpl:reland-themedata

Conversation

@guidezpl

@guidezpl guidezpl commented Nov 1, 2021 •

Copy link
Copy Markdown
Member

Reland most of #91497, which was reverted in #92820.

The re-ordering of business logic in the factory constructor seems to have caused a change in some buttons' background color. This PR includes all of #91497, minus the re-ordering of that logic.

@guidezpl
guidezpl requested a review from darrenaustin November 1, 2021 10:50
@flutter-dashboard flutter-dashboard Bot added p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Nov 1, 2021
@google-cla google-cla Bot added the cla: yes label Nov 1, 2021
@guidezpl
guidezpl requested a review from HansMuller November 1, 2021 10:50

@darrenaustin darrenaustin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, the ThemeData initialization code is a bit brittle and subtle. Easy to break.

LGTM.

@guidezpl
guidezpl merged commit 3ccf730 into flutter:master Nov 1, 2021
@guidezpl
guidezpl deleted the reland-themedata branch November 1, 2021 20:09
@Piinks

Piinks commented Nov 17, 2021

Copy link
Copy Markdown
Contributor

(Breaking change audit) Hey @guidezpl I noticed this was reverted for breaking internal tests, did these updates (part 1 & 2) make it non breaking? Or was the internal code migrated? If the later, this should have a migration guide under breaking changes on the website.

@darrenaustin

Copy link
Copy Markdown
Contributor

I am pretty sure this was an ordering issue with some initialization code. The fix was reordering it in such a way that it maintained the previous behaviour. This shouldn't be a breaking change.

@Piinks

Piinks commented Nov 17, 2021

Copy link
Copy Markdown
Contributor

Thanks @darrenaustin! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants