[material_ui] Add Checkbox.markInsets - #12690
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces 'markInsets' to 'Checkbox' and 'CheckboxThemeData' to allow insetting the check mark and indeterminate dash within the checkbox, shrinking them proportionally without altering the overall box size. The feedback suggests adding assertions in the 'Checkbox' constructors and the '_CheckboxPainter' setter to enforce that 'markInsets' is non-negative, as specified in the documentation.
| }) : _checkboxType = _CheckboxType.material, | ||
| assert(tristate || value != null); |
There was a problem hiding this comment.
Since the documentation for markInsets states that it "Must be non-negative", we should enforce this constraint with an assertion in the constructor to catch invalid values early in development.
}) : _checkboxType = _CheckboxType.material,
assert(tristate || value != null),
assert(markInsets == null || (markInsets.left >= 0.0 && markInsets.top >= 0.0 && markInsets.right >= 0.0 && markInsets.bottom >= 0.0), 'markInsets must be non-negative.');| }) : _checkboxType = _CheckboxType.adaptive, | ||
| assert(tristate || value != null); |
There was a problem hiding this comment.
Similarly, we should add the non-negative assertion for markInsets in the Checkbox.adaptive constructor.
}) : _checkboxType = _CheckboxType.adaptive,
assert(tristate || value != null),
assert(markInsets == null || (markInsets.left >= 0.0 && markInsets.top >= 0.0 && markInsets.right >= 0.0 && markInsets.bottom >= 0.0), 'markInsets must be non-negative.');| set markInsets(EdgeInsets value) { | ||
| if (_markInsets == value) { | ||
| return; | ||
| } | ||
| _markInsets = value; | ||
| notifyListeners(); // triggers repaint when it changes | ||
| } |
There was a problem hiding this comment.
To ensure that invalid negative insets provided via CheckboxThemeData (which doesn't have constructor assertions) are also caught before they cause rendering anomalies, we should add an assertion in the _CheckboxPainter.markInsets setter.
set markInsets(EdgeInsets value) {
if (_markInsets == value) {
return;
}
assert(value.left >= 0.0 && value.top >= 0.0 && value.right >= 0.0 && value.bottom >= 0.0, 'markInsets must be non-negative.');
_markInsets = value;
notifyListeners(); // triggers repaint when it changes
}|
Thank you for your contribution! Because of the volume of PRs we receive, we require that new contributors use our checklist to guide them through critical steps in creating a Flutter PR. This PR's description is missing that checklist, so it is being marked as a Draft. Please edit the PR description to add the checklist, then ensure that you have completed all of the steps. Once you've done that, please mark the PR as ready for review. If you need help, consider asking for advice on the #hackers-new channel on Discord. |
This PR ports my changes from flutter/flutter#188258 to
flutter/packages,
as part of flutter/flutter#188444.