Repository navigation
ListTileTheme should be a part of ThemeData #31247
Copy link
Copy link
Closed
Labels
c: new featureNothing broken; request for a new capabilityNothing broken; request for a new capabilityframeworkflutter/packages/flutter repository. See also f: labels.flutter/packages/flutter repository. See also f: labels.p: material_uimaterial_ui package in flutter/packagesmaterial_ui package in flutter/packages
Description
Steps to Reproduce
- Create a new flutter app
- Add a list tile widget with a secondary icon
- Add all icon themes to the ThemeData, Notice secondary icon attributes do not change
- Wrap Widget with ListTileTheme and add icon attributes, Notice secondary icon attributes changed.
https://gist.github.com/ibrahim-mubarak/f6f777396372ac400e0a7f129e5b1186
Logs
Output of flutter run --verbose
https://gist.github.com/ibrahim-mubarak/573222b2988f0149df74bd9a8bca0bd5
Output of flutter analyze
Analyzing list_tile_theme...
No issues found! (ran in 1.4s)
Output of flutter doctor -v
[✓] Flutter (Channel stable, v1.2.1, on Mac OS X 10.14.4 18E226, locale en-IN)
• Flutter version 1.2.1 at /Users/ibrahim/development/flutter
• Framework revision 8661d8aecd (9 weeks ago), 2019-02-14 19:19:53 -0800
• Engine revision 3757390fa4
• Dart version 2.1.2 (build 2.1.2-dev.0.0 0a7dcf17eb)
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
• Android SDK at /Users/ibrahim/Library/Android/sdk
• Android NDK location not configured (optional; useful for native profiling support)
• Platform android-28, build-tools 28.0.3
• ANDROID_HOME = /Users/ibrahim/Library/Android/sdk
• Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1248-b01)
• All Android licenses accepted.
[✓] iOS toolchain - develop for iOS devices (Xcode 10.2)
• Xcode at /Applications/Xcode.app/Contents/Developer
• Xcode 10.2, Build version 10E125
• ios-deploy 1.9.4
• CocoaPods version 1.6.1
[✓] Android Studio (version 3.3)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin version 34.0.1
• Dart plugin version 182.5215
• Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1248-b01)
[✓] Android Studio
• Android Studio at /Applications/Android Studio 3.5 Preview.app/Contents
• Flutter plugin version 34.0.4
• Dart plugin version 191.6183.87
• Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1343-b01)
[✓] VS Code (version 1.33.1)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 2.25.1
[✓] Connected device (1 available)
• iPhone Xʀ • CB9F9632-F724-40C0-8C12-8FB04A58DD26 • ios • com.apple.CoreSimulator.SimRuntime.iOS-12-2 (simulator)
• No issues found!
If we are using The Navigators onGenerateRoute Function we have to wrap all our page widgets with ListTileTheme or the MaterialApp itself with ListTileTheme, both in my opinion, feels hacky
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
c: new featureNothing broken; request for a new capabilityNothing broken; request for a new capabilityframeworkflutter/packages/flutter repository. See also f: labels.flutter/packages/flutter repository. See also f: labels.p: material_uimaterial_ui package in flutter/packagesmaterial_ui package in flutter/packages
Activity
HansMuller commentedon Apr 18, 2019
CC @johnsonmh
smark91 commentedon Oct 21, 2019
Any news on this?
vasilich6107 commentedon Nov 22, 2019
Hi 👋!
Any updates?
HansMuller commentedon Nov 22, 2019
I agree that a ListTileTheme entry belongs in ThemeData. We're going to try and make some substantial revisions to ThemeData et al. starting early next year. We'll post more details in a GitHub issue like #43035, once the holidays have passed by.
CC @darrenaustin
GroovinChip commentedon Apr 12, 2020
Any progress on this @HansMuller?
Levi-Lesches commentedon Apr 17, 2020
The new proposal he was talking about has been published -- check out #54776 and subscribe to that thread. It solves a number of different issues across the board all at once.
mistercx commentedon May 17, 2020
I wait too this fix. Because I have a paradox: if I load my dark theme - my icon in ListTile change color very nice, but if I load my light theme - icon in ListTile become gray (default). I hope, description ListTileTheme inside ThemeData - it'll be very-very right!
mistercx commentedon May 17, 2020
How can I subscribe? Sorry, but I'm beginner...
vasilich6107 commentedon May 17, 2020
@mistercx hit this button https://tppr.me/IT6Bj
AyushBherwani1998 commentedon Jul 18, 2020
@shihaohong I would like to work on this issue.
shihaohong commentedon Jul 22, 2020
@AyushBherwani1998 Sounds good! I'll assign the issue to you.
Edit:
Note that. Here's an example of what newer themes should look like: https://gist.github.com/HansMuller/9d9917bcd9f43327395a6b4800d8c557. The issue with ListTileTheme is that it does not have aListTileThemewill not conform to the new norms for theming, but it should be okay to expose ListTileTheme in ThemeDataListTileThemeData, and instead contains its properties as part of the inherited widget itself. There's a mention about it at flutter.dev/go/material-theme-system-updates under Lists.Edit2: Actually, I take that back. It might be better to hold off on this and instead make the breaking change towards using a
ListTileThemeDataobject that houses all the properties forListTilejust like in https://gist.github.com/HansMuller/9d9917bcd9f43327395a6b4800d8c557. Otherwise, we'd be making a breaking change to ThemeData as well.3 remaining items
AyushBherwani1998 commentedon Jul 24, 2020
That's great, should I start with a design doc?
shihaohong commentedon Jul 27, 2020
That sounds like a good idea. Let me know if you run into any issues or need any help with preparing one
AyushBherwani1998 commentedon Aug 5, 2020
Hey @darrenaustin, I had conversation with Shi Hao offline and he told me that you have some suggestions for migration.
darrenaustin commentedon Aug 12, 2020
Hi @AyushBherwani1998, sorry for the delay in my response. Thanks for the interest in contributing! For some background if you haven't already seen it, Hans put together an overall design for how we want to update our theme system.
As has been pointed out in other comments the core design issue is the need to pull out the theme properties into a
ListTileThemeDataclass and just have theListTileThemebe the inherited widget wrapper for this data. This would be pretty straightforward if we were implementing this fresh today.However, the existing
ListTileThemeclass is already in use by multiple widgets in the Material Library as well as a bunch of Google internal clients that use it. For these kinds of breaking changes, we have been striving to have a "soft" break were we stage the changes such that they don't break for any given build along the way (see our breaking change guide for more info). Usually this involves some kind of "opt-in" feature where we implement the new interface with it turned off by default, but with an opt-in flag that allows us to land the feature without breaking anything. Then we go through and update all the clients to opt-in, then we can remove the old interface and the opt-in flag.In this case (and a couple of others like
DialogTheme) it is a lot harder, because we are not switching to a new API that can coexist along side the existing one. Rather, we need to change an existing API in place.The core interface changes to
ListTileThemewould involve:ListTileThemeData datafield.ListTileTheme.ofto be aListTileThemeData.ListTileTheme.mergefunction as this is usually done withListTileThemeData.copyWithThe biggest sticking point is the
offunction return type. There is a fair amount of clients that use this function and they expect aListTileTheme.One possibility that I have been exploring is to see if we might be able to stage this by having a temporary mixin called
ListTileThemeDatathat has the data interface, but no real implementation.ListTileThemecould implement this mixin so that it is also considered aListTileThemeData. Then we could slowly migrate the existing clients to use theListTileThemeDataclass instead ofListTileTheme. Once everything is only referencingListTileThemeDatathen we could swap out the mixin for a real data class, and change the return type ofListTileTheme.ofwithout breaking anything. Kind of convoluted, but I haven't been able to come up with a better way to soft land this kind of change.Anyway, those are the issues as I see them. Just putting together a design doc for what we want the final API to look like would be a good first step. The hard part will be coming up with a migration plan to softly land these changes. I am more than happy to help out with this if you are still interesting in working on it.
AyushBherwani1998 commentedon Aug 13, 2020
Hey @darrenaustin, thank you for the response. You are right, Shi Hao and I had a similar discussion about the migration and
ofmethod being a roadblock. The approach to use a mixin being a complex migration process still looks promising which can be figured out as we move forward.I'm still interested to work on this and learn a few things along the way. As you said, I'll start working on the design doc, and reach out for the help if I'm stuck. Would you mind if I reach out to you on discord in case if I'm stuck?
JonHacon commentedon Nov 8, 2020
did this go anywhere? what is the suggested approach at the moment?
e200 commentedon Feb 22, 2021
Any news? This really should be added as part of ThemeData.
tirth-stasis commentedon Jul 1, 2021
Any updates on this? It's 2021!
fernando-s97 commentedon Sep 11, 2021
So... How's that?
TiffApps commentedon Sep 25, 2021
It feels weird to (still) not have this as a part of ThemeData, could anyone take care of this issue please?
HansMuller commentedon Oct 8, 2021
Working on this now: #31247
github-actions commentedon Oct 26, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.