Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public struct ToastView: View {
.accessibilityIdentifier("ToastElement")
.accessibilityElement(children: .combine)
.padding(.horizontal, 12)
.task {
.task(id: toast.id) {
guard self.toast?.mode == .automaticDismiss else { return }
do {
try await Task.sleep(nanoseconds: 3 * NSEC_PER_SEC)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ protocol AddEditFolderDelegate: AnyObject {
/// Called when the folder has been successfully created.
func folderAdded(_ folderView: FolderView)

/// Called when the folder has been successfully edited.
/// Called when the folder has been successfully deleted.
func folderDeleted()

/// Called when the folder has been successfully deleted.
/// Called when the folder has been successfully edited.
func folderEdited()
}

Expand Down
9 changes: 6 additions & 3 deletions BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disa
case .addAccount:
delegate?.didTapAddAccount()
case .addFolder:
showAddFolder()
showAddFolder(delegate: context as? AddEditFolderDelegate)
case let .addItem(group, newCipherOptions, organizationId, type):
Task {
let hasPremium = await services.vaultRepository.doesActiveAccountHavePremium()
Expand Down Expand Up @@ -294,11 +294,14 @@ final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disa

/// Shows the add folder screen.
///
private func showAddFolder() {
/// - Parameter delegate: A `AddEditFolderDelegate` that is notified when the user makes a
/// change to folders.
///
private func showAddFolder(delegate: AddEditFolderDelegate?) {
let navigationController = module.makeNavigationController()
let coordinator = module.makeAddEditFolderCoordinator(stackNavigator: navigationController)
coordinator.start()
coordinator.navigate(to: .addEditFolder(folder: nil))
coordinator.navigate(to: .addEditFolder(folder: nil), context: delegate)

stackNavigator?.present(navigationController)
}
Expand Down
16 changes: 16 additions & 0 deletions BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,22 @@ class VaultCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this type_

XCTAssertTrue(module.addEditFolderCoordinator.isStarted)
XCTAssertEqual(module.addEditFolderCoordinator.routes, [.addEditFolder(folder: nil)])
XCTAssertNil(module.addEditFolderCoordinator.contexts.last as? AddEditFolderDelegate)
}

/// `navigate(to:)` with `.addFolder` and a context passes the context along to the add/edit
/// folder coordinator as the delegate.
@MainActor
func test_navigateTo_addFolder_withDelegate() throws {
let delegate = MockAddEditFolderDelegate()
subject.navigate(to: .addFolder, context: delegate)

XCTAssertTrue(module.addEditFolderCoordinator.isStarted)
XCTAssertEqual(module.addEditFolderCoordinator.routes, [.addEditFolder(folder: nil)])
XCTAssertIdentical(
module.addEditFolderCoordinator.contexts.last as? AddEditFolderDelegate,
delegate,
)
}

/// `navigate(to:)` with `.autofillList` replaces the stack navigator's stack with the autofill list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ final class VaultListProcessor: StateProcessor<
& HasTimeProvider
& HasVaultRepository

// MARK: Static Properties

/// The toast shown while the vault is taking an unusually long time to load. A new instance is
/// returned on each access so that the view animates between successive toasts.
private static var slowLoadingToast: Toast {
Toast(title: Localizations.thisIsTakingLongerThanExpected, mode: .manualDismiss)
}

// MARK: Private Properties

/// The `Coordinator` that handles navigation.
Expand Down Expand Up @@ -152,7 +160,7 @@ final class VaultListProcessor: StateProcessor<
override func receive(_ action: VaultListAction) {
switch action {
case .addFolder:
coordinator.navigate(to: .addFolder)
coordinator.navigate(to: .addFolder, context: self)
case let .addItemPressed(type):
addItem(type: type)
case .appReviewPromptShown:
Expand Down Expand Up @@ -366,6 +374,17 @@ extension VaultListProcessor {
}
}

/// Dismisses the toast shown while the vault is taking an unusually long time to load, if
/// that's the toast currently displayed.
///
/// Any other toast is left in place, so that a toast shown in response to a folder or item
/// operation isn't cleared out from under the user when the vault list refreshes.
///
private func dismissSlowLoadingToast() {
guard state.toast == Self.slowLoadingToast else { return }
state.toast = nil
}

/// If the vault has ciphers which failed to decrypt, and the cipher decryption failure alert
/// hasn't been shown yet, notify the user that a cipher(s) failed to decrypt.
///
Expand Down Expand Up @@ -494,11 +513,11 @@ extension VaultListProcessor {
try await Task.sleep(forSeconds: 5)
// If we already have data, don't show the toast
guard case .loading = self.state.loadingState else { return }
self.state.toast = Toast(title: Localizations.thisIsTakingLongerThanExpected, mode: .manualDismiss)
self.state.toast = Self.slowLoadingToast
}
defer {
state.toast = nil
takingTimeTask.cancel()
dismissSlowLoadingToast()
}

try await services.vaultRepository.fetchSync(
Expand Down Expand Up @@ -774,7 +793,7 @@ extension VaultListProcessor {
if !needsSync || !value.isEmpty {
// Dismiss the "this is taking a while" toast now that we have data,
// since this might not happen because of the sync in `refreshVault()`.
state.toast = nil
dismissSlowLoadingToast()
// If the data is not empty or if a sync is not needed, set the data.
state.loadingState = .data(value)
} else {
Expand Down Expand Up @@ -813,6 +832,22 @@ extension VaultListProcessor {
}
}

// MARK: - AddEditFolderDelegate

extension VaultListProcessor: AddEditFolderDelegate {
func folderAdded(_: FolderView) {
state.toast = Toast(title: Localizations.folderCreated)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

πŸ€” One thing I noticed is that if this toast is displayed while the "This is taking longer than expected" toast is visible, the "New folder created" toast is never dismissed.

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-08-26.at.10.06.10.mov

This might be an existing bug introduced by this: https://github.com/bitwarden/ios/pull/2827/changes#diff-68b12b1d47e8d01df3d910e70d76c1ba459cafe2c142501b36a2febad412be45. Any idea if we can fix this, revert that change, or switch the task to onChange(of:)?

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.

Good catch, thanks. The root cause was in ToastView rather than here:

  • The .task running the dismiss timer had no id, and .id(toast.id) sits further in on the VStack, so swapping state.toast for a different toast never restarted it.
  • The task that did run started while the manual dismiss toast was up, hit the guard mode == .automaticDismiss and returned, so the "New folder created" toast ended up with no timer at all.
  • Changed it to .task(id: toast.id) in 87458dd. That also fixes a second case where one automatic toast replacing another inherited the first's leftover time and could disappear almost immediately.

Verified in the simulator with both toasts in sequence.

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.

Screen.Recording.2026-08-27.at.12.18.37.mov

}

func folderDeleted() {
// No-op: deleting a folder isn't supported from the vault list.
}

func folderEdited() {
// No-op: editing a folder isn't supported from the vault list.
}
}

// MARK: - CipherItemOperationDelegate

extension VaultListProcessor: CipherItemOperationDelegate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,38 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ
XCTAssertEqual(reviewPromptService.userActions, [])
}

/// `folderAdded(_:)` delegate method shows the expected toast.
@MainActor
func test_delegate_folderAdded() {
XCTAssertNil(subject.state.toast)

subject.folderAdded(.fixture())

XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `folderDeleted()` delegate method leaves the toast untouched, since folders can't be
/// deleted from the vault list.
@MainActor
func test_delegate_folderDeleted() {
subject.state.toast = Toast(title: Localizations.folderCreated)

subject.folderDeleted()

XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `folderEdited()` delegate method leaves the toast untouched, since folders can't be edited
/// from the vault list.
@MainActor
func test_delegate_folderEdited() {
subject.state.toast = Toast(title: Localizations.folderCreated)

subject.folderEdited()

XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `perform(_:)` with `.checkAppReviewEligibility` schedules a review prompt if the user is eligible
/// and the feature flags are enabled.
@MainActor
Expand Down Expand Up @@ -822,6 +854,17 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ
XCTAssertEqual(vaultRepository.fetchSyncForceSync, false)
}

/// `perform(_:)` with `.refreshVault` leaves a toast that was shown for an unrelated reason
/// in place, rather than clearing it once the sync completes.
@MainActor
func test_perform_refreshVault_doesNotDismissUnrelatedToast() async {
subject.state.toast = Toast(title: Localizations.folderCreated)

await subject.perform(.refreshVault)

XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `perform(_:)` with `.refreshVault` requests a vault sync and sets the loading state if the
/// vault is empty; in this case sync is not flagged as periodic.
@MainActor
Expand Down Expand Up @@ -1325,6 +1368,44 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ
XCTAssertEqual(stateService.accountSetupImportLogins["1"], .complete)
}

/// `perform(_:)` with `.streamVaultList` dismisses the toast shown while the vault was taking
/// a long time to load, once vault data arrives.
@MainActor
func test_perform_streamVaultList_dismissesSlowLoadingToast() {
subject.state.toast = Toast(title: Localizations.thisIsTakingLongerThanExpected, mode: .manualDismiss)
vaultRepository.vaultListSubject.send(VaultListData(
sections: [VaultListSection(id: "1", items: [.fixture()], name: "Name")],
))

let task = Task {
await subject.perform(.streamVaultList)
}

waitFor(subject.state.toast == nil)
task.cancel()

XCTAssertNil(subject.state.toast)
}

/// `perform(_:)` with `.streamVaultList` leaves a toast that was shown for an unrelated reason
/// in place when vault data arrives, so that it isn't cleared out from under the user.
@MainActor
func test_perform_streamVaultList_doesNotDismissUnrelatedToast() {
subject.state.toast = Toast(title: Localizations.folderCreated)
vaultRepository.vaultListSubject.send(VaultListData(
sections: [VaultListSection(id: "1", items: [.fixture()], name: "Name")],
))

let task = Task {
await subject.perform(.streamVaultList)
}

waitFor(subject.state.loadingState != .loading(nil))
task.cancel()

XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `perform(_:)` with `.streamVaultList` doesn't dismiss the import logins action card if the
/// vault list is empty.
@MainActor
Expand Down Expand Up @@ -1986,12 +2067,14 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ
XCTAssertEqual(coordinator.routes.last, .addAccount)
}

/// `receive(_:)` with `.addFolder` navigates to the `.addFolder` route.
/// `receive(_:)` with `.addFolder` navigates to the `.addFolder` route with the processor as
/// the delegate.
@MainActor
func test_receive_addFolder() {
subject.receive(.addFolder)

XCTAssertEqual(coordinator.routes.last, .addFolder)
XCTAssertIdentical(coordinator.contexts.last as? AddEditFolderDelegate, subject)
}

/// `receive(_:)` with `.addItemPressed` navigates to the `.addItem` route.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ extension AddEditItemProcessor: GeneratorCoordinatorDelegate {
extension AddEditItemProcessor: AddEditFolderDelegate {
func folderAdded(_ folderView: FolderView) {
state.folder = .custom(folderView)
state.toast = Toast(title: Localizations.folderCreated)
}

func folderDeleted() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,18 @@ class AddEditItemProcessorTests: BitwardenTestCase {
XCTAssertEqual(subject.state.toast, Toast(title: Localizations.itemUpdated))
}

/// `folderAdded(_:)` sets the selected folder to the folder that was added.
/// `folderAdded(_:)` sets the selected folder to the folder that was added and shows the
/// folder created toast.
@MainActor
func test_folderAdded() {
let newFolder = FolderView.fixture(name: "New folder")
subject.state.folders = [.default, .custom(newFolder)]
XCTAssertNil(subject.state.toast)

subject.folderAdded(newFolder)

XCTAssertEqual(subject.state.folder, .custom(newFolder))
XCTAssertEqual(subject.state.toast, Toast(title: Localizations.folderCreated))
}

/// `init(appExtensionDelegate:coordinator:delegate:services:state:)` with adding configuration
Expand Down
Loading