Skip to content

ListTileTheme should be a part of ThemeData #31247

Closed

Description

@ibrahim-mubarak

Steps to Reproduce

  1. Create a new flutter app
  2. Add a list tile widget with a secondary icon
  3. Add all icon themes to the ThemeData, Notice secondary icon attributes do not change
  4. 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

Activity

  1. added
    p: material_uimaterial_ui package in flutter/packages
    frameworkflutter/packages/flutter repository. See also f: labels.
    c: new featureNothing broken; request for a new capability
    on Apr 18, 2019
  2. HansMuller commented on Apr 18, 2019

    @HansMuller
    Contributor
  3. smark91 commented on Oct 21, 2019

    @smark91

    Any news on this?

  4. vasilich6107 commented on Nov 22, 2019

    @vasilich6107
    Contributor

    Hi 👋!
    Any updates?

  5. HansMuller commented on Nov 22, 2019

    @HansMuller
    Contributor

    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

  6. GroovinChip commented on Apr 12, 2020

    @GroovinChip
    Contributor

    Any progress on this @HansMuller?

  7. Levi-Lesches commented on Apr 17, 2020

    @Levi-Lesches
    Contributor

    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.

  8. mistercx commented on May 17, 2020

    @mistercx

    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!

  9. mistercx commented on May 17, 2020

    @mistercx

    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.

    How can I subscribe? Sorry, but I'm beginner...

  10. vasilich6107 commented on May 17, 2020

    @vasilich6107
    Contributor
  11. AyushBherwani1998 commented on Jul 18, 2020

    @AyushBherwani1998
    Member

    @shihaohong I would like to work on this issue.

  12. shihaohong commented on Jul 22, 2020

    @shihaohong
    Contributor

    @AyushBherwani1998 Sounds good! I'll assign the issue to you.

    Edit: Note that ListTileTheme will not conform to the new norms for theming, but it should be okay to expose ListTileTheme in ThemeData. 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 a ListTileThemeData, 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 ListTileThemeData object that houses all the properties for ListTile just like in https://gist.github.com/HansMuller/9d9917bcd9f43327395a6b4800d8c557. Otherwise, we'd be making a breaking change to ThemeData as well.

  13. 3 remaining items

  14. AyushBherwani1998 commented on Jul 24, 2020

    @AyushBherwani1998
    Member

    That's great, should I start with a design doc?

  15. shihaohong commented on Jul 27, 2020

    @shihaohong
    Contributor

    That sounds like a good idea. Let me know if you run into any issues or need any help with preparing one

  16. AyushBherwani1998 commented on Aug 5, 2020

    @AyushBherwani1998
    Member

    Hey @darrenaustin, I had conversation with Shi Hao offline and he told me that you have some suggestions for migration.

  17. darrenaustin commented on Aug 12, 2020

    @darrenaustin
    Contributor

    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 ListTileThemeData class and just have the ListTileTheme be the inherited widget wrapper for this data. This would be pretty straightforward if we were implementing this fresh today.

    However, the existing ListTileTheme class 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 ListTileTheme would involve:

    • Change the constructor to no longer take in properties themselves, but rather a ListTileThemeData data field.
    • Change the return type of ListTileTheme.of to be a ListTileThemeData.
    • Deprecate/remove the ListTileTheme.merge function as this is usually done with ListTileThemeData.copyWith

    The biggest sticking point is the of function return type. There is a fair amount of clients that use this function and they expect a ListTileTheme.

    One possibility that I have been exploring is to see if we might be able to stage this by having a temporary mixin called ListTileThemeData that has the data interface, but no real implementation. ListTileTheme could implement this mixin so that it is also considered a ListTileThemeData. Then we could slowly migrate the existing clients to use the ListTileThemeData class instead of ListTileTheme. Once everything is only referencing ListTileThemeData then we could swap out the mixin for a real data class, and change the return type of ListTileTheme.of without 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.

  18. AyushBherwani1998 commented on Aug 13, 2020

    @AyushBherwani1998
    Member

    Hey @darrenaustin, thank you for the response. You are right, Shi Hao and I had a similar discussion about the migration and of method 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?

  19. JonHacon commented on Nov 8, 2020

    @JonHacon

    did this go anywhere? what is the suggested approach at the moment?

  20. e200 commented on Feb 22, 2021

    @e200

    Any news? This really should be added as part of ThemeData.

  21. tirth-stasis commented on Jul 1, 2021

    @tirth-stasis

    Any updates on this? It's 2021!

  22. fernando-s97 commented on Sep 11, 2021

    @fernando-s97

    So... How's that?

  23. TiffApps commented on Sep 25, 2021

    @TiffApps

    It feels weird to (still) not have this as a part of ThemeData, could anyone take care of this issue please?

  24. HansMuller commented on Oct 8, 2021

    @HansMuller
    Contributor

    Working on this now: #31247

  25. github-actions commented on Oct 26, 2021

    @github-actions

    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 -v and a minimal reproduction of the issue.

  26. locked as resolved and limited conversation to collaborators on Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

c: new featureNothing broken; request for a new capabilityframeworkflutter/packages/flutter repository. See also f: labels.p: material_uimaterial_ui package in flutter/packages

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions