Skip to content
This repository has been archived by the owner on Oct 19, 2022. It is now read-only.

OT-717: Unreadable Status Bar #511

Merged
merged 4 commits into from
Apr 8, 2020
Merged

Conversation

phranck
Copy link
Contributor

@phranck phranck commented Apr 6, 2020

Fixes #449

BUGFIX

  • Fixes a bug that happens on switching between Light and Dark mode, which results in an unreadable system status bar

ADDITIONS & CHANGES

  • Adds a new ThemeKey enum value system
  • Changes setting for Light/Dark mode in user profile by removing the switch
  • Adds a new screen for switching application appearance to Light/Dark or System
  • Refactors all ThemeKey enum values from UPPERCASE to lowercase
  • Adds vibrateLight method for haptic feedback when user changes appearance
  • Adds new assets for all three theme states: Light, Dark, System
  • Adds method extension with a getter stringValue for enum ThemeKey to get a string of the enum value only
  • Cleans up code

ADDITIONAL NOTES

  • Adds activeColor setting to all used Switch.adaptive widgets with the value of CustomTheme.of(context).accent, because on iOS they had the default green color which doesn't fit our application color theme (unrelated regarding the topic of the ticket).

Removed after first discussion

  • Adds Preference class with just two static methods set({@required value, @required String forKey}) and value({@required String forKey}) for convenience and better readability
  • Adds static getter/setter on Preference class for easier handling of preference values and better readability (NOTE: Should be discussed if we adopt this system overall)
    So instead of calling:
    setPreference(preferenceAppThemeKey, describeEnum(ThemeKey.light));
    
    we can call:
    Preference.themeKey = ThemeKey.light.stringValue;
    

Frank Gregor added 2 commits April 6, 2020 11:53
Fixes #449

BUGFIX
* Fixes a bug that happens on switching between Light and Dark mode, which results in an unreadable system status bar

ADDITIONS & CHANGES
* Adds a new `ThemeKey` enum value `system`
* Changes setting for Light/Dark mode in user profile by removing the switch
* Adds a new screen for switching application appearance to `Light`/`Dark` or `System`
* Refactors all `ThemeKey` enum values from UPPERCASE to lowercase
* Adds `vibrateLight` method for haptic feedback when user changes appearance
* Adds new assets for all three theme states: Light, Dark, System
* Cleans up code

ADDITIONAL NOTES
* Adds `activeColor` setting to all used `Switch.adaptive` widgets with the value of `CustomTheme.of(context).accent`, because on iOS they had the default green color which doesn't fit our application color theme.
* Adds `Preference` class with just two static methods `set({@required value, @required String forKey})` and `value({@required String forKey})` for convenience and better readability
* Adds static getter/setter on `Preference` class for easier handling of preference values and better readability (NOTE: Should be discussed if we adopt this system overall)
  So instead of calling:
  ```
   setPreference(preferenceAppThemeKey, ThemeKey.light.toString());
  ```
  we can call:
  ```
  Preference.themeKey = ThemeKey.light.toString();
  ```
@phranck phranck added bug Something isn't working enhancement New feature or request labels Apr 6, 2020
@phranck phranck requested review from Boehrsi and florianhaar April 6, 2020 10:02
Copy link
Contributor

@florianhaar florianhaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After setting the appearance to "system" the status bar is completely dark. Light and dark work perfect. See attached image:
Screenshot_20200407-063454_OX COI Messenger Dev

lib/src/brandable/custom_theme.dart Outdated Show resolved Hide resolved
@phranck
Copy link
Contributor Author

phranck commented Apr 7, 2020

After setting the appearance to "system" the status bar is completely dark. Light and dark work perfect. See attached image:
Screenshot_20200407-063454_OX COI Messenger Dev

Oh damn... this is a regression. Thanks!

@phranck phranck force-pushed the OT-717_UnreadableStatusBar branch 2 times, most recently from cba61a9 to ed5a0d9 Compare April 7, 2020 10:24
@phranck
Copy link
Contributor Author

phranck commented Apr 7, 2020

@florianhaar @Boehrsi
Triggered by this Flutter issue I tried the Flutter beta channel and tested again. Et voilá - it works as expected on both Android and iOS!

* Adds missing line break after last line of code
* Fixes bug on using describeEnum(..)
* Updates assets to new versions
* Adapts corner radius at image border according Cornelius' request
* adapts UI according to Cornelius' request
* fixes Image height
* removes code regarding convenience class 'Preference'
@phranck phranck force-pushed the OT-717_UnreadableStatusBar branch from ed5a0d9 to 9e59ede Compare April 7, 2020 13:38
@Boehrsi
Copy link
Collaborator

Boehrsi commented Apr 7, 2020

Notes / stuff to check tomorrow for /me:

  • Setting the accent color on numerous elements shouldn't be required as this should normally be handled by the global theme
  • Dimensions file: 6dp value should be a specific value not a default
  • Check theme logic and compare with system and set something depending on this seems to be quite often in there. Possible to simplify in 1-2 methods?
  • Show all team members the "reduced boiler plate event / state handling" (context: lib/src/settings/settings_appearance_event_state.dart)

Copy link
Collaborator

@Boehrsi Boehrsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will check functionality and the framework problem tomorrow morning. Also have some questions / ideas we can talk about tomorrow (see my previous comment).

@Boehrsi Boehrsi merged commit cc9e6f7 into develop Apr 8, 2020
@Boehrsi Boehrsi deleted the OT-717_UnreadableStatusBar branch April 8, 2020 11:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unreadable status bar if dark mode is enabled
3 participants