Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reland Refactored ListTileTheme: ListTileThemeData, ThemeData.listThemeData #91840

Merged
merged 2 commits into from Oct 14, 2021
Merged

Reland Refactored ListTileTheme: ListTileThemeData, ThemeData.listThemeData #91840

merged 2 commits into from Oct 14, 2021

Conversation

HansMuller
Copy link
Contributor

@HansMuller HansMuller commented Oct 14, 2021 •
edited

Reland #91449

Fixed a typo in the ListTileTheme.data getter that caused an internal test to fail. Added a regression test.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Oct 14, 2021
@google-cla google-cla bot added the cla: yes label Oct 14, 2021
darrenaustin
darrenaustin approved these changes Oct 14, 2021
View reviewed changes
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.

LGTM

@HansMuller HansMuller merged commit acab6dc into flutter:master Oct 14, 2021
@HansMuller HansMuller deleted the reland_list_tile_theme_data branch October 14, 2021 22:13
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021
@HansMuller @clocksmith
Reland Refactored ListTileTheme: ListTileThemeData, ThemeData.listThe…
e934dd5 
…meData (flutter#91840)
@HansMuller HansMuller mentioned this pull request Nov 15, 2021
Open
40 tasks
@guidezpl
Copy link
Member

@HansMuller @Piinks I'm curious if Dart fix supports this kind of change?

@HansMuller
Copy link
Contributor Author

@guidezpl - I'm assuming that class renames are supported, so in this case ListTileTheme => ListTileThemeData.

@guidezpl
Copy link
Member

It's a bit more than that, right, with adding a constructor
Screen Shot 2021-11-17 at 17 40 46

@HansMuller
Copy link
Contributor Author

Yes, that's right. The old constructor still works however a complete upgrade would require the transformation you've shown. I don't know if Dart Fix can do that.

@bwilkerson - is this possible?

Foo(a: b, c: d) => Bar(data: BarData(a: b, c: d))

@Piinks
Copy link
Contributor

Piinks commented Nov 17, 2021

Hey all, thanks for thinking of this! I do not believe this is supported right now. Dart fixes work within a class right now, not across them. There is a feature request for this though in dart-lang/sdk#45393

The TextSelectionThemeData ones seem similar, they were added in #78897, so it may be worth double checking.

Overall, this does seem like something that really should have a migration guide, are you going to make one @HansMuller?

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

@darrenaustin darrenaustin darrenaustin approved these changes

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@HansMuller @guidezpl @Piinks @darrenaustin