diff --git a/TFI/Features/Assistant/MobileAssistantView.swift b/TFI/Features/Assistant/MobileAssistantView.swift index 20803d89..e6704440 100644 --- a/TFI/Features/Assistant/MobileAssistantView.swift +++ b/TFI/Features/Assistant/MobileAssistantView.swift @@ -62,18 +62,23 @@ struct MobileAssistantView: View { Task { await model.move(conversation, pinned: !conversation.isPinned) } } .disabled(model.conversation.isEmpty || model.conversation.isArchived) - Button( - model.conversation.isArchived ? "Unarchive" : "Archive", - systemImage: model.conversation.isArchived ? "arrow.uturn.backward" : "archivebox" - ) { - let conversation = model.conversation - Task { await model.archive(conversation, archived: !conversation.isArchived) } + if model.conversation.isArchived { + Button("Unarchive", systemImage: "arrow.uturn.backward") { + let conversation = model.conversation + Task { await model.archive(conversation, archived: false) } + } + .disabled(!canChangeHistory) + Button("Delete", systemImage: "trash", role: .destructive) { + deleting = model.conversation + } + .disabled(!canChangeHistory) + } else { + Button("Archive", systemImage: "archivebox", role: .destructive) { + let conversation = model.conversation + Task { await model.archive(conversation, archived: true) } + } + .disabled(!canChangeHistory) } - .disabled(!canChangeHistory) - Button("Delete", systemImage: "trash", role: .destructive) { - deleting = model.conversation - } - .disabled(!canChangeHistory) } label: { MobileOverflowMenuLabel("More") } @@ -348,10 +353,9 @@ struct MobileAssistantLibrary: View { .tint(.accentColor) .disabled(!canChangeHistory(conversation)) } else { - Button("Archive", systemImage: "archivebox") { + Button("Archive", systemImage: "archivebox", role: .destructive) { Task { await model.archive(conversation, archived: true) } } - .tint(.orange) .disabled(!canChangeHistory(conversation)) } } @@ -374,14 +378,19 @@ struct MobileAssistantLibrary: View { Task { await model.move(conversation, pinned: !conversation.isPinned) } }.disabled(conversation.isStreaming || conversation.isCompacting || conversation.isHistoryChanging) } - Button( - conversation.isArchived ? "Unarchive" : "Archive", - systemImage: conversation.isArchived ? "arrow.uturn.backward" : "archivebox" - ) { - Task { await model.archive(conversation, archived: !conversation.isArchived) } - }.disabled(conversation.isStreaming || conversation.isCompacting || conversation.isHistoryChanging) - Button("Delete", systemImage: "trash", role: .destructive) { deleting = conversation } + if conversation.isArchived { + Button("Unarchive", systemImage: "arrow.uturn.backward") { + Task { await model.archive(conversation, archived: false) } + } .disabled(!canChangeHistory(conversation)) + Button("Delete", systemImage: "trash", role: .destructive) { deleting = conversation } + .disabled(!canChangeHistory(conversation)) + } else { + Button("Archive", systemImage: "archivebox", role: .destructive) { + Task { await model.archive(conversation, archived: true) } + } + .disabled(!canChangeHistory(conversation)) + } } private func canChangeHistory(_ conversation: ConversationModel) -> Bool { diff --git a/TFITests/MobileNavigationParityTests.swift b/TFITests/MobileNavigationParityTests.swift index 465ffb30..0446f7b1 100644 --- a/TFITests/MobileNavigationParityTests.swift +++ b/TFITests/MobileNavigationParityTests.swift @@ -32,6 +32,9 @@ struct MobileNavigationParityTests { let assistantRows = try #require( assistant.components(separatedBy: "private func conversationRow").last? .components(separatedBy: "private func select").first) + let conversationActions = try #require( + assistant.components(separatedBy: "@ViewBuilder private func actions").last? + .components(separatedBy: "private func canChangeHistory").first) let workspaceRows = try #require( workspaceList.components(separatedBy: "private var workspaceRows").last? .components(separatedBy: "private func statusTint").first) @@ -62,8 +65,14 @@ struct MobileNavigationParityTests { #expect(!assistant.contains("geometry.size.width >= 760")) #expect(!assistant.contains("MobileAssistantLibrary(model: model, selected: {})")) #expect(assistant.contains(".swipeActions(edge: .trailing, allowsFullSwipe: true)")) - #expect(assistant.contains("Button(\"Archive\", systemImage: \"archivebox\")")) + #expect(assistant.contains("Button(\"Archive\", systemImage: \"archivebox\", role: .destructive)")) #expect(assistant.contains("Button(\"Unarchive\", systemImage: \"arrow.uturn.backward\")")) + #expect(currentConversationMenu.contains("if model.conversation.isArchived")) + #expect(currentConversationMenu.contains("Button(\"Delete\", systemImage: \"trash\", role: .destructive)")) + #expect(conversationActions.contains("if conversation.isArchived")) + #expect(conversationActions.contains("Button(\"Delete\", systemImage: \"trash\", role: .destructive)")) + #expect(assistantRows.contains("Button(\"Archive\", systemImage: \"archivebox\", role: .destructive)")) + #expect(!assistantRows.contains(".tint(.orange)")) #expect(!assistant.contains("MobileCardIcon(symbol: conversation.isArchived")) #expect(assistant.contains(".foregroundStyle(.secondary)")) let conversations = try #require(currentConversationMenu.range(of: "Button(\"Conversations\"")) diff --git a/TFM/Features/Assistant/AssistantView.swift b/TFM/Features/Assistant/AssistantView.swift index 032411f0..c5ee73c8 100644 --- a/TFM/Features/Assistant/AssistantView.swift +++ b/TFM/Features/Assistant/AssistantView.swift @@ -1244,7 +1244,7 @@ struct AssistantConversationRow: View { .accessibilityAddTraits(isSelected ? .isSelected : []) } - private var contextMenu: [NativeMenuEntry] { + var contextMenu: [NativeMenuEntry] { let managementDisabled = conversation.isStreaming || conversation.isCompacting var entries: [NativeMenuEntry] = [ NativeMenuEntry( @@ -1271,9 +1271,8 @@ struct AssistantConversationRow: View { action: onBeginRename ) ) - // Archive and Delete are both ways of getting a chat off the list, so - // the rule goes above the pair rather than between them — Pin and Rename - // above it only change how the chat is listed. + // Archive gets an active chat off the list. Permanent deletion is only + // offered after the chat has already moved to Archive. entries.append(.separator) entries.append( NativeMenuEntry( @@ -1287,13 +1286,15 @@ struct AssistantConversationRow: View { ) } ) - entries.append( - NativeMenuEntry( - title: localized("Delete"), symbol: MenuSymbol.delete - ) { - onDelete() - } - ) + if conversation.isArchived { + entries.append( + NativeMenuEntry( + title: localized("Delete"), symbol: MenuSymbol.delete + ) { + onDelete() + } + ) + } return entries } diff --git a/TFMTests/AssistantConversationMenuTests.swift b/TFMTests/AssistantConversationMenuTests.swift new file mode 100644 index 00000000..df0da449 --- /dev/null +++ b/TFMTests/AssistantConversationMenuTests.swift @@ -0,0 +1,38 @@ +import SwiftUI +import TermindApplication +import TermindDomain +import Testing + +@testable import Termind + +@Suite @MainActor +struct AssistantConversationMenuTests { + @Test + func deleteIsOnlyOfferedForArchivedConversations() { + let conversation = ConversationModel() + var row = conversationRow(conversation) + + #expect(row.contextMenu.contains { $0.symbol == MenuSymbol.archive }) + #expect(!row.contextMenu.contains { $0.symbol == MenuSymbol.delete }) + + conversation.archivedAt = .now + row = conversationRow(conversation) + + #expect(row.contextMenu.contains { $0.symbol == MenuSymbol.unarchive }) + #expect(row.contextMenu.contains { $0.symbol == MenuSymbol.delete }) + } + + private func conversationRow(_ conversation: ConversationModel) -> AssistantConversationRow { + AssistantConversationRow( + conversation: conversation, + isSelected: false, + onOpen: {}, + onDelete: {}, + isRenaming: false, + renameTitle: .constant(""), + onBeginRename: {}, + onCommitRename: {}, + onCancelRename: {} + ) + } +}