Skip to content

Commit

Permalink
Review supereg
Browse files Browse the repository at this point in the history 
Modifications:

* identify `OnboardingStack` views through `id: any Hashable` instead of
  `id: String`
* rename `.id(_:)` view modifier to `.onboardingIdentifier(_:)` to
  disambiguate from SwiftUI's own `.id(_:)` view modifier
* refactor `OnboardingStepIdentifier` to store a custom hash of the
  given view / view id
* add `extension ModifiedContent` to conditionally make it
  `Identifiable`
* adjust UITest to verify that the `.onboardingIdentifier(_:)` works
* add `.onboardingIdentifier(_:)` example in `OnboardingStack` DocC
  documentation
* make sure to only expose API that has to be exposed
  • Loading branch information
@felixschlegel
felixschlegel committed Apr 15, 2024
1 parent 8edfc57 commit 167f639
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 41 deletions.
31 changes: 5 additions & 26 deletions Sources/SpeziOnboarding/OnboardingFlow/OnboardingNavigationPath.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public class OnboardingNavigationPath {
/// - onboardingStepType: The type of the onboarding `View` which should be displayed next. Must be declared within the ``OnboardingStack``.
public func append(_ onboardingStepType: any View.Type) {
let onboardingStepIdentifier = OnboardingStepIdentifier(
onboardingStepType: String(describing: onboardingStepType),
onboardingStepType: onboardingStepType,
custom: false
)
guard onboardingSteps.keys.contains(onboardingStepIdentifier) else {
Expand All @@ -165,34 +165,16 @@ public class OnboardingNavigationPath {
/// - Parameters:
/// - customView: A custom onboarding `View` instance that should be shown next in the onboarding flow.
/// It isn't required to declare this view within the ``OnboardingStack``.
public func append<V: View>(customView: V) {
public func append(customView: any View) {
let customOnboardingStepIdentifier = OnboardingStepIdentifier(
onboardingStepType: String(describing: V.self),
view: customView,
custom: true
)
customOnboardingSteps[customOnboardingStepIdentifier] = customView

appendToInternalNavigationPath(of: customOnboardingStepIdentifier)
}

/// Moves the navigation path to the custom `Identifiable` view.
///
/// - Note: The custom `View` does not have to be declared within the ``OnboardingStack``.
/// Resulting from that, the internal state of the ``OnboardingNavigationPath`` is still referencing to the last regular `OnboardingStep`.
///
/// - Parameters:
/// - customView: A custom onboarding `View` instance that should be shown next in the onboarding flow.
/// It isn't required to declare this view within the ``OnboardingStack``.
public func append<V: View & Identifiable>(customView: V) {
let customOnboardingStepIdentifier = OnboardingStepIdentifier(
onboardingStepType: String(describing: customView.id),
custom: true
)
customOnboardingSteps[customOnboardingStepIdentifier] = customView

appendToInternalNavigationPath(of: customOnboardingStepIdentifier)
}

/// Removes the last element on top of the navigation path.
///
/// This method allows to manually move backwards within the onboarding navigation flow.
Expand All @@ -217,14 +199,11 @@ public class OnboardingNavigationPath {
self.onboardingStepsOrder.removeAll(keepingCapacity: true)

for view in views {
let onboardingStepIdentifier = OnboardingStepIdentifier(
onboardingStepType: String(describing: type(of: view)),
custom: false
)
let onboardingStepIdentifier = OnboardingStepIdentifier(view: view)

guard self.onboardingSteps[onboardingStepIdentifier] == nil else {
preconditionFailure("""
SpeziOnboarding: Duplicate Onboarding step of type `\(onboardingStepIdentifier.onboardingStepType)` identified.
SpeziOnboarding: Duplicate Onboarding step identifier`\(onboardingStepIdentifier)`.
Ensure unique Onboarding view instances within the `OnboardingStack`!
""")
}
Expand Down
21 changes: 21 additions & 0 deletions Sources/SpeziOnboarding/OnboardingFlow/OnboardingStack.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ import SwiftUI
/// }
/// }
/// ```
///
/// ### Identifying Onboarding Views
///
/// Apply the `onboardingIdentifier(_:)` modifier to clearly identify a view in the `OnboardingStack`.
/// This is particularly useful in scenarios where multiple instances of the same view type might appear in the stack.
///
/// ```swift
/// struct Onboarding: View {
/// @AppStorage(StorageKeys.onboardingFlowComplete) var completedOnboardingFlow = false
///
/// var body: some View {
/// OnboardingStack(onboardingFlowComplete: $completedOnboardingFlow) {
/// MyOwnView().onboardingIdentifier("my-own-view-1")
/// MyOwnView().onboardingIdentifier("my-own-view-2")
/// // Other views as needed
/// }
/// }
/// }
/// ```
///
/// - Note: When the `onboardingIdentifier(_:)` modifier is applied multiple times to the same view, the most recently applied identifier takes precedence.
public struct OnboardingStack: View {
@State var onboardingNavigationPath: OnboardingNavigationPath
private let collection: _OnboardingFlowViewCollection
Expand Down
29 changes: 28 additions & 1 deletion Sources/SpeziOnboarding/OnboardingFlow/OnboardingStepIdentifier.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,33 @@ import SwiftUI
///
/// It contains both the identifier for an onboarding step (the view's type) as well as a flag that indicates if it's a custom onboarding step.
struct OnboardingStepIdentifier: Hashable, Codable {
let onboardingStepType: String
let custom: Bool
let identifierHash: Int

/// Initializes an identifier using a view. If the view conforms to `Identifiable`, its `id` is used; otherwise, the view's type is used.
/// - Parameters:
/// - view: The view used to initialize the identifier.
/// - custom: A flag indicating whether the step is custom.
init<V: View>(view: V, custom: Bool = false) {
self.custom = custom
var hasher = Hasher()
if let identifiable = view as? any Identifiable {
let id = identifiable.id
hasher.combine(id)
} else {
hasher.combine(String(describing: type(of: view)))
}
self.identifierHash = hasher.finalize()
}

/// Initializes an identifier using a view type.
/// - Parameters:
/// - onboardingStepType: The class of the view used to initialize the identifier.
/// - custom: A flag indicating whether the step is custom.
init(onboardingStepType: any View.Type, custom: Bool = false) {
self.custom = custom
var hasher = Hasher()
hasher.combine(String(describing: onboardingStepType))
self.identifierHash = hasher.finalize()
}
}
43 changes: 32 additions & 11 deletions Sources/SpeziOnboarding/OnboardingIdentifiableViewModifier.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,52 @@
import SwiftUI

/// Wrap `Content` `View` in an `Identifiable` `View`.
public struct OnboardingIdentifiableView<Content>: View, Identifiable where Content: View {
private struct OnboardingIdentifiableView<Content, ID>: View, Identifiable where Content: View, ID: Hashable {
/// Unique identifier of the wrapped `View`.
public let id: String
let id: ID
/// Wrapped `View`.
public var body: Content
let body: Content
}

struct OnboardingIdentifiableViewModifier: ViewModifier {
let identifier: String
private struct OnboardingIdentifiableViewModifier<ID>: ViewModifier, Identifiable where ID: Hashable {
let id: ID

func body(content: Content) -> some View {
OnboardingIdentifiableView(
id: self.identifier,
id: self.id,
body: content
)
}
}


extension View {
/// `ViewModifier` assigning an identifier to the `View` it is applied to.
/// When applying this modifier repeatedly, the outermost ``id(_:)`` counts.
/// When applying this modifier repeatedly, the outermost ``onboardingIdentifier(_:)`` counts.
///
/// - Note: This `ViewModifier` should only be used to identify `View`s of the same type within an ``OnboardingStack``.
///
/// - Parameters:
/// - identifier: The `String` identifier given to the view.
public func id(_ identifier: String) -> some View {
modifier(OnboardingIdentifiableViewModifier(identifier: identifier))
/// - identifier: The `Hashable` identifier given to the view.
///
/// ```swift
/// struct Onboarding: View {
/// @AppStorage(StorageKeys.onboardingFlowComplete) var completedOnboardingFlow = false
///
/// var body: some View {
/// OnboardingStack(onboardingFlowComplete: $completedOnboardingFlow) {
/// MyOwnView().onboardingIdentifier("my-own-view-1")
/// MyOwnView().onboardingIdentifier("my-own-view-2")
/// }
/// }
/// }
/// ```
public func onboardingIdentifier<ID>(_ identifier: ID) -> some View where ID: Hashable {
modifier(OnboardingIdentifiableViewModifier(id: identifier))
}
}

extension ModifiedContent: Identifiable where Modifier: Identifiable {
public var id: Modifier.ID {
self.modifier.id
}
}
4 changes: 3 additions & 1 deletion Tests/UITests/TestApp/OnboardingTestsView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ struct OnboardingTestsView: View {
OnboardingSequentialTestView()
OnboardingConsentMarkdownTestView()
OnboardingConsentMarkdownRenderingView()

OnboardingTestViewNotIdentifiable(text: "Leland").onboardingIdentifier("a")
OnboardingTestViewNotIdentifiable(text: "Stanford").onboardingIdentifier("b")

if showConditionalView {
OnboardingConditionalTestView()
}
Expand Down
41 changes: 41 additions & 0 deletions Tests/UITests/TestApp/Views/OnboardingTestViewNotIdentifiable.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//
// This source file is part of the Stanford Spezi open-source project
//
// SPDX-FileCopyrightText: 2024 Stanford University and the project authors (see CONTRIBUTORS.md)
//
// SPDX-License-Identifier: MIT
//

import SpeziOnboarding
import SwiftUI

struct OnboardingTestViewNotIdentifiable: View {
var text: String

@Environment(OnboardingNavigationPath.self) private var path


var body: some View {
VStack(spacing: 12) {
Text(self.text)

Button {
path.nextStep()
} label: {
Text("Next")
}
}
}
}

#if DEBUG
struct OnboardingTestViewNotIdentifiable_Previews: PreviewProvider {
static var previews: some View {
OnboardingStack(startAtStep: OnboardingTestViewNotIdentifiable.self) {
for onboardingView in OnboardingFlow.previewSimulatorViews {
onboardingView
}
}
}
}
#endif
18 changes: 16 additions & 2 deletions Tests/UITests/TestAppUITests/SpeziOnboardingTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,15 @@ final class OnboardingTests: XCTestCase { // swiftlint:disable:this type_body_le

XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()


XCTAssert(app.staticTexts["Leland"].waitForExistence(timeout: 2))
XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()

XCTAssert(app.staticTexts["Stanford"].waitForExistence(timeout: 2))
XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()

// Check if on final page
XCTAssert(app.staticTexts["Onboarding complete"].waitForExistence(timeout: 2))
}
Expand Down Expand Up @@ -412,7 +420,13 @@ final class OnboardingTests: XCTestCase { // swiftlint:disable:this type_body_le

XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()


XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()

XCTAssert(app.buttons["Next"].waitForExistence(timeout: 2))
app.buttons["Next"].tap()

if showConditionalView {
// Check if on conditional test view
XCTAssert(app.staticTexts["Conditional Test View"].waitForExistence(timeout: 2))
Expand Down
4 changes: 4 additions & 0 deletions Tests/UITests/UITests.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
2F6D139A28F5F386007C25D6 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 2F6D139928F5F386007C25D6 /* Assets.xcassets */; };
2FA7382C290ADFAA007ACEB9 /* TestApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2FA7382B290ADFAA007ACEB9 /* TestApp.swift */; };
61040A1D2BAFA2F600EDD4EC /* OnboardingIdentifiableTestViewCustom.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61040A1B2BAFA2F600EDD4EC /* OnboardingIdentifiableTestViewCustom.swift */; };
61F1697E2BCA888600D1622B /* OnboardingTestViewNotIdentifiable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61F1697D2BCA888600D1622B /* OnboardingTestViewNotIdentifiable.swift */; };
970D444B2A6F031200756FE2 /* OnboardingConsentMarkdownTestView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 970D444A2A6F031200756FE2 /* OnboardingConsentMarkdownTestView.swift */; };
970D444F2A6F048A00756FE2 /* OnboardingWelcomeTestView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 970D444E2A6F048A00756FE2 /* OnboardingWelcomeTestView.swift */; };
970D44512A6F04ED00756FE2 /* OnboardingSequentialTestView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 970D44502A6F04ED00756FE2 /* OnboardingSequentialTestView.swift */; };
Expand Down Expand Up @@ -48,6 +49,7 @@
2FA7382B290ADFAA007ACEB9 /* TestApp.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestApp.swift; sourceTree = "<group>"; };
2FB0758A299DDB9000C0B37F /* TestApp.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = TestApp.xctestplan; sourceTree = "<group>"; };
61040A1B2BAFA2F600EDD4EC /* OnboardingIdentifiableTestViewCustom.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingIdentifiableTestViewCustom.swift; sourceTree = "<group>"; };
61F1697D2BCA888600D1622B /* OnboardingTestViewNotIdentifiable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingTestViewNotIdentifiable.swift; sourceTree = "<group>"; };
970D444A2A6F031200756FE2 /* OnboardingConsentMarkdownTestView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingConsentMarkdownTestView.swift; sourceTree = "<group>"; };
970D444E2A6F048A00756FE2 /* OnboardingWelcomeTestView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingWelcomeTestView.swift; sourceTree = "<group>"; };
970D44502A6F04ED00756FE2 /* OnboardingSequentialTestView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingSequentialTestView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -145,6 +147,7 @@
97A8FF2D2A7444FC008CD91A /* OnboardingCustomTestView2.swift */,
970D44542A6F119600756FE2 /* OnboardingConditionalTestView.swift */,
97C6AF7E2ACC94450060155B /* OnboardingFlow+PreviewSimulator.swift */,
61F1697D2BCA888600D1622B /* OnboardingTestViewNotIdentifiable.swift */,
);
path = Views;
sourceTree = "<group>";
Expand Down Expand Up @@ -289,6 +292,7 @@
buildActionMask = 2147483647;
files = (
97C6AF792ACC88270060155B /* TestAppDelegate.swift in Sources */,
61F1697E2BCA888600D1622B /* OnboardingTestViewNotIdentifiable.swift in Sources */,
970D44552A6F119600756FE2 /* OnboardingConditionalTestView.swift in Sources */,
97C6AF772ACC86B70060155B /* ExampleStandard.swift in Sources */,
970D44532A6F0B1900756FE2 /* OnboardingStartTestView.swift in Sources */,
Expand Down

0 comments on commit 167f639

Please sign in to comment.