Add mouse cursor property to `CupertinoRadio`#149681

Merged

auto-submit[bot] merged 5 commits intoflutter:masterfrom

victorsanni:improve-cupertino-radio

Jun 12, 2024

Contributor

victorsanni commented Jun 4, 2024 •

edited

Loading

Adds mouseCursor property in Radio to CupertinoRadio and Radio.adaptive.

The mouseCursor property added is of type MouseCursor and not WidgetStateProperty<MouseCursor> to match Radio's mouseCursor.

Part of #58192

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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


          Add mouse cursor property to CupertinoRadio

a6d3df1

github-actions bot added framework f: material design f: cupertino labels

victorsanni marked this pull request as ready for review

June 4, 2024 18:59

victorsanni requested review from MitchellGoodwin, Piinks and justinmc

June 4, 2024 19:46

Piinks reviewed

View reviewed changes

packages/flutter/lib/src/cupertino/radio.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/cupertino/radio.dart Show resolved Hide resolved

packages/flutter/lib/src/cupertino/radio.dart Outdated Show resolved Hide resolved

packages/flutter/test/cupertino/radio_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/cupertino/radio_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/cupertino/radio_test.dart

@@ @@ -428,4 +429,45 @@ void main() { @@
                   // Release pointer after widget disappeared.
                   await gesture.up();
                 });

Contributor

Piinks Jun 4, 2024

Can you add a test that sets and verifies the mouse cursor when the user provides a WidgetStateProperty?

Contributor Author

victorsanni Jun 5, 2024 •

edited

Loading

Ah. I think I've run into an issue here.

It turns out that I can't even provide a WidgetStateProperty<MouseCursor> to the Material equivalent - only MouseCursors are allowed. Should I

a) Refactor in CupertinoRadio so that mouseCursor is a WidgetStateProperty<MouseCursor> and not a MouseCursor (possibly doing the same in Radio) or

b) Have mouseCursor be a plain old MouseCursor here?

cc @MitchellGoodwin

Contributor

justinmc Jun 5, 2024

I wonder if we have an existing plan to convert parameters like these to WidgetStateProperty, maybe @MitchellGoodwin knows? Should we be making stuff like this a WidgetStateProperty when we can?

Contributor Author

victorsanni Jun 11, 2024

What I currently do is keep mouseCursor as a MouseCursor, then if the user wants to configure mouseCursor in different widget states they can use WidgetStateMouseCursor. I added a test reflecting this behavior.

packages/flutter/test/cupertino/radio_test.dart Outdated Show resolved Hide resolved

justinmc approved these changes

View reviewed changes

Contributor

justinmc left a comment

LGTM with the caveat that we should figure out what to do about making mouseCursor a WidgetStateProperty.

packages/flutter/lib/src/cupertino/radio.dart Outdated Show resolved Hide resolved

packages/flutter/test/cupertino/radio_test.dart

@@ @@ -428,4 +429,45 @@ void main() { @@
                   // Release pointer after widget disappeared.
                   await gesture.up();
                 });

Contributor

justinmc Jun 5, 2024

I wonder if we have an existing plan to convert parameters like these to WidgetStateProperty, maybe @MitchellGoodwin knows? Should we be making stuff like this a WidgetStateProperty when we can?

packages/flutter/test/cupertino/radio_test.dart

+                  await gesture.moveTo(tester.getCenter(find.byType(CupertinoRadio<int>)));
+                  expect(RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1),
+                      kIsWeb ? SystemMouseCursors.click : SystemMouseCursors.basic);
+                });

Contributor

justinmc Jun 5, 2024

Nit: Maybe also add a test to Radio.adaptive that tests that CupertinoRadio ends up with the mouseCursor.

Contributor Author

victorsanni Jun 10, 2024 •

edited

Loading

Might such a test be necessary, considering Radio.adaptive delegates to CupertinoRadio on iOS/macOS (and a test for that exists), and given the CupertinoRadio mouse cursor tests written here?

victorsanni added 3 commits

June 10, 2024 15:12


          Add test that mouse cursor resolves states

c0c0a7e


          Merge branch 'master' into improve-cupertino-radio

ba02060


          Change system cursor from grab to basic

743ea3d

victorsanni changed the title ~~Add mouse cursor property to CupertinoRadio~~ Add mouse cursor property to CupertinoRadio


          Remove extra addTearDown

Piinks added the autosubmit label

auto-submit bot merged commit 3db2ece into flutter:master

engine-flutter-autoroll mentioned this pull request

Roll Flutter from 14df7be3f947 to 3db2ece735f1 (13 revisions) flutter/packages#6913

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

1e76395

…r#149681)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from 14df7be3f947 to 0d5bf0f54855 (14 revisions) flutter/packages#6914

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

4442e42

…r#149681)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from 14df7be3f947 to 0c692b86ba5e (17 revisions) flutter/packages#6916

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

76cc209

…r#149681)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from b1f9d7131c8c to 034966b3ccc4 (12 revisions) flutter/packages#6919

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

ee09e0a

…r#149681)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from b1f9d7131c8c to 01db23b82233 (15 revisions) flutter/packages#6921

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

dc6254f

…r#149681)

auto-submit bot pushed a commit to flutter/packages that referenced this pull request


          Roll Flutter from b1f9d71 to 01db23b (15 revisions) (#6921)

a0f2552

flutter/flutter@b1f9d71...01db23b

2024-06-13 [email protected] Roll Flutter Engine from c7fcbfce608f to 4cb3025d3abf (28 revisions) (flutter/flutter#150199)
2024-06-13 [email protected] Revert "[CupertinoActionSheet] Add sliding tap gesture" (flutter/flutter#150147)
2024-06-13 [email protected] RawScrollbar: don't listen for drag gestures when scrolling is not possible (flutter/flutter#149925)
2024-06-13 [email protected] Update testowners (flutter/flutter#150141)
2024-06-12 [email protected] Revert "Reland 2: [CupertinoActionSheet] Match colors to native" (flutter/flutter#150142)
2024-06-12 [email protected] Reland 2: [CupertinoActionSheet] Match colors to native (flutter/flutter#150129)
2024-06-12 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.6 to 4.1.7 (flutter/flutter#150132)
2024-06-12 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.8 to 3.25.9 (flutter/flutter#150133)
2024-06-12 [email protected] Improve build time when using SwiftPM (flutter/flutter#150052)
2024-06-12 [email protected] Reland: Request focus if accessibility focus is given to a Focus widget (#142942) (flutter/flutter#149840)
2024-06-12 [email protected] Update WidgetStatesController docs (flutter/flutter#150081)
2024-06-12 [email protected] [Reland] Fix `SegmentedButton` clipping when drawing segments (#149739) (flutter/flutter#150090)
2024-06-12 [email protected] Fix markdown hyperlinks in the style guide (flutter/flutter#150071)
2024-06-12 [email protected] Update packages desktop PR triage link lables (flutter/flutter#150124)
2024-06-12 [email protected] Add mouse cursor property to `CupertinoRadio` (flutter/flutter#149681)

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

victorsanni added a commit to victorsanni/flutter that referenced this pull request


          Add mouse cursor property to CupertinoRadio (flutter#149681)

187c6b1

Adds `mouseCursor` property in `Radio` to `CupertinoRadio` and `Radio.adaptive`.

The `mouseCursor` property added is of type `MouseCursor` and not `WidgetStateProperty<MouseCursor>` to match `Radio`'s `mouseCursor`.

victorsanni added a commit to victorsanni/flutter that referenced this pull request


          Add mouse cursor property to CupertinoRadio (flutter#149681)

070538e

Adds `mouseCursor` property in `Radio` to `CupertinoRadio` and `Radio.adaptive`.

The `mouseCursor` property added is of type `MouseCursor` and not `WidgetStateProperty<MouseCursor>` to match `Radio`'s `mouseCursor`.

victorsanni deleted the improve-cupertino-radio branch

July 12, 2024 00:28

victorsanni mentioned this pull request

Make CupertinoRadio's mouseCursor a WidgetStateProperty #151910

Merged

9 tasks

auto-submit bot pushed a commit that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (#151910)

5ef969a

#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in #151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: #149681.

Part of #58192

victorsanni added a commit to victorsanni/flutter that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (flutte…

e224308

…r#151910)

flutter#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in flutter#151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: flutter#149681.

Part of flutter#58192

victorsanni added a commit to victorsanni/flutter that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (flutte…

acca84e

…r#151910)

flutter#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in flutter#151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: flutter#149681.

Part of flutter#58192

victorsanni mentioned this pull request

[CP] Make CupertinoRadio's mouseCursor a WidgetStateProperty (#151910) #152051

Closed

victorsanni added a commit to victorsanni/flutter that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (flutte…

27705e6

…r#151910)

flutter#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in flutter#151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: flutter#149681.

Part of flutter#58192

victorsanni mentioned this pull request

[CP] Make CupertinoRadio's mouseCursor a WidgetStateProperty (#151910) #152124

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          3db2ece Add mouse cursor property to CupertinoRadio (flutter/flutte…

991be29

…r#149681)

TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (flutte…

df6c9ed

…r#151910)

flutter#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in flutter#151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: flutter#149681.

Part of flutter#58192

Buchimi pushed a commit to Buchimi/flutter that referenced this pull request


          Make CupertinoRadio's mouseCursor a WidgetStateProperty (flutte…

0116ea0

…r#151910)

flutter#149681 introduced `mouseCursor `to `CupertinoRadio` as a `MouseCursor` instead of a `WidgetStateProperty` to match Material Radio's `mouseCursor` property for `.adaptive`.

This PR changes `mouseCursor` to be of type `WidgetStateProperty<MouseCursor>` as per review comments in flutter#151788 (comment).

PR bringing `mouseCursor` into `CupertinoRadio`: flutter#149681.

Part of flutter#58192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit f: cupertino f: material design framework