diff --git a/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView+ViewInspectorTests.swift b/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView+ViewInspectorTests.swift new file mode 100644 index 0000000000..feae5306a4 --- /dev/null +++ b/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView+ViewInspectorTests.swift @@ -0,0 +1,98 @@ +// swiftlint:disable:this file_name +import SwiftUI +import ViewInspector +import ViewInspectorTestHelpers +import XCTest + +@testable import BitwardenKit + +final class ExpandableHeaderViewTests: BitwardenTestCase { + // MARK: Tests + + /// `init(title:count:buttonAccessibilityIdentifier:content:)` starts in the expanded state. + @MainActor + func test_init_withoutBinding_startsExpanded() throws { + let subject = ExpandableHeaderView(title: "Title", count: 3) { + Text("Child") + } + + // The child content is rendered only when `isExpanded` is true, so its presence is a + // proxy for the initial expansion state. + XCTAssertNoThrow(try subject.inspect().find(text: "Child")) + } + + /// `init(title:count:buttonAccessibilityIdentifier:isExpanded:content:)` reflects the caller's + /// binding value on the first render. + @MainActor + func test_init_withBinding_reflectsBindingState() throws { + let collapsedSubject = ExpandableHeaderView( + title: "Title", + count: 3, + isExpanded: .constant(false), + ) { + Text("Hidden child") + } + XCTAssertThrowsError(try collapsedSubject.inspect().find(text: "Hidden child")) + + let expandedSubject = ExpandableHeaderView( + title: "Title", + count: 3, + isExpanded: .constant(true), + ) { + Text("Visible child") + } + XCTAssertNoThrow(try expandedSubject.inspect().find(text: "Visible child")) + } + + /// Tapping the header button with an external binding writes the toggled value back through + /// that binding so the caller's persisted storage is kept in sync. + @MainActor + func test_init_withBinding_updatesThroughBinding() throws { + var isExpanded = true + let binding = Binding(get: { isExpanded }, set: { isExpanded = $0 }) + let subject = ExpandableHeaderView( + title: "Title", + count: 3, + isExpanded: binding, + ) { + Text("Child") + } + + let button = try subject.inspect().find(ViewType.Button.self) + try button.tap() + + XCTAssertFalse( + isExpanded, + "Tapping the header should write the toggled value through the caller-supplied binding", + ) + } + + /// Exercising the header repeatedly must keep the caller's binding coherent across toggles — + /// i.e. the computed `isExpanded: Binding` must continue to resolve to the + /// `externalIsExpanded` storage on every render, including renders that observe a previously + /// written value. This guards against regressions where the `withAnimation`-wrapped write + /// stops propagating after the first toggle (e.g. a future refactor to two disjoint storages + /// that accidentally captures `externalIsExpanded` by value). + @MainActor + func test_init_withBinding_roundTripsAcrossMultipleToggles() throws { + var isExpanded = true + let binding = Binding(get: { isExpanded }, set: { isExpanded = $0 }) + let subject = ExpandableHeaderView( + title: "Title", + count: 3, + isExpanded: binding, + ) { + Text("Child") + } + + for expectedAfterTap in [false, true, false] { + let button = try subject.inspect().find(ViewType.Button.self) + try button.tap() + XCTAssertEqual( + isExpanded, + expectedAfterTap, + "Each tap must write back through the caller-supplied binding; expected \(expectedAfterTap)", + ) + } + } +} diff --git a/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView.swift b/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView.swift index e9f8d5955f..0d096222ae 100644 --- a/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView.swift +++ b/BitwardenKit/UI/Platform/Application/Views/ExpandableHeaderView.swift @@ -5,6 +5,12 @@ import SwiftUI /// A wrapper around some content which can be expanded to show the content or collapsed to hide it. /// +/// Use ``init(title:count:buttonAccessibilityIdentifier:content:)`` when the view can own its +/// own expansion state (a fresh `@State` that resets on view recreation). Use +/// ``init(title:count:buttonAccessibilityIdentifier:isExpanded:content:)`` when the caller needs +/// to persist the expanded / collapsed preference across app launches or share it across views +/// (PM-35398). +/// public struct ExpandableHeaderView: View { // MARK: Properties @@ -14,25 +20,38 @@ public struct ExpandableHeaderView: View { /// The content that is shown when the section is expanded or hidden otherwise. let content: Content - /// A var to determine if the content in the section is expanded or collapsed. - @State private var isExpanded: Bool = true - /// A value indicating whether the expandable content is currently enabled or disabled. @Environment(\.isEnabled) var isEnabled: Bool /// The title of the Header button used to expand or collapse the content. let title: String - /// The count of items on the Content + /// The count of items on the Content. let count: Int + /// Expansion state ownership. When the caller supplies a `Binding` the view writes to + /// / reads from that binding directly; when the caller omits it, the view falls back to its + /// own `@State`-owned storage. Keeping both storages on the struct (rather than splitting + /// into two types) preserves the `ExpandableHeaderView(...)` initializer shape that + /// Authenticator relies on. + @State private var internalIsExpanded: Bool = true + + /// The caller-supplied binding, when present. `body` prefers this over `internalIsExpanded`. + private let externalIsExpanded: Binding? + + /// The unified binding used by `body` and `expandButton`. Resolves to `externalIsExpanded` + /// when the caller provided one, or to `$internalIsExpanded` otherwise. + private var isExpanded: Binding { + externalIsExpanded ?? $internalIsExpanded + } + // MARK: View public var body: some View { VStack(spacing: 8) { expandButton - if isExpanded { + if isExpanded.wrappedValue { content } } @@ -44,7 +63,7 @@ public struct ExpandableHeaderView: View { @ViewBuilder private var expandButton: some View { Button { withAnimation { - isExpanded.toggle() + isExpanded.wrappedValue.toggle() } } label: { HStack(spacing: 8) { @@ -52,7 +71,7 @@ public struct ExpandableHeaderView: View { SharedAsset.Icons.chevronDown16.swiftUIImage .imageStyle(.accessoryIcon16(scaleWithFont: true)) - .rotationEffect(isExpanded ? Angle(degrees: 180) : .zero) + .rotationEffect(isExpanded.wrappedValue ? Angle(degrees: 180) : .zero) } .multilineTextAlignment(.leading) .foregroundStyle(SharedAsset.Colors.textSecondary.swiftUIColor) @@ -65,11 +84,27 @@ public struct ExpandableHeaderView: View { // MARK: Initialization - /// Initialize an `ExpandableContent`. + /// Initialize an `ExpandableHeaderView` whose expansion state is owned by the view itself. + /// + /// The view creates an internal `@State` that is reset whenever the view's identity changes. + /// Use this initializer when the caller does not need to persist or share the expansion + /// state. + /// + /// The internal state defaults to expanded (`true`). This default is part of the public + /// contract — callers that need "collapsed by default" behavior should use + /// ``init(title:count:buttonAccessibilityIdentifier:isExpanded:content:)`` with a + /// pre-initialized `Binding` set to `false`. + /// + /// - Important: A given call site should commit to one initializer variant. Switching + /// between the no-binding and binding initializer at runtime (for example, inside a + /// conditional branch that flips its predicate) is undefined: both initializers produce + /// the same ``ExpandableHeaderView`` type, so SwiftUI preserves the view's identity and + /// reuses the internal `@State` cell, but whichever storage is no longer populated on the + /// new render will be silently abandoned. /// /// - Parameters: /// - title: The title of the button used to expand or collapse the content. - /// - isExpanded: A binding to determine if the content in the section is expanded or collapsed. + /// - count: The count of items on the Content. /// - buttonAccessibilityIdentifier: The accessibility identifier for the button to expand or /// collapse the content. /// - content: The content that is shown when the section is expanded or hidden otherwise. @@ -83,6 +118,41 @@ public struct ExpandableHeaderView: View { self.content = content() self.title = title self.count = count + externalIsExpanded = nil + } + + /// Initialize an `ExpandableHeaderView` whose expansion state is owned by the caller. + /// + /// Use this initializer when the expanded / collapsed preference must survive view + /// recreation — typically because it is persisted to a store and rehydrated on view appear + /// (PM-35398). The caller is responsible for providing a `Binding` whose storage + /// outlives the view. + /// + /// - Important: A given call site should commit to one initializer variant. Switching + /// between the binding and no-binding initializer at runtime is undefined; see the + /// companion discussion on ``init(title:count:buttonAccessibilityIdentifier:content:)``. + /// + /// - Parameters: + /// - title: The title of the button used to expand or collapse the content. + /// - count: The count of items on the Content. + /// - buttonAccessibilityIdentifier: The accessibility identifier for the button to expand or + /// collapse the content. + /// - isExpanded: A binding that drives whether the content is currently expanded. The view + /// reads this value on every render and writes back through it when the user toggles + /// the header. + /// - content: The content that is shown when the section is expanded or hidden otherwise. + public init( + title: String, + count: Int, + buttonAccessibilityIdentifier: String = "ExpandSectionButton", + isExpanded: Binding, + @ViewBuilder content: () -> Content, + ) { + self.buttonAccessibilityIdentifier = buttonAccessibilityIdentifier + self.content = content() + self.title = title + self.count = count + externalIsExpanded = isExpanded } } @@ -90,11 +160,29 @@ public struct ExpandableHeaderView: View { #if DEBUG @available(iOS 17, *) -#Preview { +#Preview("Internal state") { + VStack { + ExpandableHeaderView(title: Localizations.localCodes, count: 3) { + BitwardenTextValueField(value: "Option 1") + BitwardenTextValueField(value: "Option 2") + BitwardenTextValueField(value: "Option 3") + } + } + .padding() + .frame(maxHeight: .infinity, alignment: .top) + .background(SharedAsset.Colors.backgroundPrimary.swiftUIColor) +} + +@available(iOS 17, *) +#Preview("Caller-owned binding") { @Previewable @SwiftUI.State var isExpanded = false VStack { - ExpandableHeaderView(title: Localizations.localCodes, count: 3) { + ExpandableHeaderView( + title: Localizations.localCodes, + count: 3, + isExpanded: $isExpanded, + ) { BitwardenTextValueField(value: "Option 1") BitwardenTextValueField(value: "Option 2") BitwardenTextValueField(value: "Option 3")