After updating the parameter we can assign widget.mouseCursor and followed by default.

Like
final WidgetStateProperty<MouseCursor> effectiveMouseCursor = widget.mouseCursor ?? <Default>;

If I understand correctly does this mean it should be rewritten as

final MouseCursor effectiveMouseCursor = widget.mouseCursor ??      
    WidgetStateProperty.resolveWith<MouseCursor>((Set<WidgetState> states) {
        return states.contains(WidgetState.disabled)
              ? SystemMouseCursors.basic
              : kIsWeb ? SystemMouseCursors.click : SystemMouseCursors.basic
            );

(And therefore the default cursor can be extracted as a const class?)

But buildToggleable expects effectiveMouseCursor to be of type WidgetStateProperty not of type MouseCursor.

I see. Thank you for explaining.

Reviewing this I realized some widgets still "resolve" non-state cursor. This is to support WidgetStateMouseCursor. Widgets such as buttons already support this. Check out official sample here
https://api.flutter.dev/flutter/widgets/WidgetStateMouseCursor-class.html

I created a Checkbox.adaptive for your PR to test this. Try this sample before applying the suggestion and then after it.

Code sample

import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Scaffold(
        appBar: AppBar(
          title: const Text('Sample'),
        ),
        body: const Checkbox.adaptive(
          mouseCursor: CheckboxCursor(),
          value: true,
          // onChanged: (bool? value) {},
          onChanged: null,
        ),
        floatingActionButton: FloatingActionButton(
          onPressed: () {},
          child: const Icon(Icons.add),
        ),
      ),
    );
  }
}

class CheckboxCursor extends WidgetStateMouseCursor {
  const CheckboxCursor();

  @override
  MouseCursor resolve(Set<WidgetState> states) {
    if (states.contains(WidgetState.disabled)) {
      return SystemMouseCursors.forbidden;
    }
    return SystemMouseCursors.basic;
  }

  @override
  String get debugDescription => 'CheckboxCursor()';
}

This will not work with WidgetStatePropertyAll, we should use resolveWith with the Checkbox states to support WidgetStateMouseCursor. This allows allows using mouse states for Cupertino Checkbox from adaptive Checkbox.

If you apply this suggestion, we will need adaptive checkbox test to cover this use case

Ah because WidgetStateMouseCursor is a MouseCursor. This is very tricky, though, or maybe I just need to study up on WidgetStateProperty.

• If @victorsanni migrates Checkbox.mouseCursor to be a WidgetStateProperty, then will this code need to change?
• Am I supposed to be able to pass around a WidgetStateMouseCursor as if it were a MouseCursor and it will just work? Though actually the consumer of the WidgetStateMouseCursor must use resolveAs?

It will be really easy to make this same mistake elsewhere in the codebase.

Why can't we make CupertinoCheckbox.mouseCursor simply MouseCursor? WidgetStateMouseCursor extends MouseCursor and we always resolve them at some point.

Why can't we make CupertinoCheckbox.mouseCursor simply MouseCursor? WidgetStateMouseCursor extends MouseCursor and we always resolve them at some point.

I initially thought the same way too.

@justinmc mentioned WidgetStateMouseCursor felt like a hack to get MouseCursor and WidgetStateProperty<MouseCursor> to play well with each other. @TahaTesser also mentioned that new properties like color, mouse cursor, etc. should be introduced as WidgetStatePropertys so that they can be resolved in different WidgetStates.

An extra point is CheckboxThemeDatas mouseCursor is a WidgetStateProperty. So for later (possible) .adaptive work, passing WidgetStatePropertys would make it so that the property can be resolved with non-stale states in CupertinoCheckbox, instead of using stale states in Checkbox.

Does this mean we're deprecating WidgetStateMouseCursor? I don't think this is a hack. At least we have other classes that act like this as well, such as CupertinoDynamicColor, which extends Color and the widget will always resolve an incoming Color for possible brightness variety.

I think we should either:

• Use MouseCursor as the type, and always resolve it by states. Or:
• Use WidgetStateProperty<MouseCursor> as the type, but clearly state that WidgetStateMouseCursor is to be deprecated and discouraged, and add another constructor as an adaptor from T to WidgetStateProperty<T> for convenience, instead of having to use this "double wrapping" every time.

instead of having to use this "double wrapping" every time.

@dkwingsmt what do you think about migrating existing properties from T to WidgetStateProperty<T>?

For example, in this case, that would entail converting Material Checkboxs mouseCursor to be of type WidgetStateProperty<MouseCursor> instead of MouseCursor to remove the need for this double wrapping.

In my opinion, we should use MouseCursor as the type, and resolve it if a WidgetStateProperty is given, the same way mouseCursor works for the current Material checkbox. The adaptive checkbox should pass mouseCursor to the Cupertino widget with as few transformations and let the Cupertino widget handle it's default behavior if it's null.

WidgetStateMouseCursor I believe is never used as a type for constructor arguments, but it is used regularly as a default behavior with it's static const constructors like WidgetStateMouseCursor.clickable. It seems convenient for devs to make their own default behavior too. I don't see a need to deprecate it.

Ideally a property of WidgetStateProperties<T> would just automatically handle if it's passed a property of just type T, but Dart doesn't allow that currently. In several places we have parameters of type T but we check if it's a WidgetState and resolve it appropriately, then call it out in the documentation. I think that's slightly better than using WidgetStateProperty<T> in a lot of cases.

I agree. I think we should stick with the current convention for using MouseCursor. It might be a gimmick, but a good one (in my opinion).