rydmike / flex_color_picker

A highly customizable Flutter color picker.
BSD 3-Clause "New" or "Revised" License
200 stars 42 forks source link

SetState() in onChanged() causes picker to break #78

Closed eriksoderstrom closed 6 months ago

eriksoderstrom commented 9 months ago

Edit: Setting the 'color' property of ColorPicker fixes this issue. This is mentioned under the Screen Color Picker section.

ColorPicker(
  pickersEnabled: const <ColorPickerType, bool>{
    ColorPickerType.wheel: true,
    ColorPickerType.accent: true,
    ColorPickerType.primary: true,,
  },
  color: selectedColor, // Required if using setState in onColorChanged.
  onColorChanged: (Color value) {
    print('Color selected: $value');
    setState(() {
      selectedColor = value;
    });
  },
),

Hello!

I've encountered this weird issue when using setState in the onChanged callback.

As soon as I add the setState call, the picker stops working. The onChanged callback is called, but the color doesn't change in the UI.

Removing the setState call immediately fixes the issue, and the picker starts working again.

This problem occurs in both my main project, and I've been able to reproduce the issue in a minimal project. https://github.com/eriksoderstrom/test_color_picker Tested on iPhone 12 and Pixel 6 physical devices.

class SelectColorPage extends StatefulWidget {
  const SelectColorPage({super.key});
  @override
  State<SelectColorPage> createState() => _SelectColorPageState();
}

class _SelectColorPageState extends State<SelectColorPage> {
  Color selectedColor = Colors.blue;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('Select Color'),
      ),
      body: Column(
        children: <Widget>[

          ColorPicker(
            pickersEnabled: const <ColorPickerType, bool>{
              ColorPickerType.wheel: true,
              ColorPickerType.accent: true,
              ColorPickerType.primary: true,
              ColorPickerType.bw: false,
              ColorPickerType.custom: false,
            },
            onColorChanged: (Color value) {
              // This line causes the issue
              print('Color selected: $value');
              setState(() {
                selectedColor = value;
              });
            },
          ),
        ],
      ),
    );
  }
}

Flutter doctor output:

[✓] Flutter (Channel stable, 3.19.1, on macOS 14.2.1 23C71 darwin-x64, locale en-SE)
[✓] Android toolchain - develop for Android devices (Android SDK version 32.0.0-rc1)
[!] Xcode - develop for iOS and macOS (Xcode 15.2)
    ! CocoaPods 1.11.3 out of date (1.13.0 is recommended).
        CocoaPods is used to retrieve the iOS and macOS platform side's plugin code that responds to your plugin usage on the Dart side.
        Without CocoaPods, plugins will not work on iOS or macOS.
        For more info, see https://flutter.dev/platform-plugins
      To upgrade see https://guides.cocoapods.org/using/getting-started.html#updating-cocoapods for instructions.
[✓] Chrome - develop for the web
[✓] Android Studio (version 2023.1)
[✓] IntelliJ IDEA Community Edition (version 2020.3.3)
[✓] VS Code (version 1.86.0)
[✓] Connected device (4 available)
[✓] Network resources

! Doctor found issues in 1 category.
rydmike commented 9 months ago

Thanks @eriksoderstrom for the report.

Seems like you solved it already 👍🏻

It works pretty much the same way like other widgets with "value" setter and callback changer. Like e.g. the Switch:

import 'package:flutter/material.dart';
void main() => runApp(const SwitchApp());

class SwitchApp extends StatelessWidget {
  const SwitchApp({super.key});
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(useMaterial3: true),
      home: Scaffold(
        appBar: AppBar(title: const Text('Switch Sample')),
        body: const Center(
          child: SwitchExample(),
        ),
      ),
    );
  }
}

class SwitchExample extends StatefulWidget {
  const SwitchExample({super.key});
  @override
  State<SwitchExample> createState() => _SwitchExampleState();
}

class _SwitchExampleState extends State<SwitchExample> {
  bool light = true;

  @override
  Widget build(BuildContext context) {
    return Switch(
      // This bool value toggles the switch.
      value: light,
      activeColor: Colors.red,
      onChanged: (bool value) {
        // This is called when the user toggles the switch.
        setState(() {
          light = value;
        });
      },
    );
  }
}

However, since selectedColor is not required, but widget it will indeed not update if it is not fed back into the ColorPickerType, and yes it is possible to omit it, unlike the Switch example where it is required.

For the use case you show it does not really make a lot of sense why the value is not required, but I seem to recall that it was needed for some construct with one of the dialogs. It could just have been related to that I wanted it to have a default if you have and undefined value until users has selected one.

I made a note the check if there is anything in the picker design that would prevent me from making it required. If there is not I should probably do so in order to avoid this pitfall. The change would be API breaking, but I have other breaking changes on the roadmap in major release at some point, so at least then I can check if this change is viable too.

rydmike commented 8 months ago

I am not going to make color required for generation 3.x.x, as it would be a breaking change. I will however do so for version 4.0.0, which will contain many breaking changes anyway.

The behavior you describe is expected though. If you don't feed the selectedColor back to color picker's color property, it cannot know you have updated in set state, that's just how setter with callback changers work.

rydmike commented 6 months ago

Improved the API docs for ColorPicker properties color and onColorChnagedin version 3.5.0.

See:

Closing this issue.