Skip to content

[pm-41938] fix: Bank Account accessibility fixes - #2985

Draft
morganzellers-bw wants to merge 7 commits into
pm-41934-bank-account-behavior-fixesfrom
pm-41938-voiceover-accessibility-fixes
Draft

[pm-41938] fix: Bank Account accessibility fixes#2985
morganzellers-bw wants to merge 7 commits into
pm-41934-bank-account-behavior-fixesfrom
pm-41938-voiceover-accessibility-fixes

Conversation

@morganzellers-bw

@morganzellers-bw morganzellers-bw commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

  • PM-41938 - View Bank Account screen show/hide toggle has a generic button announcement
  • PM-41939 - Add & Edit Screen: show/hide toggle name announcement is not custom by line item
  • PM-41940 - Entered characters are not announced individually
  • PM-41941 - Overflow menu is not announced and can't be accessed
  • PM-41942 - MP Re-prompt information icon is not announced and can't be accessed

📔 Objective

Adds a batch of accessibility bug fixes under the Bank Account item type

📸 Screenshots

Click to reveal the technical details

PM-41938

showHideToggle.mov

PM-41939

addEditShowHideToggle.mov

PM-41940

individualCharacters.mov

PM-41941

PM-41942

@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:bug Change Type - Bug labels Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 4.76190% with 20 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (pm-41934-bank-account-behavior-fixes@0c76039). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...latform/Application/Views/BitwardenTextField.swift 5.88% 16 Missing ⚠️
...rm/Application/Views/BitwardenTextValueField.swift 0.00% 4 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                           @@
##             pm-41934-bank-account-behavior-fixes    #2985   +/-   ##
=======================================================================
  Coverage                                        ?   39.68%           
=======================================================================
  Files                                           ?      362           
  Lines                                           ?    16989           
  Branches                                        ?        0           
=======================================================================
  Hits                                            ?     6742           
  Misses                                          ?    10247           
  Partials                                        ?        0           

☔ 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.

@morganzellers-bw morganzellers-bw self-assigned this Aug 20, 2026
@morganzellers-bw
morganzellers-bw force-pushed the pm-41938-voiceover-accessibility-fixes branch from 92f03fd to 15f4a0d Compare August 25, 2026 17:11
@morganzellers-bw morganzellers-bw added ai-review Request a Claude code review and removed app:authenticator Bitwarden Authenticator app context labels Aug 26, 2026
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed the head state of this stack of VoiceOver accessibility fixes for the Bank Account item type (PM-41938 through PM-41942), based on pm-41934-bank-account-behavior-fixes; no commits were added since the previous pass. The changes apply the previously-declared-but-unused accessibilityLabel inside PasswordVisibilityButton, thread an opt-in passwordVisibilityFieldName through every BitwardenTextField initializer, add spellOutAccessibilityValue to PasswordText and BitwardenTextValueField backed by speechSpellsOutCharacters, attach a named async accessibility action to the vault row overflow button, and move the master-password re-prompt info button into BitwardenToggle's pre-existing accessory slot. All new shared-component parameters default to prior behavior, no call site supplies a competing external .accessibilityLabel that the new internal one would shadow, and speechSpellsOutCharacters is available at the project's iOS 15.0 deployment target. The new FieldValueIs(Not)VisibleTapToShow/Hide keys use correct positional format specifiers and BitwardenResources/Generated/ holds no checked-in file needing regeneration.

Code Review Details

No blocking findings.

Considered and not raised: spellOutAccessibilityValue is silently ignored on the useUIKitTextView: true branch of BitwardenTextValueField, but no caller combines the two flags today; the new BankAccountItemStateTests.accountType_defaultLocalizedName_isBlank overlaps existing coverage in AddEditBankAccountItemStateTests but does assert a distinct property (that the state's default accountType is .default); the visibility-label ternary is repeated three times in ViewBankAccountItemView.swift, but the caller-supplied-label API predates this PR; and neither the combined-row accessibility action on VaultListItemRowView.swift:93 nor the speechSpellsOutCharacters announcements are inspectable by ViewInspector, which the test doc comments already call out as needing on-device VoiceOver confirmation.

PR Metadata Assessment

  • QUESTION: The PM-41940, PM-41941, and PM-41942 screenshot sections are empty. PM-41942 in particular relocates the re-prompt info icon out of the toggle label's HStack into BitwardenToggle's accessory slot, re-laying out that row — a before/after capture would confirm no visual regression.

@morganzellers-bw
morganzellers-bw force-pushed the pm-41938-voiceover-accessibility-fixes branch from 15f4a0d to a6074fb Compare August 26, 2026 16:58
@github-actions github-actions Bot added the app:authenticator Bitwarden Authenticator app context label Aug 26, 2026
@morganzellers-bw
morganzellers-bw force-pushed the pm-41938-voiceover-accessibility-fixes branch from a6074fb to b68caa1 Compare August 27, 2026 14:24
…unt view toggles

VoiceOver announced the Account Number, PIN, and IBAN reveal toggles on the
Bank Account view screen with a generic "button" label because
PasswordVisibilityButton accepted an accessibilityLabel but never applied it.
Wires up field-specific announcements (e.g. "Account number is not visible,
tap to show.") for each toggle.
…unt add/edit toggles

VoiceOver announced the Account Number, PIN, and IBAN reveal toggles on the
Bank Account add/edit screen with the same generic "Password is/is not
visible" wording regardless of which field was being toggled. Adds an
optional field-name parameter to BitwardenTextField so these three fields
announce their own name (e.g. "PIN is visible, tap to hide.") while every
other password-style field in the app keeps its existing generic wording.
…o VoiceOver

VoiceOver was reading the Account Number, Routing Number, Branch Number, PIN,
SWIFT Code, and IBAN values on the Bank Account view screen as whole numbers
or words instead of spelling out each character, since no explicit
accessibilityValue was set for these fields. Adds an opt-in
spellOutAccessibilityValue flag to PasswordText and BitwardenTextValueField,
backed by a new String.spellingOutCharacters() helper, and wires it up only
for the six affected Bank Account fields so other consumers of these shared
components are unaffected.
…ows to VoiceOver

The row's more options button was visually tappable but unreachable by
VoiceOver because the row combines its content into a single
accessibility element, which swallows the nested button's own
announcement and activation. Adds a named accessibility action so the
overflow menu remains announced and accessible, matching the pattern
already used for PasswordHistoryListView's combined row.
…o icon to VoiceOver

The info button was nested inside BitwardenToggle's title content, so
VoiceOver collapsed the whole row into one element and swallowed the
button's own label and tap target. Move it into BitwardenToggle's
accessory slot, which keeps it independently reachable, and add an
"External link" hint, mirroring the Fill Assist toggle fix (PM-41097).
.accessibilityValue(value.spellingOutCharacters()) was being added on
top of a Text whose own content is already exposed as its
accessibility label, so VoiceOver would likely announce the value
twice: once normally, once spelled out. Use speechSpellsOutCharacters
instead, which tells VoiceOver to spell out the label itself rather
than adding a redundant value. Drops the now-unused
String.spellingOutCharacters() helper.

Also replaces a hand-rolled Task { await ... } in an
accessibilityAction with the existing accessibilityAsyncAction helper
for consistency.

ViewInspector can't inspect speechSpellsOutCharacters directly, so the
affected tests now verify the flag is passed through to PasswordText/
BitwardenTextValueField instead of the rendered accessibility value;
the actual VoiceOver announcement should be confirmed on-device.
The Add/Edit Bank Account screen's Account Type menu defaulted to
displaying "Checking" even when the user had never opened the picker,
because DefaultableType<BankAccountType>'s default-case label reused
the "Checking" option name instead of a blank placeholder.
@morganzellers-bw
morganzellers-bw force-pushed the pm-41938-voiceover-accessibility-fixes branch from b68caa1 to 04c13c5 Compare August 28, 2026 15:45
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 t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant