[PR #1707] [MERGED] Improve Theming #3065

Closed
opened 2025-11-26 23:27:51 -06:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/1707
Author: @fedemkr
Created: 1/3/2022
Status: Merged
Merged: 1/24/2022
Merged by: @fedemkr

Base: masterHead: bug/theming-fix


📝 Commits (6)

  • cc6f3f4 Improved theming logic and performance, also fixed some issues regarding changing the theme after vault timeout and fixed theme applying on password generator/history
  • c85a0b8 Merge branch 'master' into bug/theming-fix
  • ce018b8 Removed messenger from theme update, and now the navigation stack is traversed and each IThemeDirtablePage gets theme updated
  • 791ec8f Merge branch 'master' into bug/theming-fix
  • 3b93dfa Merge branch 'master' into bug/theming-fix
  • e67fb5a Improved code on update theme on pages

📊 Changes

16 files changed (+263 additions, -83 deletions)

View changed files

📝 src/Android/MainActivity.cs (+18 -16)
📝 src/App/App.xaml.cs (+13 -11)
📝 src/App/Pages/BaseContentPage.cs (+14 -0)
📝 src/App/Pages/Generator/GeneratorHistoryPage.xaml.cs (+13 -3)
📝 src/App/Pages/Generator/GeneratorHistoryPageViewModel.cs (+23 -5)
📝 src/App/Pages/Generator/GeneratorPage.xaml (+1 -0)
📝 src/App/Pages/Generator/GeneratorPage.xaml.cs (+21 -10)
📝 src/App/Styles/Black.xaml.cs (+1 -1)
📝 src/App/Styles/Dark.xaml.cs (+1 -1)
src/App/Styles/IThemeDirtablePage.cs (+15 -0)
src/App/Styles/IThemeResourceDictionary.cs (+6 -0)
📝 src/App/Styles/Light.xaml.cs (+1 -1)
📝 src/App/Styles/Nord.xaml.cs (+1 -1)
src/App/Utilities/PageExtensions.cs (+53 -0)
📝 src/App/Utilities/ThemeManager.cs (+80 -34)
📝 src/iOS/AppDelegate.cs (+2 -0)

📄 Description

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Improve theming logic and performance, also fix issues on theme applying on password generator and password generator history. Also fix exception raise on applying theme because of collection modification on enumeration because of the merged dictionaries.

It should fix https://appcenter.ms/users/kspearrin/apps/bitwarden/crashes/errors/848906559u/overview

Code changes

  • ThemeManager.cs: Changed several things here to improve performance like checking whether the current theme is already the one we want to set. Also only change the theme ResourceDictionary instead of clearing everything and adding all resources again, this was crashing the app on previous versions and raising exceptions leading to inconsistent theming state on some screens
  • MainActivity.cs and AppDelegate.cs: Added messenger send to report when the app appeared so that we can update the password generated screens on theme change
  • Generator...: Added logic to update the theming correctly on these screens
  • App.xaml.cs: Changed UpdateTheme so that it's done in proper order and applied without any issue on race conditions

Testing requirements

Steps

  • Unlock the app
  • Put the app to background
  • Change OS theme
  • Wait the vault timeout (you can set it previously on the app to 1 minute so you don't have to wait that much)
  • Return to the app
  • The colors and theme are weird on some places of the lock screen

This also affects Password Generator and Password Generator History screens, i.e.:

  • Go to Password Generator screen and then to its history (three dots)
  • Change the OS theme without waiting the vault timeout
  • Go back to the app without waiting the vault timeout
  • You'll see that not all passwords themes are changed
  • Also if you go back, the generated password of the Password Generator screen will be with previous theme

Before you submit

  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/1707 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 1/3/2022 **Status:** ✅ Merged **Merged:** 1/24/2022 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `master` ← **Head:** `bug/theming-fix` --- ### 📝 Commits (6) - [`cc6f3f4`](https://github.com/bitwarden/android/commit/cc6f3f4cc81f0e7c117f20d5c0bc44ef50ee2a90) Improved theming logic and performance, also fixed some issues regarding changing the theme after vault timeout and fixed theme applying on password generator/history - [`c85a0b8`](https://github.com/bitwarden/android/commit/c85a0b8f6710c18fb43647711124e3564ecffac3) Merge branch 'master' into bug/theming-fix - [`ce018b8`](https://github.com/bitwarden/android/commit/ce018b83e7fab7d1cd7dbcbb18bd7d6ceb49b71f) Removed messenger from theme update, and now the navigation stack is traversed and each IThemeDirtablePage gets theme updated - [`791ec8f`](https://github.com/bitwarden/android/commit/791ec8f54c6d9ecbf608f04839aea4add54bfbf7) Merge branch 'master' into bug/theming-fix - [`3b93dfa`](https://github.com/bitwarden/android/commit/3b93dfa0d5f73e1c251cc264ce920b3ecb515bd6) Merge branch 'master' into bug/theming-fix - [`e67fb5a`](https://github.com/bitwarden/android/commit/e67fb5a49089cf4d489c6771c4c41b5fc4dc02aa) Improved code on update theme on pages ### 📊 Changes **16 files changed** (+263 additions, -83 deletions) <details> <summary>View changed files</summary> 📝 `src/Android/MainActivity.cs` (+18 -16) 📝 `src/App/App.xaml.cs` (+13 -11) 📝 `src/App/Pages/BaseContentPage.cs` (+14 -0) 📝 `src/App/Pages/Generator/GeneratorHistoryPage.xaml.cs` (+13 -3) 📝 `src/App/Pages/Generator/GeneratorHistoryPageViewModel.cs` (+23 -5) 📝 `src/App/Pages/Generator/GeneratorPage.xaml` (+1 -0) 📝 `src/App/Pages/Generator/GeneratorPage.xaml.cs` (+21 -10) 📝 `src/App/Styles/Black.xaml.cs` (+1 -1) 📝 `src/App/Styles/Dark.xaml.cs` (+1 -1) ➕ `src/App/Styles/IThemeDirtablePage.cs` (+15 -0) ➕ `src/App/Styles/IThemeResourceDictionary.cs` (+6 -0) 📝 `src/App/Styles/Light.xaml.cs` (+1 -1) 📝 `src/App/Styles/Nord.xaml.cs` (+1 -1) ➕ `src/App/Utilities/PageExtensions.cs` (+53 -0) 📝 `src/App/Utilities/ThemeManager.cs` (+80 -34) 📝 `src/iOS/AppDelegate.cs` (+2 -0) </details> ### 📄 Description ## Type of change - [X] Bug fix - [ ] New feature development - [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> Improve theming logic and performance, also fix issues on theme applying on password generator and password generator history. Also fix exception raise on applying theme because of collection modification on enumeration because of the merged dictionaries. It should fix [https://appcenter.ms/users/kspearrin/apps/bitwarden/crashes/errors/848906559u/overview](https://appcenter.ms/users/kspearrin/apps/bitwarden/crashes/errors/848906559u/overview ) ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> * **ThemeManager.cs:** Changed several things here to improve performance like checking whether the current theme is already the one we want to set. Also only change the theme ResourceDictionary instead of clearing everything and adding all resources again, this was crashing the app on previous versions and raising exceptions leading to inconsistent theming state on some screens * **MainActivity.cs and AppDelegate.cs:** Added messenger send to report when the app appeared so that we can update the password generated screens on theme change * **Generator...:** Added logic to update the theming correctly on these screens * **App.xaml.cs:** Changed UpdateTheme so that it's done in proper order and applied without any issue on race conditions ## Testing requirements <!--What functionality requires testing by QA? This includes testing new behavior and regression testing--> Steps - Unlock the app - Put the app to background - Change OS theme - Wait the vault timeout (you can set it previously on the app to 1 minute so you don't have to wait that much) - Return to the app - The colors and theme are weird on some places of the lock screen This also affects Password Generator and Password Generator History screens, i.e.: - Go to Password Generator screen and then to its history (three dots) - Change the OS theme without waiting the vault timeout - Go back to the app without waiting the vault timeout - You'll see that not all passwords themes are changed - Also if you go back, the generated password of the Password Generator screen will be with previous theme ## Before you submit - [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required) - [ ] This change requires a **documentation update** (notify the documentation team) - [ ] This change has particular **deployment requirements** (notify the DevOps team) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2025-11-26 23:27:51 -06:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#3065