[Material] Add selection control themes#70311
[Material] Add selection control themes#70311fluttergithubbot merged 18 commits intoflutter:masterfrom
Conversation
flutter-dashboard
bot
added
f: material design
perclasson
force-pushed
the
add_themes_for_selection_controls
branch
from
9197b97 to
00ed0cb
Compare
guidezpl
left a comment
guidezpl
left a comment
There was a problem hiding this comment.
_ ___ _____ __ __
| | / __|_ _| \/ |
| |_| (_ | | | | |\/| |
|____\___| |_| |_| |_|
JoseAlba
left a comment
JoseAlba
left a comment
There was a problem hiding this comment.
██╗ ██████╗ ████████╗███╗ ███╗
██║ ██╔════╝ ╚══██╔══╝████╗ ████║
██║ ██║ ███╗ ██║ ██╔████╔██║
██║ ██║ ██║ ██║ ██║╚██╔╝██║
███████╗╚██████╔╝ ██║ ██║ ╚═╝ ██║
╚══════╝ ╚═════╝ ╚═╝ ╚═╝ ╚═╝
|
New themes should use |
|
https://api.flutter.dev/flutter/material/InkResponse/overlayColor.html defines the splash color as well as the focus and hover colors. Is there any reason we shouldn't follow that pattern in the new control themes? |
I forgot about |
| /// | ||
| /// Resolves in the following states: | ||
| /// * [MaterialState.hovered]. | ||
| /// * [MaterialState.focused]. |
There was a problem hiding this comment.
and [MaterialState.pressed] because splash
Here and elsewhere.
There was a problem hiding this comment.
It's not used in the pressed state. The selection controls are currently using the active color to determine the pressed overlay color. It will be a follow up to allow us to change that.
There was a problem hiding this comment.
Follow up issue for the pressed state: #70828
| /// | ||
| /// If null, then the value of [activeColor] is used in the selected | ||
| /// state. If that is also null, the value of [CheckboxThemeData.fillColor] | ||
| /// is used. |
There was a problem hiding this comment.
We're not explaining the component defaults here. It's a shame that the defaults aren't (yet) defined in terms of the theme's colorScheme however for the sake of consistency we should reveal what the defaults are.
There was a problem hiding this comment.
I did this now for Checkbox, Radio and Switch. For Switch it ended up being quite complicated to document the default values so I made a table for them. Hopefully that is easy to read.
There was a problem hiding this comment.
It's easy to read and I appreciate the trouble you've gone to to make it correct.
Generally speaking, we want the default values of component color properties to be based on the theme's colorScheme, obviously there's still much work to do. That will have to wait for another PR.
…or_selection_controls
6 participants
Description
Add selection control themes
CheckboxTheme,RadioTheme, andSwitchTheme.Related Issues
Relates to #64365
Tests
I added the following tests:
checkbox_theme_test.dartradio_theme_test.dartswitch_theme_test.dartChecklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.