Skip to content

[PM-11610] feat: Change TOTP countdown circle color to urgent when ≤ 7 seconds remain#2579

Merged
KatherineInCode merged 18 commits into
mainfrom
pm-11610/totp-countdown-color
Apr 30, 2026
Merged

[PM-11610] feat: Change TOTP countdown circle color to urgent when ≤ 7 seconds remain#2579
KatherineInCode merged 18 commits into
mainfrom
pm-11610/totp-countdown-color

Conversation

@KatherineInCode
Copy link
Copy Markdown
Contributor

@KatherineInCode KatherineInCode commented Apr 24, 2026

🎟️ Tracking

PM-11610

📔 Objective

Consolidates duplicated TOTP types (TOTPCodeModel, TOTPExpirationCalculator, TOTPCountdownTimer, TOTPCountdownTimerView) from BitwardenShared and AuthenticatorShared into BitwardenKit, then implements the PM-11610 color change: the countdown circle turns red (SharedAsset.Colors.danger) when ≤ Constants.totpUrgentCountdownThreshold (7) seconds remain, and shows the primary blue (SharedAsset.Colors.tintPrimary) otherwise. This also fixes the Authenticator's outdated timer color palette and a pre-existing trailing-space bug in TOTPCodeModel.displayCode.

📷 Screenshots

Simulator Screenshot - iPhone 17 Pro - 2026-04-28 at 10 27 23 Simulator Screenshot - iPhone 17 Pro - 2026-04-28 at 12 08 23

@KatherineInCode KatherineInCode added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 24, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature labels Apr 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.09%. Comparing base (2373dce) to head (95b890d).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2579      +/-   ##
==========================================
- Coverage   87.17%   86.09%   -1.09%     
==========================================
  Files        1886     2114     +228     
  Lines      166863   181482   +14619     
==========================================
+ Hits       145463   156242   +10779     
- Misses      21400    25240    +3840     

☔ View full report in Codecov by Sentry.
📢 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.

@github-actions github-actions Bot added the t:llm Change Type - LLM related change (e.g. CLAUDE.md files) label Apr 24, 2026
…7 seconds remain

Consolidates TOTPCodeModel, TOTPExpirationCalculator, TOTPCountdownTimer,
and TOTPCountdownTimerView from BitwardenShared and AuthenticatorShared into
BitwardenKit, eliminating duplication and fixing the Authenticator's outdated
color palette. Adds timerColor() to switch the circle from tintPrimary to
error color at the Constants.totpUrgentCountdownThreshold (7 second) boundary.
Also fixes a pre-existing trailing-space bug in TOTPCodeModel.displayCode.
@KatherineInCode KatherineInCode force-pushed the pm-11610/totp-countdown-color branch from 09f51e3 to 73f71b4 Compare April 24, 2026 21:08
@github-actions github-actions Bot removed the t:llm Change Type - LLM related change (e.g. CLAUDE.md files) label Apr 24, 2026
@KatherineInCode
Copy link
Copy Markdown
Contributor Author

This was the original Claude plan:
PM-11610-plan.md

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR consolidates the duplicated TOTPCodeModel, TOTPExpirationCalculator, TOTPCountdownTimer, and TOTPCountdownTimerView from BitwardenShared and AuthenticatorShared into BitwardenKit, then implements the urgent-countdown color change at the shared Constants.totpUrgentCountdownThreshold (7s). It also fixes a pre-existing retain cycle in TOTPCountdownTimer ([weak self] in the timer block), corrects the dark-mode value of the new danger color asset, and resolves a trailing-space bug in TOTPCodeModel.displayCode whose snapshot fallout in VaultRepositoryTests is included. Tests are comprehensive: boundary cases at, above, and below the threshold; a dedicated retain-cycle test; and the existing expiration test migrated from Task.sleep/waitFor to withContinuationTimeout. All call sites in the password-manager and authenticator views compile against the new public API.

Code Review Details

No actionable findings.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

Logo
Checkmarx One – Scan Summary & Detailscd40204f-0232-47d5-ab2f-ece79384dee4

Great job! No new security vulnerabilities introduced in this pull request

Comment thread BitwardenKit/UI/Vault/Views/TOTPCountdownTimerTests.swift Outdated
@KatherineInCode KatherineInCode marked this pull request as ready for review April 29, 2026 13:42
@KatherineInCode KatherineInCode requested review from a team and matt-livefront as code owners April 29, 2026 13:42
repeats: true,
block: { _ in
self.updateCountdown()
block: { [weak self] _ in
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.

👍 Good catch.

onExpiration: { resume() },
)
}
_ = subject
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 this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without it, the compiler complains about writing to subject without ever reading it, though the test still passes. Claude also generated an explanation that the line also prevents an aggressively optimizing compiler from deallocating the now-weak object before resume is called, so keeping it prevents that potentiality in the future.

I'm so-so on that, because the test passes with out it, but I think I'd rather keep it to avoid the warning.

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.

Ah, makes sense!

@KatherineInCode KatherineInCode merged commit 3f0cc8b into main Apr 30, 2026
41 checks passed
@KatherineInCode KatherineInCode deleted the pm-11610/totp-countdown-color branch April 30, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants