Skip to content

[PM-38507] feat: Add Binding<Bool> initializer to ExpandableHeaderView#2583

Merged
SaintPatrck merged 2 commits into
mainfrom
vault/pm-35398-ios-expandable-header-binding-init
Jun 3, 2026
Merged

[PM-38507] feat: Add Binding<Bool> initializer to ExpandableHeaderView#2583
SaintPatrck merged 2 commits into
mainfrom
vault/pm-35398-ios-expandable-header-binding-init

Conversation

@SaintPatrck

@SaintPatrck SaintPatrck commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-38507

📔 Objective

ExpandableHeaderView stores its expand/collapse state in an internal @State, so the preference resets whenever the view is recreated. The upcoming BWPM vault-list section headers need that preference to survive app relaunch, which requires caller-owned storage.

This adds a second initializer accepting isExpanded: Binding<Bool> so callers can drive expansion from their own store. The existing (title:count:content:) initializer is unchanged, so Authenticator's call site is untouched.

@SaintPatrck SaintPatrck added t:feature-app Change Type - Product feature or enhancement app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context ai-review-vnext Request a Claude code review using the vNext workflow labels Apr 24, 2026
@github-actions github-actions Bot added t:feature and removed t:feature-app Change Type - Product feature or enhancement labels Apr 24, 2026
@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.65%. Comparing base (0a5ea91) to head (6149c22).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2583      +/-   ##
==========================================
- Coverage   87.22%   86.65%   -0.57%     
==========================================
  Files        1919     2137     +218     
  Lines      173520   187486   +13966     
==========================================
+ Hits       151346   162460   +11114     
- Misses      22174    25026    +2852     

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

@SaintPatrck
SaintPatrck force-pushed the vault/pm-35398-ios-promote-expandable-header branch from f9e4cd5 to a51442e Compare May 29, 2026 20:06
@SaintPatrck
SaintPatrck force-pushed the vault/pm-35398-ios-expandable-header-binding-init branch from 7132959 to 4dad4b4 Compare May 29, 2026 20:06
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the additive change to ExpandableHeaderView that introduces a second initializer accepting isExpanded: Binding<Bool> for caller-owned expansion state. The original (title:count:content:) initializer's behavior is preserved, the Authenticator call site at AuthenticatorShared/UI/Vault/ItemList/ItemList/ItemListView.swift:279 is untouched, and the new internalIsExpanded / externalIsExpanded dual-storage pattern with a unified isExpanded: Binding<Bool> computed property is a sound SwiftUI idiom. Doc comments explicitly call out the runtime contract (commit to one initializer variant per call site) and the default-expanded behavior, and ViewInspector tests cover initial-state reflection, write-back, and a multi-toggle round trip that guards against future regressions where the binding could stop propagating after the first write.

Code Review Details

No findings.

@SaintPatrck
SaintPatrck force-pushed the vault/pm-35398-ios-expandable-header-binding-init branch from 4dad4b4 to 3f4713a Compare May 29, 2026 20:18
Base automatically changed from vault/pm-35398-ios-promote-expandable-header to main June 3, 2026 13:30
@SaintPatrck SaintPatrck changed the title [PM-35398] feat: Add Binding<Bool> initializer to ExpandableHeaderView [PM-38507] feat: Add Binding<Bool> initializer to ExpandableHeaderView Jun 3, 2026
Adds an additive `isExpanded: Binding<Bool>` initializer to
`ExpandableHeaderView` so callers whose expand/collapse preference must
survive view recreation (e.g., persisted across app launches in PR 6c)
can drive the view from caller-owned storage. The original
zero-ceremony `ExpandableHeaderView(title:count:content:)` initializer
is preserved unchanged, so Authenticator's existing `ItemListView` call
site compiles without modification.

Chose option (c) from the brief's "cleanest approach" guidance: dual
storage on a single type. The struct carries both an `@State private
var internalIsExpanded` (used when no binding is supplied) and an
`externalIsExpanded: Binding<Bool>?` (set by the binding init). A
computed `isExpanded: Binding<Bool>` resolves to whichever storage the
initializer populated. This:

- Avoids the brittle `State(initialValue:).projectedValue` assign-in-
  init trick, which loses SwiftUI's state-tracking cell.
- Avoids a public wrapper-view shim (approach a), which would expose
  additional API surface and leak an implementation type into call
  sites.
- Keeps `body` on a single `@Binding` read path.
- Preserves the `ExpandableHeaderView<Content>` identity for both
  initializers — no subtype or factory-function redirection needed.

Authenticator's call site in
`AuthenticatorShared/UI/Vault/ItemList/ItemList/ItemListView.swift:275`
remains unchanged and still resolves to the no-binding initializer.

Adds `ExpandableHeaderView+ViewInspectorTests.swift` covering:

- `test_init_withoutBinding_startsExpanded` — no-binding init renders
  child content on first render (expansion state defaults to true).
- `test_init_withBinding_reflectsBindingState` — binding init reads the
  initial value from the caller's binding (true ⇒ content visible,
  false ⇒ content hidden).
- `test_init_withBinding_updatesThroughBinding` — tapping the header
  writes the toggled value back through the caller-supplied binding.

All three tests pass against BitwardenKit on iPhone 17 / iOS 26.1.
…nd-trip test)

Applies four follow-up fixes from the PR 6b architecture and code reviews.

- I-1 (arch): Document the SwiftUI identity invariant on both initializers.
  A given call site must commit to one initializer variant; switching
  between them at runtime is undefined because SwiftUI preserves the
  view's identity across renders and reuses the internal `@State` cell,
  silently abandoning whichever storage is no longer populated.

- I-3 (arch): Name the default expansion state (`true`) as part of the
  no-binding initializer's public contract, and redirect callers who
  need "collapsed by default" behavior to the binding initializer with
  a pre-initialized `Binding<Bool>(false)`.

- ⚠️1 (code): Add a runtime round-trip test that exercises the header
  button tap repeatedly and asserts each tap writes back through the
  caller's binding. The existing single-tap test was already mediated
  by `ViewType.Button.tap()` (which invokes the action closure —
  including the `withAnimation` wrapper — through ViewInspector rather
  than calling the closure directly), so that test is kept; the new
  `test_init_withBinding_roundTripsAcrossMultipleToggles` guards
  against regressions where the `withAnimation` write stops propagating
  on subsequent toggles (e.g. a future refactor that captures
  `externalIsExpanded` by value).

- ⚠️2 (code): Remove the stale unused `@Previewable @swiftui.State var
  isExpanded = false` from the preview and split the preview into two
  canvases — "Internal state" exercising the no-binding init,
  "Caller-owned binding" exercising the new `Binding<Bool>` init with
  the previously-dead state variable now wired through the binding.

All four tests pass on iPhone 17 / iOS 26.1. `AuthenticatorShared`
still builds unchanged.
@SaintPatrck
SaintPatrck force-pushed the vault/pm-35398-ios-expandable-header-binding-init branch from 3f4713a to 6149c22 Compare June 3, 2026 13:38
@SaintPatrck
SaintPatrck marked this pull request as ready for review June 3, 2026 14:05
@SaintPatrck
SaintPatrck requested review from a team and matt-livefront as code owners June 3, 2026 14:05
@SaintPatrck
SaintPatrck merged commit a1c2648 into main Jun 3, 2026
25 checks passed
@SaintPatrck
SaintPatrck deleted the vault/pm-35398-ios-expandable-header-binding-init branch June 3, 2026 15:44
@SaintPatrck

Copy link
Copy Markdown
Contributor Author

Thanks @morganzellers-bw

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