Create cupertino checkbox #122244

MitchellGoodwin · 2023-03-08T23:08:17Z

Addresses: #61789.

Adds a macOS/iOS styled checkbox widget to Cupertino. Works with keyboard focus, and keyboard interactions. The goal is to make this widget adaptable with the Material Checkbox.

Notably SwiftUI does not provide checkboxes for iOS development, only macOS, so our comparison tool could not be used. So we have to compare with desktop checkboxes and the checkboxes in the Human Interface Guidelines.

Some screenshots of the checkbox in the macOS settings, and from the Human Interface Guidelines:

Here is the CupertinoCheckbox in action, both with mouse and keyboard interactions:

Screen.Recording.2023-03-08.at.3.06.53.PM.mov

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

MitchellGoodwin · 2023-03-08T23:10:02Z

packages/flutter/lib/src/cupertino/checkbox.dart

+/// Note: in the Apple ecosystem, checkboxes are encouraged to only be used in
+/// macOS, not iOS. If a multi-selection component is needed, iOS encourages the
+/// developer to use Switches, or [CupertinoSwitch] instead, or find a creative
+/// custom solution.


I'm not confident on the wording here. I would like to warn developers of Apple's guidance, without discouraging them from doing what they would like to do.

guidezpl · 2023-03-09T08:28:22Z

FYI for iOS it's in the HIG but only implicitly (e.g. in tables https://developer.apple.com/design/human-interface-guidelines/components/layout-and-organization/lists-and-tables/#platform-considerations)

bernaferrari · 2023-03-09T17:48:34Z

@GroovinChip might be the best person to give feedback on this, he made the macOS checkbox on Flutter.

gspencergoog

Just made an initial pass. Mostly it looks pretty good.

gspencergoog · 2023-03-10T00:25:10Z

packages/flutter/lib/src/cupertino/checkbox.dart

+/// Note: in the Apple ecosystem, checkboxes are encouraged to only be used in
+/// macOS, not iOS. If a multi-selection component is needed, iOS encourages the
+/// developer to use Switches, or [CupertinoSwitch] instead, or find a creative
+/// custom solution.


How about this?

Suggested change

/// Note: in the Apple ecosystem, checkboxes are encouraged to only be used in

/// macOS, not iOS. If a multi-selection component is needed, iOS encourages the

/// developer to use Switches, or [CupertinoSwitch] instead, or find a creative

/// custom solution.

/// In the Apple Human Interface Guidelines (HIG), checkboxes are encouraged for use on macOS, but not on iOS.

/// If a multi-selection component is needed on iOS, the HIG encourages the developer to use switches ([CupertinoSwitch] in Flutter)

/// instead, or to find a creative custom solution.

Does it actively discourage checkboxes on iOS? If not, then I'd just say that "but is silent about their use on iOS".

And probably link the appropriate HIG guidelines. Also note that (lol!) our style guide discourages empty prose like "Note" "Note that", etc.

gspencergoog · 2023-03-10T00:27:41Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  /// Whether this checkbox is checked.
+  ///
+  /// When [tristate] is true, a value of null corresponds to the mixed state.
+  /// When [tristate] is false, this value must not be null.


Does it assert? If so, you should mention that it asserts in debug mode.

gspencergoog · 2023-03-10T00:28:43Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  ///
+  /// When the checkbox is tapped, if [tristate] is false (the default) then
+  /// the [onChanged] callback will be applied to `!value`. If [tristate] is
+  /// true this callback cycle from false to true to null.


Suggested change

/// true this callback cycle from false to true to null.

/// true this callback cycle from false to true to null and back to false again.

gspencergoog · 2023-03-10T00:30:03Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  /// value is true, and to false if value is null (i.e. it cycles through false
+  /// => true => null => false when tapped).
+  ///
+  /// If tristate is false (the default), [value] must not be null.


Also mention if it asserts in debug mode.

gspencergoog · 2023-03-10T00:30:34Z

packages/flutter/lib/src/cupertino/checkbox.dart

+
+  /// The color and width of the checkbox's border.
+  ///
+  /// If this property is null, then the side will be width 1.


Suggested change

/// If this property is null, then the side will be width 1.

/// If this property is null, then the side defaults to a width of 1.

gspencergoog · 2023-03-10T00:39:10Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  }
+}
+
+const double _kEdgeSize = CupertinoCheckbox.width;


Why can't these use locations for this use CupertinoCheckbox.width directly? It is also a const, and would show where it comes from.

gspencergoog · 2023-03-10T00:41:46Z

packages/flutter/lib/src/cupertino/checkbox.dart

+
+  void _drawCheck(Canvas canvas, Offset origin, Paint paint) {
+    final Path path = Path();
+    const Offset start = Offset(_kEdgeSize * 0.25, _kEdgeSize * 0.52);


These numbers in the offsets should be converted to constants to make it easier to read/understand them. Or you could comment on how the values are derived right here in a comment if this is the only instance of the number.

gspencergoog · 2023-03-10T00:43:36Z

packages/flutter/lib/src/cupertino/toggleable.dart

+import 'package:flutter/rendering.dart';
+import 'package:flutter/widgets.dart';
+
+/// A mixin for [StatefulWidget]s that implement ios-themed toggleable


Suggested change

/// A mixin for [StatefulWidget]s that implement ios-themed toggleable

/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable

gspencergoog · 2023-03-10T00:43:59Z

packages/flutter/lib/src/cupertino/toggleable.dart

+/// A mixin for [StatefulWidget]s that implement ios-themed toggleable
+/// controls (e.g.[Checkbox]es).
+///
+/// The mixin implements the logic for toggling the control (e.g. when tapped).


Suggested change

/// The mixin implements the logic for toggling the control (e.g. when tapped).

/// The mixin implements the logic for toggling the control when tapped.

gspencergoog · 2023-03-10T00:44:25Z

packages/flutter/lib/src/cupertino/toggleable.dart

+/// the [buildToggleable]. [State] objects using this mixin should call that
+/// method from their [build] method.
+///
+/// This mixin is used to implement the cupertino components for [Checkbox]


Suggested change

/// This mixin is used to implement the cupertino components for [Checkbox]

/// This mixin is used to implement the Cupertino components for [Checkbox]

gspencergoog · 2023-03-10T01:41:18Z

packages/flutter/lib/src/cupertino/checkbox.dart

+
+  @override
+  Widget build(BuildContext context) {
+


Remove extra blank line

gspencergoog · 2023-03-10T01:41:54Z

packages/flutter/lib/src/cupertino/checkbox.dart

+        focusNode: widget.focusNode,
+        autofocus: widget.autofocus,
+        onFocusChange: onFocusChange,
+        size: const Size(48.0, 48.0),


Adaptive widgets don't need to be the same size, and I think it's probably better to have it conform to the standard interactive size for Cupertino, since that should fit better with other Cupertino widgets. It's not much different anyhow.

gspencergoog · 2023-03-16T20:47:43Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  /// The color used if the checkbox is inactive. By default,
+  /// [CupertinoColors.inactiveGray] is used.


Suggested change

/// The color used if the checkbox is inactive. By default,

/// [CupertinoColors.inactiveGray] is used.

/// The color used if the checkbox is inactive.

///

/// By default, [CupertinoColors.inactiveGray] is used.

The first paragraph is used as a short description in the docs, and should only be one sentence long.

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#dartdoc-specific-requirements

gspencergoog · 2023-03-16T20:49:50Z

packages/flutter/lib/src/cupertino/checkbox.dart

+
+  void _drawCheck(Canvas canvas, Offset origin, Paint paint) {
+    final Path path = Path();
+    // The ratios for the offsets below where found from looking at the checkbox


Suggested change

// The ratios for the offsets below where found from looking at the checkbox

// The ratios for the offsets below were found from looking at the checkbox

gspencergoog · 2023-03-16T23:15:20Z

packages/flutter/lib/src/cupertino/toggleable.dart

+import 'package:flutter/rendering.dart';
+import 'package:flutter/widgets.dart';
+
+/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable


Suggested change

/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable

/// A mixin for [StatefulWidget]s that implements iOS-themed toggleable

gspencergoog · 2023-03-16T23:16:18Z

packages/flutter/lib/src/cupertino/toggleable.dart

+import 'package:flutter/widgets.dart';
+
+/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable
+/// controls (e.g.[Checkbox]es).


Suggested change

/// controls (e.g.[Checkbox]es).

/// controls (e.g. [CupertinoCheckbox]es).

Or describe how this class relates to the Material Checkbox.

gspencergoog · 2023-03-16T23:16:34Z

packages/flutter/lib/src/cupertino/toggleable.dart

+/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable
+/// controls (e.g.[Checkbox]es).
+///
+/// The mixin implements the logic for toggling the control when tapped.


Suggested change

/// The mixin implements the logic for toggling the control when tapped.

/// This mixin implements the logic for toggling the control when tapped.

gspencergoog · 2023-03-16T23:23:38Z

packages/flutter/lib/src/cupertino/toggleable.dart

+
+  /// False if this control is "inactive" (not checked, off, or unselected).
+  ///
+  /// If value is true then the control "active" (checked, on, or selected). If


Suggested change

/// If value is true then the control "active" (checked, on, or selected). If

/// If [value] is true then the control "active" (checked, on, or selected). If

gspencergoog · 2023-03-16T23:26:01Z

packages/flutter/lib/src/cupertino/toggleable.dart

+
+  @override
+  void initState() {
+    super.initState();


You can remove this entire function, since it's only calling super.

gspencergoog · 2023-03-16T23:27:07Z

packages/flutter/lib/src/cupertino/toggleable.dart

+/// a Toggleable.
+///
+/// Subclasses must implement the [paint] method to draw the actual visuals of
+/// the Toggleable. In their [paint] method subclasses may call


Suggested change

/// the Toggleable. In their [paint] method subclasses may call

/// the Toggleable. In their [paint] method, subclasses may call

I'm going to remove this sentence entirely, because I missed that I had deleted the method this is referring too.

gspencergoog · 2023-03-16T23:27:58Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  /// If null, then the value of [CupertinoColors.white] is used.
+  final Color? checkColor;
+
+  /// If true the checkbox's [value] can be true, false, or null.


Suggested change

/// If true the checkbox's [value] can be true, false, or null.

/// If true, the checkbox's [value] can be true, false, or null.

gspencergoog · 2023-03-16T23:31:27Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  /// value is true, and to false if value is null (i.e. it cycles through false
+  /// => true => null => false when tapped).
+  ///
+  /// If tristate is false (the default), [value] is asseted to not be null.


Suggested change

/// If tristate is false (the default), [value] is asseted to not be null.

/// If tristate is false (the default), [value] must not be null, and [onChanged] will only toggle between true and false.

MitchellGoodwin · 2023-03-17T17:46:54Z

@gspencergoog this is ready for review again 👍

gspencergoog

gspencergoog · 2023-03-20T20:40:23Z

packages/flutter/lib/src/cupertino/checkbox.dart

+///  * [Checkbox], the Material Design equivalent.
+///  * [CupertinoSwitch], a widget with semantics similar to [CupertinoCheckbox].
+///  * [CupertinoSlider], for selecting a value in a range.
+///  * <https://developer.apple.com/design/human-interface-guidelines/components/selection-and-input/toggles/>


Maybe describe what this is a link to? (or make it into an actual Markdown link to the descriptive text?)

gspencergoog · 2023-03-20T20:41:44Z

packages/flutter/lib/src/cupertino/checkbox.dart

+///  * [CupertinoSlider], for selecting a value in a range.
+///  * <https://developer.apple.com/design/human-interface-guidelines/components/selection-and-input/toggles/>
+class CupertinoCheckbox extends StatefulWidget {
+  /// Creates a macOS styled checkbox.


Suggested change

/// Creates a macOS styled checkbox.

/// Creates a macOS-style checkbox.

Just a tiny nit: when I read this the first time, I read it as a "styled checkbox" for macOS.

gspencergoog · 2023-03-20T20:44:53Z

packages/flutter/lib/src/cupertino/checkbox.dart

+
+  /// The color and width of the checkbox's border.
+  ///
+  /// If this property is null, then the side defaults to a width of 1.


Suggested change

/// If this property is null, then the side defaults to a width of 1.

/// If this property is null, then the side defaults to a one pixel wide

/// black, solid border.

gspencergoog · 2023-03-20T20:50:26Z

packages/flutter/lib/src/cupertino/checkbox.dart

+  @override
+  void paint(Canvas canvas, Size size) {
+    final Paint strokePaint = _createStrokePaint();
+    final Offset origin = size / 2.0 - const Size.square(CupertinoCheckbox.width) / 2.0 as Offset;


Too bad we have to cast here (but we do). Someday we should add a asOffset to Size and a asSize to Offset.

flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels Mar 8, 2023

MitchellGoodwin commented Mar 8, 2023

View reviewed changes

MitchellGoodwin marked this pull request as ready for review March 8, 2023 23:52

MitchellGoodwin requested a review from gspencergoog March 9, 2023 23:54

gspencergoog reviewed Mar 10, 2023

View reviewed changes

MitchellGoodwin force-pushed the create-cupertino-checkbox branch from b501bdf to 9beffb6 Compare March 10, 2023 19:35

gspencergoog reviewed Mar 16, 2023

View reviewed changes

MitchellGoodwin force-pushed the create-cupertino-checkbox branch from 9beffb6 to 0810582 Compare March 17, 2023 16:48

MitchellGoodwin force-pushed the create-cupertino-checkbox branch from 3916a7b to 9a5eec2 Compare March 20, 2023 16:18

gspencergoog approved these changes Mar 20, 2023

View reviewed changes

MitchellGoodwin force-pushed the create-cupertino-checkbox branch from 9a5eec2 to f084121 Compare March 20, 2023 21:16

MitchellGoodwin added 6 commits March 20, 2023 15:59

Start of setting up checkbox

c41acb9

Create CupertinoCheckbox

4c85463

Refactor documentation and constants

bbcb515

Update documentation

77bfec1

remove whitespace

5a2cf6f

Update docs

6ccacb8

MitchellGoodwin force-pushed the create-cupertino-checkbox branch from f084121 to 6ccacb8 Compare March 20, 2023 22:59

MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 21, 2023

auto-submit bot merged commit 1ba3f99 into flutter:master Mar 21, 2023
66 checks passed

engine-flutter-autoroll mentioned this pull request Mar 21, 2023

Roll Flutter from 23f5582ea654 to 1ba3f99bb0aa (21 revisions) flutter/packages#3505

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 21, 2023

1ba3f99bb Create cupertino checkbox (flutter/flutter#122244)

5dcc501

HansMuller mentioned this pull request Mar 24, 2023

Flutter Framework Team: Notable Commits #121415

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

1ba3f99bb Create cupertino checkbox (flutter/flutter#122244)

da369e2

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

1ba3f99bb Create cupertino checkbox (flutter/flutter#122244)

2ffd06f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create cupertino checkbox #122244

Create cupertino checkbox #122244

MitchellGoodwin commented Mar 8, 2023 •

edited

MitchellGoodwin Mar 8, 2023

guidezpl commented Mar 9, 2023 •

edited

bernaferrari commented Mar 9, 2023 •

edited

gspencergoog left a comment

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023 •

edited

gspencergoog Mar 10, 2023 •

edited

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 10, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

MitchellGoodwin Mar 17, 2023

gspencergoog Mar 16, 2023

gspencergoog Mar 16, 2023

MitchellGoodwin commented Mar 17, 2023

gspencergoog left a comment

gspencergoog Mar 20, 2023

gspencergoog Mar 20, 2023

gspencergoog Mar 20, 2023

gspencergoog Mar 20, 2023

	/// true this callback cycle from false to true to null.
	/// true this callback cycle from false to true to null and back to false again.

	/// If this property is null, then the side will be width 1.
	/// If this property is null, then the side defaults to a width of 1.

	/// A mixin for [StatefulWidget]s that implement ios-themed toggleable
	/// A mixin for [StatefulWidget]s that implement iOS-themed toggleable

	/// The mixin implements the logic for toggling the control (e.g. when tapped).
	/// The mixin implements the logic for toggling the control when tapped.

	/// This mixin is used to implement the cupertino components for [Checkbox]
	/// This mixin is used to implement the Cupertino components for [Checkbox]

		/// The color used if the checkbox is inactive. By default,
		/// [CupertinoColors.inactiveGray] is used.

	// The ratios for the offsets below where found from looking at the checkbox
	// The ratios for the offsets below were found from looking at the checkbox

	/// controls (e.g.[Checkbox]es).
	/// controls (e.g. [CupertinoCheckbox]es).

	/// The mixin implements the logic for toggling the control when tapped.
	/// This mixin implements the logic for toggling the control when tapped.

	/// If value is true then the control "active" (checked, on, or selected). If
	/// If [value] is true then the control "active" (checked, on, or selected). If

	/// the Toggleable. In their [paint] method subclasses may call
	/// the Toggleable. In their [paint] method, subclasses may call

	/// If true the checkbox's [value] can be true, false, or null.
	/// If true, the checkbox's [value] can be true, false, or null.

	/// If tristate is false (the default), [value] is asseted to not be null.
	/// If tristate is false (the default), [value] must not be null, and [onChanged] will only toggle between true and false.

	/// Creates a macOS styled checkbox.
	/// Creates a macOS-style checkbox.

	/// If this property is null, then the side defaults to a width of 1.
	/// If this property is null, then the side defaults to a one pixel wide
	/// black, solid border.

Create cupertino checkbox #122244

Create cupertino checkbox #122244

Conversation

MitchellGoodwin commented Mar 8, 2023 • edited

Pre-launch Checklist

Choose a reason for hiding this comment

guidezpl commented Mar 9, 2023 • edited

bernaferrari commented Mar 9, 2023 • edited

gspencergoog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gspencergoog Mar 10, 2023 • edited

Choose a reason for hiding this comment

gspencergoog Mar 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitchellGoodwin commented Mar 17, 2023

gspencergoog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitchellGoodwin commented Mar 8, 2023 •

edited

guidezpl commented Mar 9, 2023 •

edited

bernaferrari commented Mar 9, 2023 •

edited

gspencergoog Mar 10, 2023 •

edited

gspencergoog Mar 10, 2023 •

edited