-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix InputDecorator suffix and prefix IconButtons ignore IconButtonTheme
#145473
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.
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
Fix InputDecorator suffix and prefix IconButtons ignore IconButtonTheme
#145473
Conversation
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks Taha glad you're back!
06cae8e to
6d2e9bb
Compare
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some customers that are broken by this. It looks like they'll need to update their code before we can land this.
Unless, could it be overriding a color value unexpectedly? Would an empty (null valued) theme here override the widget value?
Here is an example of one of the failures, the left is expected, the right is the result with this change:
|
Parent If this is a disruptive change, i could instead introduce flutter/packages/flutter/lib/src/material/dropdown_menu_theme.dart Lines 31 to 35 in 859eb2e
|
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The affected customers may just be expecting the incorrect behavior, if so it should be relatively easy for them to be updated. We'll need to take a look at their code first. I think it make sense to inherit the theme in this way, rather than creating a special scenario for just DropdownMenu, but I'll defer to @justinmc. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late response here!
I think maybe there is actually a problem that is being exposed by the Google test failures. The MaterialStateProperty colors in the iconButtonTheme here are not having the materialState of the InputDecorator applied to them. See the example below.
I would expect the icon to be purple in this example, but it's lime
import 'package:flutter/material.dart';
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({Key? key}) : super(key: key);
@override
Widget build(BuildContext context) {
return MaterialApp(
title: 'Flutter Demo',
theme: ThemeData(
primarySwatch: Colors.blue,
iconButtonTheme: IconButtonThemeData(
style: ButtonStyle(
foregroundColor: WidgetStateProperty.resolveWith((Set<WidgetState> states) {
if (states.contains(WidgetState.error)) {
return Colors.deepPurple;
}
return Colors.limeAccent;
}),
),
),
),
home: const MyHomePage(title: 'Flutter Demo Home Page'),
);
}
}
class MyHomePage extends StatelessWidget {
const MyHomePage({Key? key, required this.title}) : super(key: key);
final String title;
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: Text(title),
),
body: Center(
child: Column(
mainAxisAlignment: MainAxisAlignment.center,
children: <Widget>[
TextField(
decoration: InputDecoration(
errorText: 'error state',
suffixIcon: IconButton(
onPressed: () {
},
icon: const Icon(Icons.phonelink_erase),
),
),
),
],
),
),
);
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the details!
It turns out the inherited IconButtonTheme.style.foreground couldn't be overridden by _getPrefixIconColor & _getPrefixIconColor and resolve input decoration states.
I've updated both _getPrefixIconColor & _getSuffixIconColor to inherit the IconButtonTheme.style.foreground and resolve the state while placing in the order it can be superseded by inputDecorationTheme and decoration.suffixIconColor.
I think maybe there is actually a problem that is being exposed by the Google test failures
Add two new tests to verify _getPrefixIconColor & _getSuffixIconColor respects input decoration states
for IconButtonTheme.style.foreground color in the input_decorator_test.dart.
6d2e9bb to
b10fad4
Compare
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit 👍
Thanks for fixing that! That fixes my use case, and it looks like it fixed the Google tests too. Thanks for testing those cases as well.
IconButton widget ignore IconButtonThemeIconButtonTheme
Roll Flutter from ac2ca93 to 477ebd8 (32 revisions) flutter/flutter@ac2ca93...477ebd8 2024-04-05 [email protected] Roll Flutter Engine from 95ffe73ac2a8 to 6974dbac35a1 (2 revisions) (flutter/flutter#146350) 2024-04-05 [email protected] Roll pub packages (flutter/flutter#146348) 2024-04-05 [email protected] Roll Packages from dce6f0c to 764d997 (4 revisions) (flutter/flutter#146344) 2024-04-05 [email protected] Roll Flutter Engine from 292c48b9af5e to 95ffe73ac2a8 (1 revision) (flutter/flutter#146343) 2024-04-05 [email protected] Roll pub packages (flutter/flutter#146331) 2024-04-05 [email protected] Roll Flutter Engine from 23a1d0d2fce6 to 292c48b9af5e (1 revision) (flutter/flutter#146330) 2024-04-05 [email protected] Roll Flutter Engine from 597d33f57183 to 23a1d0d2fce6 (1 revision) (flutter/flutter#146327) 2024-04-05 [email protected] Roll Flutter Engine from 1fa0623bb07f to 597d33f57183 (1 revision) (flutter/flutter#146325) 2024-04-05 [email protected] Roll Flutter Engine from f17d586de6f1 to 1fa0623bb07f (1 revision) (flutter/flutter#146324) 2024-04-05 [email protected] Roll Flutter Engine from d44462a42d73 to f17d586de6f1 (4 revisions) (flutter/flutter#146322) 2024-04-05 [email protected] Fix cursor is not centered when cursorHeight is set (non-Apple platforms). (flutter/flutter#145829) 2024-04-05 [email protected] refactor: Perform plugin resolution per platform (flutter/flutter#144506) 2024-04-04 [email protected] `ExpansionTile` Unable to remove right padding from title (flutter/flutter#145271) 2024-04-04 [email protected] Roll Flutter Engine from dcc99d652154 to d44462a42d73 (1 revision) (flutter/flutter#146311) 2024-04-04 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.1 to 4.2.0 (flutter/flutter#146310) 2024-04-04 [email protected] Roll Flutter Engine from e3104af94328 to dcc99d652154 (4 revisions) (flutter/flutter#146308) 2024-04-04 [email protected] Roll Flutter Engine from fa4d97e363ff to e3104af94328 (2 revisions) (flutter/flutter#146300) 2024-04-04 [email protected] Remove dead `compareIosVersions` function (flutter/flutter#146298) 2024-04-04 [email protected] Adds semanticsLabel to MenuItemButton (flutter/flutter#145846) 2024-04-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (#146181)" (flutter/flutter#146305) 2024-04-04 [email protected] Roll Flutter Engine from 918c72b803cc to fa4d97e363ff (2 revisions) (flutter/flutter#146299) 2024-04-04 [email protected] Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (flutter/flutter#146181) 2024-04-04 [email protected] Roll Flutter Engine from 03cd54d4285a to 918c72b803cc (2 revisions) (flutter/flutter#146296) 2024-04-04 [email protected] Fix InputDecorator suffix and prefix IconButtons ignore `IconButtonTheme` (flutter/flutter#145473) 2024-04-04 [email protected] Give `generate_gradle_lockfiles.dart` functionality to exclude certain subdirectories (flutter/flutter#146228) 2024-04-04 [email protected] Update documentation to discourage using the TextEditingController.text setter (flutter/flutter#146151) 2024-04-04 [email protected] Roll Flutter Engine from b2ceabf7acce to 03cd54d4285a (1 revision) (flutter/flutter#146292) 2024-04-04 [email protected] Roll Flutter Engine from b435a8a87df0 to b2ceabf7acce (1 revision) (flutter/flutter#146283) 2024-04-04 [email protected] Roll Flutter Engine from bd51ee80d74b to b435a8a87df0 (1 revision) (flutter/flutter#146279) 2024-04-04 [email protected] Roll Packages from 0e848fa to dce6f0c (4 revisions) (flutter/flutter#146276) 2024-04-04 [email protected] Flutter templates example app Gradle memory settings (flutter/flutter#146275) 2024-04-04 [email protected] Roll Flutter Engine from 4303dc0d7a73 to bd51ee80d74b (2 revisions) (flutter/flutter#146273) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...
|
@TahaTesser Thank you for taking on the issue I created! However, I believe my complaint wasn't entirely fixed. Is this just not intended for the Material DropdownMenu or am I using the properties wrongly? |
|
@felixwoestmann ).merge(iconButtonTheme.style),I'll address this soon and add test that checks all |
…eme` (flutter#145473) fixes [DropdownMenu TrailingIcon can't be styled through providing an IconButtonTheme](flutter#145081) ### Code sample <details> <summary>expand to view the code sample</summary> ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @OverRide Widget build(BuildContext context) { return MaterialApp( debugShowCheckedModeBanner: false, home: Scaffold( body: Center( child: IconButtonTheme( data: IconButtonThemeData( style: IconButton.styleFrom( foregroundColor: const Color(0xffff0000), shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(10.0), ), ), ), child: SizedBox( width: 300, child: Column( mainAxisSize: MainAxisSize.min, children: [ const Text('IconButton'), IconButton(onPressed: () {}, icon: const Icon(Icons.search)), const Text('TextField'), TextField( decoration: InputDecoration( prefixIcon: IconButton( onPressed: () {}, icon: const Icon(Icons.search), ), suffixIcon: IconButton( onPressed: () {}, icon: const Icon(Icons.search), ), ), ), ], ), ), ), ), ), ); } } ``` </details> | Before | After | | --------------- | --------------- | | <img src="https://github.com/flutter/flutter/assets/48603081/69b5966b-c95d-4934-b867-3262d1377f70" /> | <img src="https://github.com/flutter/flutter/assets/48603081/0064db2b-0379-4424-a5bf-39bdc5441fe8" /> |
Roll Flutter from ac2ca93 to 477ebd8 (32 revisions) flutter/flutter@ac2ca93...477ebd8 2024-04-05 [email protected] Roll Flutter Engine from 95ffe73ac2a8 to 6974dbac35a1 (2 revisions) (flutter/flutter#146350) 2024-04-05 [email protected] Roll pub packages (flutter/flutter#146348) 2024-04-05 [email protected] Roll Packages from dce6f0c to 764d997 (4 revisions) (flutter/flutter#146344) 2024-04-05 [email protected] Roll Flutter Engine from 292c48b9af5e to 95ffe73ac2a8 (1 revision) (flutter/flutter#146343) 2024-04-05 [email protected] Roll pub packages (flutter/flutter#146331) 2024-04-05 [email protected] Roll Flutter Engine from 23a1d0d2fce6 to 292c48b9af5e (1 revision) (flutter/flutter#146330) 2024-04-05 [email protected] Roll Flutter Engine from 597d33f57183 to 23a1d0d2fce6 (1 revision) (flutter/flutter#146327) 2024-04-05 [email protected] Roll Flutter Engine from 1fa0623bb07f to 597d33f57183 (1 revision) (flutter/flutter#146325) 2024-04-05 [email protected] Roll Flutter Engine from f17d586de6f1 to 1fa0623bb07f (1 revision) (flutter/flutter#146324) 2024-04-05 [email protected] Roll Flutter Engine from d44462a42d73 to f17d586de6f1 (4 revisions) (flutter/flutter#146322) 2024-04-05 [email protected] Fix cursor is not centered when cursorHeight is set (non-Apple platforms). (flutter/flutter#145829) 2024-04-05 [email protected] refactor: Perform plugin resolution per platform (flutter/flutter#144506) 2024-04-04 [email protected] `ExpansionTile` Unable to remove right padding from title (flutter/flutter#145271) 2024-04-04 [email protected] Roll Flutter Engine from dcc99d652154 to d44462a42d73 (1 revision) (flutter/flutter#146311) 2024-04-04 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.1 to 4.2.0 (flutter/flutter#146310) 2024-04-04 [email protected] Roll Flutter Engine from e3104af94328 to dcc99d652154 (4 revisions) (flutter/flutter#146308) 2024-04-04 [email protected] Roll Flutter Engine from fa4d97e363ff to e3104af94328 (2 revisions) (flutter/flutter#146300) 2024-04-04 [email protected] Remove dead `compareIosVersions` function (flutter/flutter#146298) 2024-04-04 [email protected] Adds semanticsLabel to MenuItemButton (flutter/flutter#145846) 2024-04-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (#146181)" (flutter/flutter#146305) 2024-04-04 [email protected] Roll Flutter Engine from 918c72b803cc to fa4d97e363ff (2 revisions) (flutter/flutter#146299) 2024-04-04 [email protected] Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (flutter/flutter#146181) 2024-04-04 [email protected] Roll Flutter Engine from 03cd54d4285a to 918c72b803cc (2 revisions) (flutter/flutter#146296) 2024-04-04 [email protected] Fix InputDecorator suffix and prefix IconButtons ignore `IconButtonTheme` (flutter/flutter#145473) 2024-04-04 [email protected] Give `generate_gradle_lockfiles.dart` functionality to exclude certain subdirectories (flutter/flutter#146228) 2024-04-04 [email protected] Update documentation to discourage using the TextEditingController.text setter (flutter/flutter#146151) 2024-04-04 [email protected] Roll Flutter Engine from b2ceabf7acce to 03cd54d4285a (1 revision) (flutter/flutter#146292) 2024-04-04 [email protected] Roll Flutter Engine from b435a8a87df0 to b2ceabf7acce (1 revision) (flutter/flutter#146283) 2024-04-04 [email protected] Roll Flutter Engine from bd51ee80d74b to b435a8a87df0 (1 revision) (flutter/flutter#146279) 2024-04-04 [email protected] Roll Packages from 0e848fa to dce6f0c (4 revisions) (flutter/flutter#146276) 2024-04-04 [email protected] Flutter templates example app Gradle memory settings (flutter/flutter#146275) 2024-04-04 [email protected] Roll Flutter Engine from 4303dc0d7a73 to bd51ee80d74b (2 revisions) (flutter/flutter#146273) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...

fixes DropdownMenu TrailingIcon can't be styled through providing an IconButtonTheme
Code sample
expand to view the code sample
Before
After
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.