Skip to content

Conversation

@guidezpl
Copy link
Member

@guidezpl guidezpl commented Nov 1, 2021

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 f: material design flutter/packages/flutter/material repository. 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
Copy link
Contributor

@darrenaustin darrenaustin left a comment

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

Piinks commented Nov 17, 2021

(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
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
Copy link
Contributor

Piinks commented Nov 17, 2021

Thanks @darrenaustin! :)

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

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants