Skip to content

[PM-38360] feat: Add reusable DateFieldPicker component#2731

Draft
SaintPatrck wants to merge 7 commits into
mainfrom
vault/pm-38360-ios-date-field-picker
Draft

[PM-38360] feat: Add reusable DateFieldPicker component#2731
SaintPatrck wants to merge 7 commits into
mainfrom
vault/pm-38360-ios-date-field-picker

Conversation

@SaintPatrck

@SaintPatrck SaintPatrck commented May 29, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

  • Jira: PM-38360

📔 Objective

Adds a reusable DateFieldPicker SwiftUI component to BitwardenKit for optional calendar-date entry. The field renders as a collapsed row (title + chevron); a tap reveals a native inline graphical calendar (DatePicker with .datePickerStyle(.graphical)) directly beneath the row, and a Clear control resets the selection back to empty. When no date is set, the field shows only its title — matching the design comps.

The calendar and its reveal animation are entirely native. The thin wrapper exists only to add an optional Date? (empty state + Clear), which a native DatePicker cannot represent on its own, inside the Add/Edit forms' ScrollView — where the native .compact inline-expansion style isn't available. This mirrors the existing BitwardenMenuField precedent of wrapping a native control for that context.

Built once for its consumers, this unblocks date fields for the new item types: Driver's License (date of birth, issue date, expiration) and Passport (date of birth, issue date, expiration).

Also adds an ISO 8601 calendar-date (yyyy-MM-dd) conversion helper on Date for round-tripping the string-backed date fields, which rejects invalid calendar dates (e.g. Feb 30). The Clear control uses a new 24pt circle-x24 icon asset per Figma.

Note — date handling is still in flux: the Bitwarden SDK is expected to change these fields from String? to a strongly-typed Date (bitwarden/sdk-internal#1154). Once it does, the ISO 8601 string conversion and the invalid-date handling here become unnecessary and can be removed.

Tests: 7 DateFieldPicker ViewInspector tests + 6 Date ISO 8601 unit tests (all passing); snapshot tests added, disabled per repo convention.

📸 Screenshots

Collapsed

Figma Actual

Expanded

Figma Actual

@SaintPatrck SaintPatrck added the ai-review-vnext Request a Claude code review using the vNext workflow label May 29, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature labels May 29, 2026
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new reusable DateFieldPicker BitwardenKit component, the Date ISO 8601 calendar-date conversion helpers, the circle-x24 icon asset, the localization additions, and the Test Harness showcase wiring. The component follows the established BitwardenMenuField precedent of wrapping a native control to support an optional Date?, uses AccessoryButton correctly for the Clear control, is well documented, and ships with focused ViewInspector and unit tests (including invalid/malformed date input) plus disabled snapshot tests per repo convention.

Code Review Details

No new blocking findings. The code is well-structured and tested.

Several areas are already under active discussion in existing review threads and were not duplicated here: VoiceOver navigability of the .graphical picker and the wheel-style fallback, the Clear-label and select-date hint wording, and how iso8601DateOnlyFormatter relates to the built-in ISO8601DateFormatter.

One latent consideration for the eventual consumers (not blocking — the PR description already flags date handling as in flux pending the SDK change): the iso8601DateOnlyString / init(iso8601DateOnlyString:) helpers use a UTC formatter, while a .date DatePicker produces a Date at the start of the selected day in the device's local time zone. Round-tripping a string through these helpers is stable, but converting a picker-produced local-time date to the UTC date-only string could shift the day for users west of UTC. Worth confirming when the helpers are wired up to the picker.

@codecov

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.23944% with 76 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.96%. Comparing base (8c9d9eb) to head (a8048db).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...lication/Views/DateFieldPicker+SnapshotTests.swift 0.00% 39 Missing ⚠️
...I/Platform/Application/Views/DateFieldPicker.swift 72.38% 37 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2731      +/-   ##
==========================================
- Coverage   87.93%   86.96%   -0.97%     
==========================================
  Files        1728     1919     +191     
  Lines      168727   182468   +13741     
==========================================
+ Hits       148365   158688   +10323     
- Misses      20362    23780    +3418     

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

/// concurrency domains. Uses the POSIX locale and UTC time zone so output and parsing are
/// deterministic regardless of the device locale, and disables lenient parsing so invalid dates
/// such as `2023-02-30` fail to parse rather than rolling over.
static func iso8601DateOnlyFormatter() -> DateFormatter {

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.

🤔 How is this different from the built-in ISO8601DateFormatter?

@SaintPatrck

Copy link
Copy Markdown
Contributor Author

@RishikaSG-28 Figma didn't explicitly state which picker should be used for the dates, but I'm assuming this barrel spinner is expected since it's date only. I noticed there is no way to "Clear" a selected date so I've added a small "Clear" button. Can you take a look at the screenshot and let me know what you think.

@bitwarden/team-ios @matt-livefront I'm not super familiar with the iOS native date picker. Is the the right approach to add a "Clear" button, or am I missing something that would handle clearing natively?

@SaintPatrck SaintPatrck requested a review from RishikaSG-28 June 1, 2026 14:58
@KatherineInCode

Copy link
Copy Markdown
Contributor

@SaintPatrck Having it be a popup rather than a pseudo-keyboard or inline feels weird to me. The Contacts app still does a pseudo-keyboard; Reminders and Calendar put the date selector inline with a calendar; what are some Apple apps that do the popup (as it's possible I'm just not thinking of any offhand that do)?

@RishikaSG-28

Copy link
Copy Markdown

@SaintPatrck, Emily added the date picker when she designed new item types. I've tagged you in it, please have a look.

@SaintPatrck

SaintPatrck commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@KatherineInCode @RishikaSG-28 I've updated the picker based on our conversation. LMK what you think about the updated screenshots.

@fedemkr fedemkr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, just a few a11y concerns.

Comment on lines +153 to +159
Button {
toggleExpanded()
} label: {
labelContent()
}
.buttonStyle(.plain)
.accessibilityIdentifier("DateFieldHeaderButton")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 Does the user understand that they need to tap to select a different date from VoiceOver?

if date != nil {
AccessoryButton(
asset: SharedAsset.Icons.circleX24,
accessibilityLabel: Localizations.clear,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⛏️ I believe this should say "Clear {title of the header}" as in VoiceOver the user may not know if that clear is for the date or for something that follow this field.

Comment on lines +106 to +117
@ViewBuilder
private func datePicker() -> some View {
if let range {
DatePicker("", selection: selection(), in: range, displayedComponents: [.date])
.labelsHidden()
.datePickerStyle(.graphical)
} else {
DatePicker("", selection: selection(), displayedComponents: [.date])
.labelsHidden()
.datePickerStyle(.graphical)
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚠️ :accessibility: When in VoiceOver, .graphical style is really problematic as it's very hard to navigate. I think we should hide graphical pickers from accessibility and provide wheel style for such case which is easier for VoiceOver users to use.

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.

@maxkpower @RishikaSG-28 thoughts on this? Makes sense, imo.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That makes sense! If it's doable to add that condition then let's go for it!

@SaintPatrck SaintPatrck force-pushed the vault/pm-38360-ios-date-field-picker branch from 65dc4d0 to 641453b Compare June 9, 2026 14:21
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Warning

@SaintPatrck Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Warning

@SaintPatrck Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details.

Add a reusable SwiftUI `DateFieldPicker` to BitwardenKit for optional date
(and optional time) entry. The field renders as a collapsed disclosure row
that expands an inline native graphical `DatePicker` on a single tap, and
supports clearing back to no selection. This unblocks date fields for the
new item types (Driver's License expiration; Passport date of birth, issue,
and expiration dates).

Also add an ISO 8601 calendar-date (`yyyy-MM-dd`) conversion helper on `Date`
for round-tripping the string-backed date fields, rejecting invalid dates.
Tapping the field now presents the native graphical DatePicker in a popover
dialog (forced via presentationCompactAdaptation on iOS 16.4+) instead of
expanding inline, matching the design.
- Switch the popover dialog to the native wheel date picker style
- Remove the unused time (displayedComponents) support — all consumers are
  date-only (YAGNI)
- Bound the wheel height and fix the dialog sizing so the Clear button is no
  longer clipped against the popover edge
Name the clear control after its field, add a header hint, and switch to
a wheel picker under VoiceOver where the graphical calendar is hard to
navigate (keeping it expanded so continuous scrubbing does not dismiss it).
@SaintPatrck SaintPatrck force-pushed the vault/pm-38360-ios-date-field-picker branch from 641453b to a8048db Compare June 15, 2026 14:08
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.

4 participants