Skip to content

[PM-33981] feat: Add device management UI and Account Security integration#2871

Open
andrebispo5 wants to merge 6 commits into
pm-33981/device-table-corefrom
pm-33981/device-table-ui
Open

[PM-33981] feat: Add device management UI and Account Security integration#2871
andrebispo5 wants to merge 6 commits into
pm-33981/device-table-corefrom
pm-33981/device-table-ui

Conversation

@andrebispo5

@andrebispo5 andrebispo5 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-33981

📔 Objective

⛓️ Depends on #2870 (PM-33981 Core models and API layer) — merge that PR first.

Adds the complete device management UI on top of the Core layer from the preceding PR.

  • Adds PillBadgeStyle.info (blue palette) to PillBadgeView for the Current Session badge
  • Adds the DeviceManagementView (list with pull-to-refresh and empty state), DeviceRow (pill badge at top, Trusted label, Recently active and First login info rows, full-row tap), and DeviceManagementProcessor (parallel device + pending-request loading, calendar-day activity buckets, pendingRequestMatchKey-based request matching)
  • Wires .deviceManagement route in SettingsRoute and SettingsCoordinator
  • Adds manageDevices feature flag; gates a "Devices" row in the Account Security Other section behind it; reorders the section: Account fingerprint phrase → Two-step login → Devices → Lock now → Log out → Delete account
  • Adds DeviceManagementProcessorTests (Swift Testing) and DeviceManagementViewTests (ViewInspector); adds AccountSecurityProcessorTests coverage for the new action and flag load

📸 Screenshots

image image image

…ation

- Add PillBadgeStyle.info (blue: #EEF6FF bg / #BEDBFF border / #1A41AC
  text, dark: #162455 bg / #1A41AC border / #DBE5F6 text) for the
  Current Session badge in DeviceRow
- Add manageDevices feature flag; gate the Devices row in Account Security
  behind it; reorder Other section: Fingerprint → Two-step → Devices →
  Lock now → Log out → Delete account
- Add DeviceManagementView (list with pull-to-refresh), DeviceRow (pill
  badge at top, Trusted label, Recently active and First login rows,
  ContentBlock-managed dividers and background, full-row tap via Button),
  and DeviceManagementProcessor (parallel loading, calendar-day activity
  status, pendingRequestMatchKey-based request matching)
- Wire .deviceManagement route in SettingsRoute and SettingsCoordinator
- Add DeviceManagementProcessorTests (Swift Testing) and
  DeviceManagementViewTests (ViewInspector)
- Add AccountSecurityProcessor tests for manageDevicesTapped navigation
  and isManageDevicesEnabled feature-flag load
Copilot AI review requested due to automatic review settings July 9, 2026 09:06
@andrebispo5 andrebispo5 requested review from a team and matt-livefront as code owners July 9, 2026 09:06
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature labels Jul 9, 2026
@andrebispo5 andrebispo5 added ai-review Request a Claude code review innovation Feature work related to Innovation Sprint or BEEEP projects labels Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the device management UI layer stacked on the Core PR: DeviceManagementView/DeviceRow/DeviceManagementProcessor, the manageDevices feature-flag gating and reordering in Account Security, the .deviceManagement route wiring in SettingsRoute/SettingsCoordinator, the new PillBadgeStyle.info and badge color assets, and the accompanying processor/view tests. The unidirectional data-flow, coordinator navigation, and dependency-injection patterns are followed correctly, and the feature flag defaults to false so the change is safely gated. All prior Copilot review points (FeatureFlag allCases, ViewInspector import, non-actionable row traits) have been addressed in the latest commits.

Code Review Details

No blocking findings.

Notes considered and intentionally not flagged:

  • The matchPendingRequestsToDevices best-effort matching (browser vs. extension sharing one platform key, first match wins) is documented inline and confirmed as intentional given the server sends the same requestDeviceType for both variants.
  • On load failure loadData sets .data([]) and surfaces an error alert, briefly showing the empty state behind the alert — an acceptable UX tradeoff, not a correctness issue.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds the Device Management UI flow (gated behind a feature flag) and integrates it into Account Security/Settings navigation, including a new “info” pill badge style for highlighting the current session.

Changes:

  • Introduces the Device Management screen (view + row UI) and a processor that loads devices + pending login requests in parallel and matches requests to devices.
  • Wires a new .deviceManagement route through Settings and adds a feature-flagged “Devices” entry in Account Security.
  • Adds UI assets/localizations for the new screen and badge style, plus new unit/UI tests.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
BitwardenShared/UI/Platform/Settings/SettingsRoute.swift Adds a new route case for device management navigation.
BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift Wires the new route and presents DeviceManagementView; adds required services.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceRow.swift New device list row UI (badges, trusted label, activity/login info).
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementView+ViewInspectorTests.swift New ViewInspector UI tests for the Device Management view.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementView.swift New screen: loading/empty/list states, refresh, toast, and navigation chrome.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementState.swift Adds state for device loading and toast presentation.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementProcessorTests.swift Adds processor tests for load/sort/matching behavior and navigation.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementProcessor.swift Implements parallel loading, pending-request matching, sorting, navigation, and toast behavior.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementEffect.swift Adds the effect(s) used by the new processor (loadData).
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementAction.swift Adds actions for device tap/dismiss/toast state updates.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityView.swift Adds a feature-flagged “Devices” row and hides the old pending-requests section when enabled.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityState.swift Adds isManageDevicesEnabled flag to drive UI gating.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityProcessorTests.swift Adds coverage for the new navigation action and feature flag loading.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityProcessor.swift Loads the manage-devices feature flag and navigates to device management on tap.
BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityAction.swift Adds .manageDevicesTapped action.
BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift Adds FeatureFlag.manageDevices.
BitwardenResources/Localizations/en.lproj/Localizable.strings Adds strings for device management UI labels/badges.
BitwardenResources/Colors.xcassets/Badge/badgeInfoText.colorset/Contents.json Adds color asset for info badge text.
BitwardenResources/Colors.xcassets/Badge/badgeInfoBorder.colorset/Contents.json Adds color asset for info badge border.
BitwardenResources/Colors.xcassets/Badge/badgeInfoBackground.colorset/Contents.json Adds color asset for info badge background.
BitwardenKit/UI/Platform/Application/Views/PillBadgeView.swift Adds .info badge style using the new color assets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift
…l badges

Info border dark:      #BEDBFF → #1A41AC
Warning background dark: #FFF8F1 → #441600
Warning border dark:   #FCD9BD → #8A2203
Warning text dark:     #B23300 → #FCD9BD
Success background dark: #F0FDF4 → #032E15
Success border dark:     #B9F8CF → #016630
Success text dark:       #08540F → #B9F8CF
Danger background dark:  #FEF2F2 → #460809
Danger border dark:      #FFC9C9 → #9F0712
Danger text dark:        #951B2A → #FFC9C9
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.86348% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.01%. Comparing base (94237b4) to head (e811124).
⚠️ Report is 1 commits behind head on pm-33981/device-table-core.

Files with missing lines Patch % Lines
...curity/DeviceManagement/DeviceManagementView.swift 64.81% 19 Missing ⚠️
...y/DeviceManagement/DeviceManagementProcessor.swift 88.39% 13 Missing ⚠️
...red/UI/Platform/Settings/SettingsCoordinator.swift 10.00% 9 Missing ⚠️
...Settings/AccountSecurity/AccountSecurityView.swift 33.33% 8 Missing ⚠️
...s/AccountSecurity/DeviceManagement/DeviceRow.swift 93.47% 6 Missing ⚠️
.../UI/Platform/Application/Views/PillBadgeView.swift 50.00% 3 Missing ⚠️
...Shared/Core/Platform/Models/Enum/FeatureFlag.swift 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           pm-33981/device-table-core    #2871       +/-   ##
===============================================================
+ Coverage                       40.58%   79.01%   +38.43%     
===============================================================
  Files                             358     1160      +802     
  Lines                           16665    73878    +57213     
===============================================================
+ Hits                             6763    58373    +51610     
- Misses                           9902    15505     +5603     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- DeviceRow: only wrap in Button when hasPendingRequest; non-actionable
  rows are now plain non-interactive views with no button trait
- FeatureFlag: add manageDevices to allCases (alphabetically between
  fillAssistTargetingRules and migrateMyVaultToMyItems)
- DeviceManagementView+ViewInspectorTests: add import ViewInspector

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Comment on lines +161 to +170
if let index = updatedDevices.firstIndex(where: { device in
!device.deviceType.pendingRequestMatchKey.isEmpty &&
device.deviceType.pendingRequestMatchKey.lowercased()
== request.requestDeviceType.lowercased()
}) {
// Only set if no pending request has been set yet (keeps the most recent).
if updatedDevices[index].pendingRequest == nil {
updatedDevices[index].pendingRequest = request
}
}
…uestDeviceType for browser and extension

The server sets requestDeviceType to the platform display name ('Chrome',
'Safari', etc.) for all device variants including extensions. The
pendingRequestMatchKey property returned 'Chrome Extension' for
chromeExtension, which never matched the server-sent 'Chrome', causing
extension devices to never receive a pending request badge.

Revert to matching on platform directly; when a user has both a Chrome
browser and a Chrome extension active the match is best-effort (first
in the list wins), documented in an inline comment.
Comment on lines -4 to +10
"color" : {
"color-space" : "srgb",
"components" : {
"alpha" : "1.000",
"blue" : "0xF2",
"green" : "0xF2",
"red" : "0xFE"
"color": {
"color-space": "srgb",
"components": {
"alpha": "1.000",
"blue": "0xF2",
"green": "0xF2",
"red": "0xFE"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓ Any idea why this format changed? Is this Xcode changing this?

Comment on lines +49 to +59
VStack(spacing: 20) {
Image(decorative: Asset.Images.Illustrations.devices)
.resizable()
.frame(width: 100, height: 100)

Text(Localizations.noDevicesFound)
.styleGuide(.body)
.foregroundStyle(SharedAsset.Colors.textPrimary.swiftUIColor)
.multilineTextAlignment(.center)
}
.frame(maxWidth: .infinity)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🎨 Could we use IllustratedMessageView for displaying the empty state to match other views?

/// - Parameter devices: The devices to display.
///
private func devicesList(_ devices: [DeviceListItem]) -> some View {
VStack(spacing: 24) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓ Is the VStack here necessary given that it has only one child (ContentBlock)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context innovation Feature work related to Innovation Sprint or BEEEP projects t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants