-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Add adaptive Checkbox and CheckboxListTile #123132
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
Add adaptive Checkbox and CheckboxListTile #123132
Conversation
QuncCccccc
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.
This looks good to me! Just left two comments about the enum type checking:)
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.
Can we use switch for this check? Just think it would be more consistent with other widgetX.adaptive().
Such as:
switch (_checkboxType) {
case _CheckboxType.material:
break;
case _CheckboxType.adaptive: {
final ThemeData theme = Theme.of(context);
switch (theme.platform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.windows:
break;
case TargetPlatform.iOS:
case TargetPlatform.macOS:
return CupertinoCheckbox(...);
}
}
}
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.
Same here. I think we can do a similar change like the SwitchListTile(code link) so that we don't have to create two objects ahead of time:)
QuncCccccc
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!
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.
nit: probably don't need this empty line:)
8dc6d8d to
1686989
Compare
6586b7e to
785da77
Compare
Addresses #102810. Adds an adaptive constructor for the Checkbox and CheckboxListTile widgets that displays the checkboxes as either a Material or Cupertino checkbox depending on platform.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.