BirjuVachhani / adaptive_theme

Easiest way to add support for light and dark theme in your flutter app.
https://pub.dev/packages/adaptive_theme
Apache License 2.0
464 stars 37 forks source link

[AdaptiveTheme] builder parameters #25

Closed IvoBiaus closed 2 years ago

IvoBiaus commented 3 years ago

Describe the bug The builder functions gives 2 parameters, both are Themes. According to the documentation and the examples like: builder: (theme, darkTheme) => MaterialApp(... the first one is always Light Theme and the second one is always Dark Theme. But its currently not working that way, both parameters are sending the same twice. When the app is on Light mode both params are Light Theme, and when the app is in dark mode, both params are Dark Theme.

To Reproduce Steps to reproduce the behavior:

  1. Create an AdaptiveTheme with dark and light themes
  2. At the AdaptiveTheme's "builder" function call a print with both theme params and check they are the same at both app modes.

Expected behavior A clear and concise description of what you expected to happen.

Screenshots image image

Desktop (please complete the following information): Doesnt matter

Smartphone (please complete the following information): Doesnt matter

Additional context Add any other context about the problem here.

felipemy commented 3 years ago

Have similar problem. On my test, calling AdaptiveTheme.getThemeMode() returned the light theme (that is set as initial), but the theme on the device is always the dark. And by calling AdaptiveTheme.of(context).setLight() does nothing, stills dark. If the app is restarted, it stills reports ThemeMode == light, no matter what was the set, but always display the dark, so also looks like not saving as well. Both on android and ios, it set the theme based on the system theme. So the only way to change the theme is by change the device, that is working.

Expected:

  1. ignore system theme since initial theme is set
  2. set the correct theme when invoking manual function
  3. save it accordingly
BirjuVachhani commented 3 years ago

@felipemy Can you check if this is working for you? https://adaptivetheme.codemagic.app/

BirjuVachhani commented 3 years ago

@IvoBiaus It is intended that way that both the theme in the builder is light when you set theme to light mode. The name darkTheme in example indicates that that param is to be passed to darkTheme param of MaterialApp.

I it the core of the package that gives it ability to switch themes based on system preference. It's the way flutter works. Since we are not passing mode as in themeMode to the MaterialApp, it considers it to be system dependent so passing both as light theme gives the package real control on it. Hence it is not a BUG!!

BirjuVachhani commented 3 years ago

@IvoBiaus What are you trying to achieve here and what issues is it causing to you?

felipemy commented 3 years ago

@BirjuVachhani, running the example app in browser seems to be working fine. So I understand that is not a system problem. The package was implemented exactly as described in the documentation, but is not working as expected from start. May be there is conflict with other package, but couldn't identify what is causing the error since all methods of the Adaptive seems to return right, just the theme that doesn't get applied by calling manually, only by changing on device.

IvoBiaus commented 3 years ago

[It is intended that way that both the theme in the builder is light when you set theme to light mode. ...]

[... so passing both as light theme gives the package real control on it. ...]

I'm not sure i understood you compleatly; So, the fact that both parameters are the same is on purpose? in that case, why not just use one?

[... The name darkTheme in example indicates that that param is to be passed to darkTheme param of MaterialApp. ...]

Yes i know that, but as i showed in the screenshot the darkTheme from the builder its a lightTheme when the app is set to lightMode and its a darkTheme when the app is set to darkMode, same happens for the first parameter. So if both parameters are intended to be the same, you can remove one 🤷‍♂️

You are not sending the (lightTheme, darkTheme) => ... insted seems its: (currentTheme, currentTheme) => ...

BirjuVachhani commented 2 years ago

@IvoBiaus Both params will be different if theme mode is system.