Add IconThemeData adjustment to ListTile trailing element #11858
Conversation
…istent with leading element)
|
This will need a test. |
| alignment: FractionalOffset.centerRight, | ||
| child: trailing, | ||
| children.add(IconTheme.merge( | ||
| data: new IconThemeData(color: _iconColor(theme, tileTheme)), |
There was a problem hiding this comment.
Might be worth factoring out new IconThemeData(color: _iconColor(theme, tileTheme)), into a value that you use for leading and trailing (which would only be initialized if either is non-null, obviously).
|
This seems pretty good, thanks for the contribution! Just needs a test to make sure we don't accidentally revert the behaviour at some point. |
| IconThemeData iconThemeData; | ||
| if (leading != null || trailing != null) { | ||
| iconThemeData = new IconThemeData(color: _iconColor(theme, tileTheme)); | ||
| } |
There was a problem hiding this comment.
nit: our style guide says to avoid the braces for one-line if statements.
There was a problem hiding this comment.
looks like you missed this one
|
The test looks great, if you could rebase (looks like there's a conflict) and fix the trivial style nit, I can land it for you! Thanks again! |
|
I think I've taken care of the mentioned issues, let me know if there's anything additional that needs to be done here. |
|
|
|
Thanks! |
|
This caused a regression, see #12221. |
It was incorrectly added in #11858
3 participants
This makes the leading and trailing Widgets behave the same way. What I changed in this PR is closely based on the existing code for the leading element.
Currently, they render differently, as you can see in these two screenshots:
