Add mouse cursor property to `CupertinoRadio` #149681

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.

Sign up for GitHub

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

Jump to bottom

Merged

auto-submit merged 5 commits into flutter:master from 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

    
                  expect(find.byKey(key), findsNothing);

                  // 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

    
                  expect(find.byKey(key), findsNothing);

                  // 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