GAM3RG33K / flutter_settings_screens

Settings Screen with Custom Shared Preference Interface
MIT License
176 stars 71 forks source link

Nested `ValueChangeObserver` broken in 0.3.3 #90

Open agersant opened 2 years ago

agersant commented 2 years ago

Hello!

I was updating my dependencies and I noticed flutter_settings_screen largely stopped working for me in version 0.3.3. Specifically, my app makes use of ValueChangeObserver to detect the user updating their theme preferences - and these observers stopped refreshing. I believe the issue came from this commit: https://github.com/GAM3RG33K/flutter_settings_screens/commit/e08dda11b8c1fce7513b5d74dbbec8ba434b8738

In my case, there is a ValueChangeObserver wrapping the entire app, and also one specifically in the settings page to show users their current preference. As soon as a RadioSettingsTile gets disposed of, all the notifiers are removed from the global _notifiers map and changes no longer get observed.

I'm happy to pin my version to 0.3.2 but I hope this can be fixed in a future release. Thanks!

BriccioCulicio commented 2 years ago

Hello, I'm starting with flutter and I was wondering how to check for a ValueChangeObserver of a key and depending on what the value is change the app theme.

Here's an example of how I used it with my Dark Mode feature in my main file, I'm trying to do this but with a custom list of values that change the theme of the app.

@override
  Widget build(BuildContext context) {
    return ValueChangeObserver<bool>(
        cacheKey: ConfigPage.KeyDarkMode,
        defaultValue: true,
        builder: (_, isDarkMode, __) => MaterialApp(
              title: _title,
              theme: isDarkMode
                  ? ThemeData.dark().copyWith(
                      primaryColor: Colors.teal,
                      scaffoldBackgroundColor: const Color(0xFF170635),
                      canvasColor: const Color(0xFF170635),
                    )
                  : ThemeData.light().copyWith(),
              home: const PagPrin(),
            ));
  }
}

And here's the DropDown Menu which has a key that I want to use to change the theme of the app itself

Widget buildTheme() => DropDownSettingsTile(
      settingKey: keyTheme,
      title: "Colors ",
      selected: 1,
      values: <int, String>{
        1: 'Select color',
        2: 'White',
        3: 'Brown',
        4: 'Grey',
      },
      onChange: (colors) {/*NOOP*/},
    );
Dragon-Seeker commented 2 years ago

Can confirm when using nested settings within something like childrenIfEnabled:[] will cause issues as when they are disposed after being disabled, they will clear the entire map causing all value observers to break.

Dragon-Seeker commented 2 years ago

Hello, I'm starting with flutter and I was wondering how to check for a ValueChangeObserver of a key and depending on what the value is change the app theme.

Here's an example of how I used it with my Dark Mode feature in my main file, I'm trying to do this but with a custom list of values that change the theme of the app.

@override
 Widget build(BuildContext context) {
   return ValueChangeObserver<bool>(
       cacheKey: ConfigPage.KeyDarkMode,
       defaultValue: true,
       builder: (_, isDarkMode, __) => MaterialApp(
             title: _title,
             theme: isDarkMode
                 ? ThemeData.dark().copyWith(
                     primaryColor: Colors.teal,
                     scaffoldBackgroundColor: const Color(0xFF170635),
                     canvasColor: const Color(0xFF170635),
                   )
                 : ThemeData.light().copyWith(),
             home: const PagPrin(),
           ));
 }
}

And here's the DropDown Menu which has a key that I want to use to change the theme of the app itself

Widget buildTheme() => DropDownSettingsTile(
      settingKey: keyTheme,
      title: "Colors ",
      selected: 1,
      values: <int, String>{
        1: 'Select color',
        2: 'White',
        3: 'Brown',
        4: 'Grey',
      },
      onChange: (colors) {/*NOOP*/},
    );

This is not the place for such discussion as your problem dose not concern the problem at hand but of a lack of understanding. Please make a new issue or look further into the Docs on pub.dev

agersant commented 1 year ago

This scenario is still broken in flutter_settings_screens 0.3.3-null-safety+2.

There is one ValueChangeObserver at the root of the app, watching for the cacheKey corresponding to the user's preferred theme. There is an additional ValueChangeObserver for that same key, created in the app's Settings page. When the ValueChangeObserver in the settings page gets disposed, the notifier for the app-level ChangeObserved is removed. The offending code is still the dispose method in _ValueChangeObserverState.

thorito commented 1 year ago

I had a similar problem but I think the problem is due to another reason.

The change listener sets the key to lowercase, so if your key contains any uppercase characters it won't trigger the notification.

settings.dart -> _fetchNotifiersForKey(key);

List? _fetchNotifiersForKey(String key) { final finalKey = key.toLowerCase().trim(); // <-- possible problem return _notifiers[finalKey]; }

I am using version: flutter_settings_screens: ^0.3.3-null-safety+2