[PR #4581] [MERGED] PM-16850-PM-16851-PM-16852 - Updating full screen loading indicator #5053

New Issue

2025-11-26T23:53:23-06:00

GiteaMirror commented

2025-11-26 23:53:23 -06:00

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/4581
Author: @phil-livefront
Created: 1/17/2025
Status: ✅ Merged
Merged: 1/20/2025
Merged by: @phil-livefront

Base: main ← Head: phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators

📝 Commits (10+)

2a73506 PM-16850-PM-16851-PM-16852 - update all dialog loading states to use full screen popup
e04c66b fix linter warnings
ce12c77 fix up unit tests
aa5e290 remove BitwardenFullScreenLoadingContent and refactor to allow the use of the loading popup
b7a6303 PR comments
49c2f7a lint fixes
bd3d623 PR comments
68e2264 Merge branch 'main' into phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators
5d5f941 fix tests to allow build to pass
8ee3e81 Merge branch 'main' into phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators

📊 Changes

29 files changed (+210 additions, -399 deletions)

View changed files

➖ app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt (+0 -43)
📝 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt (+11 -4)
➖ app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt (+0 -158)
📝 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenLoadingDialog.kt (+25 -51)
📝 app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt (+54 -57)
📝 app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt (+12 -11)
📝 app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/enterprisesignon/EnterpriseSignOnScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/removepassword/RemovePasswordScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/trusteddevice/TrustedDeviceScreenTest.kt (+6 -4)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt (+3 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationScreenTest.kt (+3 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/exportvault/ExportVaultScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/folders/addedit/FolderAddEditScreenTest.kt (+4 -3)
📝 app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherScreenTest.kt (+4 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendScreenTest.kt (+3 -2)
📝 app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt (+3 -2)

...and 9 more files

📄 Description

🎟️ Tracking

📔 Objective

Update the loading indicator for the partial and full screen loading indicators. The partial loading indicators already existed but we just needed to increase the size to match the figma docs.
Went back and forth on different implementations for the full screen loading indicator. When using a true dialog, the screen would not be fully covered and I couldn't change the background color or opacity. I went down a rabbit hole updating the ViewState to use the BitwardenFullScreenLoadingContent but the DataState.Pending logic with ViewState.Content never allowed the full screen loading to appear. Found a much simplier solution with swapping out the Dialog for a Popup which allowed more customization while also leaving our root logic intact across the app.
I kept the dialog nomenclature but I understand that its a bit skewed now. I can update the naming to OverlayState for example but I wanted to keep the changes minimal but call it out if you feel strongly about it.

📸 Screenshots

Full Screen Load

Partial Loading

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

_{🔄 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/4581 **Author:** [@phil-livefront](https://github.com/phil-livefront) **Created:** 1/17/2025 **Status:** ✅ Merged **Merged:** 1/20/2025 **Merged by:** [@phil-livefront](https://github.com/phil-livefront) **Base:** `main` ← **Head:** `phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators` --- ### 📝 Commits (10+) - [`2a73506`](https://github.com/bitwarden/android/commit/2a73506ffce6a5276a37feb18682412f31c6c1b8) PM-16850-PM-16851-PM-16852 - update all dialog loading states to use full screen popup - [`e04c66b`](https://github.com/bitwarden/android/commit/e04c66be3ecc36614adc9693884e35475ea0aabd) fix linter warnings - [`ce12c77`](https://github.com/bitwarden/android/commit/ce12c77f09ed39c1ea3045f0254c2dd784dc76d6) fix up unit tests - [`aa5e290`](https://github.com/bitwarden/android/commit/aa5e2901b75e3eafa12b93dfe153b26eb7eca6fa) remove `BitwardenFullScreenLoadingContent` and refactor to allow the use of the loading popup - [`b7a6303`](https://github.com/bitwarden/android/commit/b7a6303058dccdd5ac3c4f138ef6cc2ee793086e) PR comments - [`49c2f7a`](https://github.com/bitwarden/android/commit/49c2f7adce668affbf14614809a3fbf596322790) lint fixes - [`bd3d623`](https://github.com/bitwarden/android/commit/bd3d6233897ff9e6d8d16bb98a28b398b5f806f0) PR comments - [`68e2264`](https://github.com/bitwarden/android/commit/68e22643dfba71a9a1bfb304d9ee5d6e5d520168) Merge branch 'main' into phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators - [`5d5f941`](https://github.com/bitwarden/android/commit/5d5f941b8229f135d45d354598fd8b4e48aaead8) fix tests to allow build to pass - [`8ee3e81`](https://github.com/bitwarden/android/commit/8ee3e816ef0495407b24716cffd5b9cfa956ae89) Merge branch 'main' into phil/PM-16850-PM-16851-PM-16852-Update-loading-indicators ### 📊 Changes **29 files changed** (+210 additions, -399 deletions) <details> <summary>View changed files</summary> ➖ `app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenFullScreenLoadingContent.kt` (+0 -43) 📝 `app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/BitwardenLoadingContent.kt` (+11 -4) ➖ `app/src/main/java/com/x8bit/bitwarden/ui/platform/components/content/ObscuredContent.kt` (+0 -158) 📝 `app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenLoadingDialog.kt` (+25 -51) 📝 `app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsScreen.kt` (+54 -57) 📝 `app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt` (+12 -11) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/enterprisesignon/EnterpriseSignOnScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/loginwithdevice/LoginWithDeviceScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/removepassword/RemovePasswordScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/trusteddevice/TrustedDeviceScreenTest.kt` (+6 -4) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt` (+3 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationScreenTest.kt` (+3 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/exportvault/ExportVaultScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/folders/addedit/FolderAddEditScreenTest.kt` (+4 -3) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherScreenTest.kt` (+4 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendScreenTest.kt` (+3 -2) 📝 `app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt` (+3 -2) _...and 9 more files_ </details> ### 📄 Description ## 🎟️ Tracking - [PM-16850](https://bitwarden.atlassian.net/browse/PM-16850) - [PM-16851](https://bitwarden.atlassian.net/browse/PM-16851) - [PM-16852](https://bitwarden.atlassian.net/browse/PM-16852) ## 📔 Objective - Update the loading indicator for the partial and full screen loading indicators. The partial loading indicators already existed but we just needed to increase the size to match the figma docs. - Went back and forth on different implementations for the full screen loading indicator. When using a true dialog, the screen would not be fully covered and I couldn't change the background color or opacity. I went down a rabbit hole updating the `ViewState` to use the `BitwardenFullScreenLoadingContent` but the `DataState.Pending` logic with `ViewState.Content` never allowed the full screen loading to appear. Found a much simplier solution with swapping out the `Dialog` for a `Popup` which allowed more customization while also leaving our root logic intact across the app. - I kept the dialog nomenclature but I understand that its a bit skewed now. I can update the naming to `OverlayState` for example but I wanted to keep the changes minimal but call it out if you feel strongly about it. ## 📸 Screenshots ### Full Screen Load ![syncing spinner](https://github.com/user-attachments/assets/0041f9c4-1d9b-4e92-ba7b-426542dbe039) ### Partial Loading #### VaultScreen [VaultScreen.webm](https://github.com/user-attachments/assets/8a69b2cc-2044-4ac7-bb86-544e0fe0b7c3) #### ViewItemScreen [ViewItemScreen.webm](https://github.com/user-attachments/assets/89ba048f-f872-4cd3-a9fa-960aff241fc2) #### SendScreen [SendScreen.webm](https://github.com/user-attachments/assets/db8b824c-c2af-4d3a-a8cc-06e674433d30) ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation or informed the documentation team ## 🦮 Reviewer guidelines  - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes [PM-16850]: https://bitwarden.atlassian.net/browse/PM-16850?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [PM-16851]: https://bitwarden.atlassian.net/browse/PM-16851?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [PM-16852]: https://bitwarden.atlassian.net/browse/PM-16852?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- <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:53:23 -06:00

GiteaMirror closed this issue