diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 70fb2397..3400855d 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -962,6 +962,9 @@ final class AppEnvironment { switch instance.type { case .invidious: return invidiousCredentialsManager case .piped: return pipedCredentialsManager + // The slot a future account-keyed store lands in. What a backend's + // accounts can do is `InstanceType.accountCapabilities`; which store + // holds their secrets is this switch, and the two move together. default: return nil } } diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index 7e7007d2..5a9fb977 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -186,6 +186,17 @@ } } }, + "accounts.current" : { + "comment" : "Marks the account the app is currently using", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Current" + } + } + } + }, "accounts.empty.description" : { "localizations" : { "en" : { @@ -5627,6 +5638,16 @@ } } }, + "login.footer.yatteeServerAccount %@" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Sign in to your account on %@" + } + } + } + }, "login.header.account" : { "comment" : "Header for account section", "localizations" : { @@ -16697,6 +16718,17 @@ } } }, + "sources.disabled.footer" : { + "comment" : "Explains the disabled sources section in the Sources tab", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Disabled sources are not browsed. Turn one back on to use it again." + } + } + } + }, "sources.editSource" : { "comment" : "Title for edit source view", "localizations" : { @@ -16730,6 +16762,17 @@ } } }, + "sources.enable" : { + "comment" : "Action that switches a disabled source back on", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Enable" + } + } + } + }, "sources.error.basicAuthInvalid" : { "comment" : "Error when HTTP Basic Auth credentials are rejected by the server", "localizations" : { @@ -17588,7 +17631,15 @@ } }, "sources.status.authFailed" : { - + "comment" : "Status when authentication was rejected", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Auth Failed" + } + } + } }, "sources.status.authRequired" : { "comment" : "Status when authentication is required", diff --git a/Yattee/Services/API/ContentService.swift b/Yattee/Services/API/ContentService.swift index 89589d48..29ea234b 100644 --- a/Yattee/Services/API/ContentService.swift +++ b/Yattee/Services/API/ContentService.swift @@ -583,6 +583,9 @@ actor ContentService: ContentServiceProtocol { instance: instance ) default: + // Routing, not policy: whether a backend has accounts to sign in to + // is `InstanceType.accountCapabilities.supportsAccounts`, and a case + // belongs here the moment that row turns true. throw APIError.notSupported } } diff --git a/Yattee/Views/Components/DataContextSwitcherMenu.swift b/Yattee/Views/Components/DataContextSwitcherMenu.swift index 0b23533a..48f1a9cf 100644 --- a/Yattee/Views/Components/DataContextSwitcherMenu.swift +++ b/Yattee/Views/Components/DataContextSwitcherMenu.swift @@ -59,38 +59,77 @@ struct DataContextSwitcherRows: View { /// Names the switchable contexts. enum DataContextSwitcherLabels { /// The context's name, plus whatever the user needs to tell it apart. - /// - /// The host is appended only when several accounts share a display name: - /// two logins on the same instance are told apart by their username, and a - /// name that already carries the host would otherwise repeat it. @MainActor static func title(for context: DataContext, in appEnvironment: AppEnvironment?) -> String { guard let appEnvironment else { return String(localized: "subscriptions.account.local") } - let name = appEnvironment.dataContextValidator.displayName(for: context) + return title( + for: context, + accountsManager: appEnvironment.accountsManager, + validator: appEnvironment.dataContextValidator, + username: { appEnvironment.accountCredentialsManager(for: $0)?.username(for: $0) } + ) + } - guard case let .account(id) = context, let account = appEnvironment.accountsManager.account(id: id) else { + /// The same label, built from only what it needs. + /// + /// Taking the pieces rather than the environment is what lets the naming + /// rules be exercised on their own: which suffix a name gets depends on the + /// other accounts, the stored identifier and the host, and on nothing else. + @MainActor + static func title( + for context: DataContext, + accountsManager: AccountsManager, + validator: DataContextValidator, + username: (Account) -> String? + ) -> String { + let name = validator.displayName(for: context) + + guard case let .account(id) = context, let account = accountsManager.account(id: id) else { return name } var title = name - if isAmbiguous(name, in: appEnvironment), - let host = appEnvironment.accountsManager.instance(for: account)?.url.host, - !name.localizedCaseInsensitiveContains(host) { - title = "\(name) · \(host)" + if isAmbiguous(name, accountsManager: accountsManager, validator: validator) { + title = disambiguated( + name, + username: username(account), + host: accountsManager.instance(for: account)?.url.host + ) } - let status = appEnvironment.accountsManager.status(for: id) + let status = accountsManager.status(for: id) guard status != .signedIn else { return title } return "\(title) — \(AccountStatusBadge.text(for: status))" } + /// What a shared name needs added to it before it names one account. + /// + /// The identifier comes first: two logins on the *same* instance share a + /// host, so a host suffix would repeat the server and still leave the user + /// choosing between two identical rows. The host alone is what is left when + /// the identifier is not on this device — tvOS never receives one — and it + /// is dropped when the name already carries it. + static func disambiguated(_ name: String, username: String?, host: String?) -> String { + if let username, !username.isEmpty { + guard let host, !host.isEmpty else { return "\(name) · \(username)" } + return "\(name) · \(username)@\(host)" + } + + guard let host, !host.isEmpty, !name.localizedCaseInsensitiveContains(host) else { return name } + return "\(name) · \(host)" + } + @MainActor - private static func isAmbiguous(_ name: String, in appEnvironment: AppEnvironment) -> Bool { - appEnvironment.accountsManager.selectableAccounts - .filter { appEnvironment.dataContextValidator.displayName(for: $0) == name } + private static func isAmbiguous( + _ name: String, + accountsManager: AccountsManager, + validator: DataContextValidator + ) -> Bool { + accountsManager.selectableAccounts + .filter { validator.displayName(for: $0) == name } .count > 1 } } diff --git a/Yattee/Views/Components/InstanceToolbarButton.swift b/Yattee/Views/Components/InstanceToolbarButton.swift deleted file mode 100644 index 0b01a6e6..00000000 --- a/Yattee/Views/Components/InstanceToolbarButton.swift +++ /dev/null @@ -1,55 +0,0 @@ -// -// InstanceToolbarButton.swift -// Yattee -// -// A toolbar button that shows the current active instance and allows switching. -// - -import SwiftUI - -struct InstanceToolbarButton: View { - @Environment(\.appEnvironment) private var appEnvironment - @State private var showingPicker = false - - var body: some View { - if let instancesManager = appEnvironment?.instancesManager, - instancesManager.enabledInstances.count > 1 { - Button { - showingPicker = true - } label: { - HStack(spacing: 4) { - Image(systemName: iconForInstance) - Text(instancesManager.activeInstance?.displayName ?? "") - .font(.subheadline) - .lineLimit(1) - Image(systemName: "chevron.down") - .font(.caption2) - } - } - .sheet(isPresented: $showingPicker) { - InstancePickerSheet() - } - } - } - - private var iconForInstance: String { - guard let instance = appEnvironment?.instancesManager.activeInstance else { - return "server.rack" - } - return instance.type.systemImage - } -} - -// MARK: - Preview - -#Preview { - NavigationStack { - Text("Content") - .toolbar { - ToolbarItem(placement: .automatic) { - InstanceToolbarButton() - } - } - } - .appEnvironment(.preview) -} diff --git a/Yattee/Views/MediaBrowser/MediaSourcesView.swift b/Yattee/Views/MediaBrowser/MediaSourcesView.swift index 11572f14..1f1c19f1 100644 --- a/Yattee/Views/MediaBrowser/MediaSourcesView.swift +++ b/Yattee/Views/MediaBrowser/MediaSourcesView.swift @@ -31,9 +31,16 @@ struct MediaSourcesView: View { appEnvironment?.sourcesSettings } + /// Whether there is nothing here at all — not merely nothing switched on. + /// + /// A user whose only source is disabled has sources; offering them the + /// "add your first source" screen would be wrong, and would hide the one + /// control that gets them back. private var isEmpty: Bool { (instancesManager?.enabledInstances.isEmpty ?? true) && - (mediaSourcesManager?.enabledSources.isEmpty ?? true) + (mediaSourcesManager?.enabledSources.isEmpty ?? true) && + disabledInstances.isEmpty && + disabledMediaSources.isEmpty } private var listStyle: VideoListStyle { @@ -221,6 +228,8 @@ struct MediaSourcesView: View { // Grouped by type (default) groupedSourcesSections } + + disabledSourcesSection } } ) @@ -389,6 +398,131 @@ struct MediaSourcesView: View { #endif } + // MARK: - Disabled Sources + + /// The sources switched off, and the way back on. + /// + /// Every listing above is filtered to what is enabled, so switching a + /// source off makes it vanish from the only screen that browses it — and + /// the toggle that undoes it lives several taps away in Settings. This + /// section is that toggle, kept at the bottom where it stays out of the way + /// until it is the thing being looked for. + @ViewBuilder + private var disabledSourcesSection: some View { + let instances = disabledInstances + let sources = disabledMediaSources + + if !instances.isEmpty || !sources.isEmpty { + sectionHeader(String(localized: "sources.status.disabled")) + .accessibilityIdentifier("sources.disabledSection") + + sectionCard { + ForEach(Array(instances.enumerated()), id: \.element.id) { index, instance in + disabledRow( + name: instance.displayName, + detail: "\(instance.type.displayName) - \(instance.url.host ?? instance.url.absoluteString)", + systemImage: instance.type.systemImage, + isLast: sources.isEmpty && index == instances.count - 1 + ) { + instancesManager?.toggleEnabled(instance) + } + } + + ForEach(Array(sources.enumerated()), id: \.element.id) { index, source in + disabledRow( + name: source.name, + detail: "\(source.type.displayName) - \(source.urlDisplayString)", + systemImage: source.type.systemImage, + isLast: index == sources.count - 1 + ) { + mediaSourcesManager?.toggleEnabled(source) + } + } + } + + Text(String(localized: "sources.disabled.footer")) + .font(.caption) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .leading) + #if os(macOS) + .padding(.horizontal, 16) + #else + .padding(.horizontal, listStyle == .inset ? 32 : 16) + #endif + .padding(.bottom, 16) + } + } + + /// Oldest first, matching the order every other listing settles into. + private var disabledInstances: [Instance] { + (instancesManager?.instances ?? []) + .filter { !$0.isEnabled } + .sorted { $0.dateAdded < $1.dateAdded } + } + + private var disabledMediaSources: [MediaSource] { + (mediaSourcesManager?.sources ?? []) + .filter { !$0.isEnabled } + .sorted { $0.dateAdded < $1.dateAdded } + } + + /// A source that is off: dimmed, and tappable only to turn it back on. + private func disabledRow( + name: String, + detail: String, + systemImage: String, + isLast: Bool, + enable: @escaping () -> Void + ) -> some View { + #if os(tvOS) + let rowSpacing: CGFloat = 24 + #else + let rowSpacing: CGFloat = 12 + #endif + #if os(macOS) + let iconFont: Font = .title3 + let iconFrameWidth: CGFloat = 24 + let titleFont: Font = .body + #else + let iconFont: Font = .title2 + let iconFrameWidth: CGFloat = 32 + let titleFont: Font = .headline + #endif + + return SourceListRow(isLast: isLast, listStyle: listStyle) { + Button(action: enable) { + HStack(spacing: rowSpacing) { + Image(systemName: systemImage) + .font(iconFont) + .foregroundStyle(.secondary) + .frame(width: iconFrameWidth) + + VStack(alignment: .leading, spacing: 2) { + Text(name) + .font(titleFont) + .foregroundStyle(.secondary) + + Text(detail) + .font(.caption) + .foregroundStyle(.tertiary) + } + + Spacer() + + Label(String(localized: "sources.enable"), systemImage: "power") + .font(.caption) + .foregroundStyle(.tint) + } + .contentShape(Rectangle()) + } + #if os(macOS) + .buttonStyle(.plain) + #else + .foregroundStyle(.primary) + #endif + } + } + @ViewBuilder private func instancesSectionContent(_ instances: [Instance], firstIsGlobalFirst: Bool = false) -> some View { ForEach(Array(instances.enumerated()), id: \.element.id) { index, instance in diff --git a/Yattee/Views/Settings/AccountsListView.swift b/Yattee/Views/Settings/AccountsListView.swift index 0ea1e202..4e992125 100644 --- a/Yattee/Views/Settings/AccountsListView.swift +++ b/Yattee/Views/Settings/AccountsListView.swift @@ -202,8 +202,20 @@ struct AccountsListView: View { @ViewBuilder private func accountRow(_ account: Account, instance: Instance?) -> some View { VStack(alignment: .leading, spacing: 4) { - Text(title(for: account, instance: instance)) - .lineLimit(1) + HStack(spacing: 6) { + // The same checkmark the switcher marks the selected context + // with — this screen offers "Use this account" on every other + // row, and the row it is missing from has to say why. + if isCurrentContext(account) { + Image(systemName: "checkmark") + .font(.caption.weight(.semibold)) + .foregroundStyle(.tint) + .accessibilityLabel(String(localized: "accounts.current")) + } + + Text(title(for: account, instance: instance)) + .lineLimit(1) + } if instance == nil { // An orphan has no status worth reporting: its capabilities are @@ -282,13 +294,23 @@ struct AccountsListView: View { } private func loginSheet(_ login: AccountsListLogin) -> some View { - InstanceLoginView(instance: login.instance) { credential, username in + InstanceLoginView( + instance: login.instance, + initialUsername: storedUsername(for: login.target) + ) { credential, username in guard let appEnvironment else { return false } return AccountSignInHandler(appEnvironment: appEnvironment, instance: login.instance) .storeLogin(credential: credential, username: username, target: login.target) } } + /// The identifier an existing account last signed in with, when this device + /// holds it. A new account has nobody to name yet. + private func storedUsername(for target: AccountLoginTarget) -> String? { + guard let account = target.account else { return nil } + return appEnvironment?.accountCredentialsManager(for: account)?.username(for: account) + } + // MARK: - Presentation /// The orphan's own label, never the validator's: with no instance to diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index 9583699b..3ae3fe51 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -329,13 +329,13 @@ private struct EditRemoteServerContent: View { .presentationCompactAdaptation(.sheet) #if os(tvOS) .fullScreenCover(item: $loginTarget) { target in - InstanceLoginView(instance: instance) { credential, username in + InstanceLoginView(instance: instance, initialUsername: storedUsername(for: target)) { credential, username in storeLogin(credential: credential, username: username, target: target) } } #else .sheet(item: $loginTarget) { target in - InstanceLoginView(instance: instance) { credential, username in + InstanceLoginView(instance: instance, initialUsername: storedUsername(for: target)) { credential, username in storeLogin(credential: credential, username: username, target: target) } } @@ -417,6 +417,13 @@ private struct EditRemoteServerContent: View { return stored } + /// The identifier an existing account last signed in with, when this device + /// holds it. A new account has nobody to name yet. + private func storedUsername(for target: AccountLoginTarget) -> String? { + guard let account = target.account else { return nil } + return appEnvironment?.accountCredentialsManager(for: account)?.username(for: account) + } + private func loadServerInfo() async { guard instance.type == .yatteeServer, let appEnvironment else { return } diff --git a/Yattee/Views/Settings/InstanceAccountsSection.swift b/Yattee/Views/Settings/InstanceAccountsSection.swift index 44c73de9..5a0ce72c 100644 --- a/Yattee/Views/Settings/InstanceAccountsSection.swift +++ b/Yattee/Views/Settings/InstanceAccountsSection.swift @@ -85,8 +85,20 @@ struct InstanceAccountsSection: View { @ViewBuilder private func accountRow(_ account: Account) -> some View { VStack(alignment: .leading, spacing: 4) { - Text(displayName(for: account)) - .lineLimit(1) + HStack(spacing: 6) { + // The same checkmark the switcher marks the selected context + // with — the row's menu offers "Use this account" on every + // other account, and the row it is missing from has to say why. + if isCurrentContext(account) { + Image(systemName: "checkmark") + .font(.caption.weight(.semibold)) + .foregroundStyle(.tint) + .accessibilityLabel(String(localized: "accounts.current")) + } + + Text(displayName(for: account)) + .lineLimit(1) + } AccountStatusBadge(status: status(for: account)) } diff --git a/Yattee/Views/Settings/InstanceLoginView.swift b/Yattee/Views/Settings/InstanceLoginView.swift index 9e33cf08..d3cf1322 100644 --- a/Yattee/Views/Settings/InstanceLoginView.swift +++ b/Yattee/Views/Settings/InstanceLoginView.swift @@ -19,6 +19,14 @@ struct InstanceLoginView: View { /// one would be a dead end there. var isOptionalStep: Bool = false + /// The identifier to start the form with, when there is one to start from. + /// + /// Signing in again is almost always the same person proving themselves + /// once more, and the identifier they used is already on the device — beside + /// the credential, in the Keychain. Retyping it is work the app can do. + /// A new login has nobody to name and passes nothing. + var initialUsername: String? + /// Receives the credential **and** the identifier the user signed in with, /// and reports whether it was stored. /// @@ -143,30 +151,50 @@ struct InstanceLoginView: View { #if os(iOS) .scrollDismissesKeyboard(.interactively) #endif + .onAppear { + // Only ever fills an untouched field: the sheet's body runs again + // for every keystroke, and a prefill that reasserted itself would + // fight the user editing it. + if let initialUsername, !initialUsername.isEmpty, username.isEmpty { + username = initialUsername + } + } } // MARK: - Computed Properties - /// Returns the appropriate label for the username field based on instance type. + /// What the first field is called on this backend. + /// + /// Named per backend rather than genericised: Invidious signs in by email, + /// and calling that "Username" is how a user ends up typing the wrong thing. + /// Every case is spelled out so a backend that gains accounts — + /// `AccountCapabilities.supportsAccounts` is what decides that — arrives + /// here rather than falling through to somebody else's wording. private var usernameFieldLabel: String { switch instance.type { case .piped: return String(localized: "login.username") case .invidious: return String(localized: "login.email") - default: + case .yatteeServer: + return String(localized: "login.username") + case .peertube: return String(localized: "login.username") } } - /// Returns the appropriate footer text based on instance type. + /// What this form is signing in to, said in the user's terms. private var footerText: String { switch instance.type { case .piped: return String(localized: "login.footer.pipedAccount \(instance.displayName)") case .invidious: return String(localized: "login.footer.invidiousAccount \(instance.displayName)") - default: + case .yatteeServer: + return String(localized: "login.footer.yatteeServerAccount \(instance.displayName)") + case .peertube: + // No account concept at all, so no sign-in is offered — and if one + // ever were, an empty footer explains nothing. return "" } } diff --git a/Yattee/Views/Settings/InstancePickerSheet.swift b/Yattee/Views/Settings/InstancePickerSheet.swift deleted file mode 100644 index bc9fa3cb..00000000 --- a/Yattee/Views/Settings/InstancePickerSheet.swift +++ /dev/null @@ -1,233 +0,0 @@ -// -// InstancePickerSheet.swift -// Yattee -// -// Quick instance picker sheet for switching between backends. -// - -import SwiftUI - -struct InstancePickerSheet: View { - @Environment(\.dismiss) private var dismiss - @Environment(\.appEnvironment) private var appEnvironment - - private var instancesManager: InstancesManager? { appEnvironment?.instancesManager } - - var body: some View { - NavigationStack { - List { - if let instancesManager { - // Enabled instances - Section(String(localized: "instance.enabled")) { - ForEach(instancesManager.enabledInstances, id: \.url) { instance in - let isActive = instance.id == instancesManager.activeInstance?.id - PickerInstanceRow(instance: instance, isEnabled: true, isPrimary: isActive) { - instancesManager.setActive(instance) - } - } - } - - // Disabled instances - let disabledInstances = instancesManager.instances.filter { !$0.isEnabled } - if !disabledInstances.isEmpty { - Section(String(localized: "instance.disabled")) { - ForEach(disabledInstances, id: \.url) { instance in - PickerInstanceRow(instance: instance, isEnabled: false, isPrimary: false) { - instancesManager.toggleEnabled(instance) - } - } - } - } - - // Add instance - Section { - NavigationLink { - QuickAddInstanceView() - } label: { - Label(String(localized: "instance.add"), systemImage: "plus.circle") - } - } - } - } - .navigationTitle(String(localized: "instance.picker.title")) - #if os(iOS) - .navigationBarTitleDisplayMode(.inline) - #endif - .toolbar { - sheetCloseToolbarItem { dismiss() } - } - } - .presentationDetents([.medium, .large]) - #if os(macOS) - .frame(minWidth: 450, minHeight: 500) - #endif - } -} - -// MARK: - Picker Instance Row - -private struct PickerInstanceRow: View { - let instance: Instance - let isEnabled: Bool - let isPrimary: Bool - let onTap: () -> Void - - var body: some View { - HStack(spacing: 12) { - // Type icon - Image(systemName: instanceIcon) - .font(.title3) - .foregroundStyle(isEnabled ? .primary : .secondary) - .frame(width: 32) - - // Info - VStack(alignment: .leading, spacing: 2) { - Text(instance.displayName) - .font(.body) - .foregroundStyle(isEnabled ? .primary : .secondary) - - HStack(spacing: 4) { - Text(instance.type.displayName) - .font(.caption) - .foregroundStyle(.secondary) - - Text(verbatim: "•") - .font(.caption) - .foregroundStyle(.tertiary) - - Text(instance.url.host ?? "") - .font(.caption) - .foregroundStyle(.tertiary) - } - } - - Spacer() - - // Checkmark for primary, or plus for disabled - if isEnabled { - if isPrimary { - Image(systemName: "checkmark") - .foregroundStyle(.tint) - } - } else { - Image(systemName: "plus.circle") - .foregroundStyle(.secondary) - } - } - .contentShape(Rectangle()) - .onTapGesture { - onTap() - } - } - - private var instanceIcon: String { - switch instance.type { - case .invidious: - return "play.rectangle" - case .piped: - return "waveform" - case .peertube: - return "film" - case .yatteeServer: - return "server.rack" - } - } -} - -// MARK: - Quick Add Instance View - -private struct QuickAddInstanceView: View { - @Environment(\.dismiss) private var dismiss - @Environment(\.appEnvironment) private var appEnvironment - - @State private var urlString = "" - @State private var isDetecting = false - @State private var errorMessage: String? - - var body: some View { - Form { - Section { - TextField(String(localized: "instance.url.placeholder"), text: $urlString) - .textContentType(.URL) - .autocorrectionDisabled() - #if os(iOS) - .textInputAutocapitalization(.never) - .keyboardType(.URL) - #endif - } header: { - Text(String(localized: "instance.url.header")) - } footer: { - Text(String(localized: "instance.url.footer")) - } - - if let error = errorMessage { - Section { - Text(error) - .foregroundStyle(.red) - } - } - } - #if os(iOS) - .scrollDismissesKeyboard(.interactively) - #endif - .navigationTitle(String(localized: "instance.add")) - #if os(iOS) - .navigationBarTitleDisplayMode(.inline) - #endif - .toolbar { - ToolbarItem(placement: .confirmationAction) { - Button { - addInstance() - } label: { - if isDetecting { - ProgressView() - .controlSize(.small) - } else { - Text(String(localized: "common.add")) - } - } - .disabled(urlString.isEmpty || isDetecting) - } - } - } - - private func addInstance() { - guard let appEnvironment else { return } - - // Normalize URL - var normalizedURL = urlString.trimmingCharacters(in: .whitespacesAndNewlines) - if !normalizedURL.hasPrefix("http://") && !normalizedURL.hasPrefix("https://") { - normalizedURL = "https://" + normalizedURL - } - - guard let url = URL(string: normalizedURL) else { - errorMessage = String(localized: "instance.error.invalidURL") - return - } - - isDetecting = true - errorMessage = nil - - Task { - let type = await appEnvironment.instanceDetector.detect(url: url) - - await MainActor.run { - if let type { - let instance = Instance(type: type, url: url, name: url.host ?? normalizedURL, isEnabled: true) - appEnvironment.instancesManager.addInstance(instance) - dismiss() - } else { - errorMessage = String(localized: "instance.error.detectionFailed") - isDetecting = false - } - } - } - } -} - -// MARK: - Preview - -#Preview { - InstancePickerSheet() - .appEnvironment(.preview) -} diff --git a/Yattee/Views/Settings/SourceRow.swift b/Yattee/Views/Settings/SourceRow.swift index 1840262f..e5354f6e 100644 --- a/Yattee/Views/Settings/SourceRow.swift +++ b/Yattee/Views/Settings/SourceRow.swift @@ -39,6 +39,19 @@ struct SourceRow: View { return appEnvironment?.needsCredentials(for: instance) ?? false } + /// Whether one of this source's logins is being refused by the server. + /// + /// The instance-level statuses know nothing about accounts, so a rejected + /// login would otherwise show up nowhere the user looks for their sources. + private var hasRejectedAccount: Bool { + guard case .remoteServer(let instance) = source, + let accountsManager = appEnvironment?.accountsManager else { return false } + + return accountsManager.accounts(forInstance: instance.id).contains { + accountsManager.status(for: $0.id) == .credentialRejected + } + } + var body: some View { #if os(tvOS) Button(action: onEdit) { @@ -99,7 +112,16 @@ struct SourceRow: View { @ViewBuilder private var statusView: some View { - if needsPassword || needsCredentials { + // A refused login comes first: it names one thing the user has to do, + // and two warnings stacked on one row say less than either alone. + if hasRejectedAccount { + Label( + AccountStatusBadge.text(for: .credentialRejected), + systemImage: "person.crop.circle.badge.exclamationmark" + ) + .font(.caption2) + .foregroundStyle(AccountStatusBadge.color(for: .credentialRejected)) + } else if needsPassword || needsCredentials { Label(String(localized: "sources.status.authRequired"), systemImage: "key.fill") .font(.caption2) .foregroundStyle(.orange) diff --git a/Yattee/Views/Settings/SourcesListView.swift b/Yattee/Views/Settings/SourcesListView.swift index 9b2e5e34..2f32f357 100644 --- a/Yattee/Views/Settings/SourcesListView.swift +++ b/Yattee/Views/Settings/SourcesListView.swift @@ -415,9 +415,31 @@ struct SourcesListView: View { appEnvironment?.needsCredentials(for: instance) ?? false } + /// Whether one of this source's logins is being refused by the server. + /// + /// The instance-level statuses below know nothing about accounts, so a + /// rejected login would otherwise show up nowhere the user looks for their + /// sources. + private func hasRejectedAccount(_ instance: Instance) -> Bool { + guard let accountsManager = appEnvironment?.accountsManager else { return false } + + return accountsManager.accounts(forInstance: instance.id).contains { + accountsManager.status(for: $0.id) == .credentialRejected + } + } + @ViewBuilder private func instanceStatusView(for instance: Instance) -> some View { - if needsCredentials(instance) { + // A refused login comes first: it names one thing the user has to do, + // and two warnings stacked on one row say less than either alone. + if hasRejectedAccount(instance) { + Label( + AccountStatusBadge.text(for: .credentialRejected), + systemImage: "person.crop.circle.badge.exclamationmark" + ) + .font(.caption2) + .foregroundStyle(AccountStatusBadge.color(for: .credentialRejected)) + } else if needsCredentials(instance) { Label(String(localized: "sources.status.authRequired"), systemImage: "key.fill") .font(.caption2) .foregroundStyle(.orange) diff --git a/YatteeTests/AccountsScreenTests.swift b/YatteeTests/AccountsScreenTests.swift index b7c89e0f..aa90dfc1 100644 --- a/YatteeTests/AccountsScreenTests.swift +++ b/YatteeTests/AccountsScreenTests.swift @@ -319,3 +319,140 @@ struct AccountStatusBadgeTests { #expect(AccountStatusBadge.color(for: .checking) == .secondary) } } + +// MARK: - Switcher labels + +@Suite("Data context switcher labels") +@MainActor +struct DataContextSwitcherLabelTests { + private func makeDefaults() -> UserDefaults { + UserDefaults(suiteName: "yattee.tests.\(UUID().uuidString)")! + } + + private func makeScope() -> CredentialIdentityScope { + CredentialIdentityScope(fixedScope: "identity-\(UUID().uuidString)") + } + + /// The real managers a label is built from, with storage of their own. + private func makeFixture() -> (AccountsManager, InstancesManager, DataContextValidator) { + let instances = makeTestInstancesManager() + let accounts = AccountsManager( + identityScope: makeScope(), + instancesManager: instances, + defaults: makeDefaults() + ) + let validator = DataContextValidator( + accountsManager: accounts, + instancesManager: instances, + toastManager: ToastManager() + ) + return (accounts, instances, validator) + } + + private func makeInstance(_ host: String) -> Instance { + Instance(type: .invidious, url: URL(string: "https://\(host)")!) + } + + private func title( + _ account: Account, + _ accounts: AccountsManager, + _ validator: DataContextValidator, + usernames: [UUID: String] = [:] + ) -> String { + DataContextSwitcherLabels.title( + for: .account(account.id), + accountsManager: accounts, + validator: validator, + username: { usernames[$0.id] } + ) + } + + @Test("A name nobody shares is left alone") + func uniqueNameIsPlain() { + let (accounts, instances, validator) = makeFixture() + let instance = makeInstance("inv.example.com") + instances.add(instance) + + let account = accounts.addAccount(instanceID: instance.id, displayName: "Work") + accounts.setStatus(.signedIn, for: account.id) + + #expect(title(account, accounts, validator) == "Work") + } + + @Test("Two sources sharing a name are told apart by their host") + func sharedNameOnTwoSourcesGetsTheHost() { + let (accounts, instances, validator) = makeFixture() + let first = makeInstance("one.example.com") + let second = makeInstance("two.example.com") + instances.add(first) + instances.add(second) + + let a = accounts.addAccount(instanceID: first.id, displayName: "Work") + let b = accounts.addAccount(instanceID: second.id, displayName: "Work") + accounts.setStatus(.signedIn, for: a.id) + accounts.setStatus(.signedIn, for: b.id) + + #expect(title(a, accounts, validator) == "Work · one.example.com") + #expect(title(b, accounts, validator) == "Work · two.example.com") + } + + @Test("Two logins on one source are told apart by who they are") + func sharedNameOnOneSourceGetsTheUsername() { + let (accounts, instances, validator) = makeFixture() + let instance = makeInstance("inv.example.com") + instances.add(instance) + + let a = accounts.addAccount(instanceID: instance.id, displayName: "Work") + let b = accounts.addAccount(instanceID: instance.id, displayName: "Work") + accounts.setStatus(.signedIn, for: a.id) + accounts.setStatus(.signedIn, for: b.id) + + let usernames = [a.id: "alice", b.id: "bob"] + + // The host is what they have in common, so a host suffix would leave + // the user choosing between two identical rows. + #expect(title(a, accounts, validator, usernames: usernames) == "Work · alice@inv.example.com") + #expect(title(b, accounts, validator, usernames: usernames) == "Work · bob@inv.example.com") + } + + @Test("A status the user has to act on is named in the label") + func statusIsAppended() { + let (accounts, instances, validator) = makeFixture() + let instance = makeInstance("inv.example.com") + instances.add(instance) + + let account = accounts.addAccount(instanceID: instance.id, displayName: "Work") + accounts.setStatus(.credentialRejected, for: account.id) + + let label = title(account, accounts, validator) + #expect(label.hasPrefix("Work")) + #expect(label.contains(AccountStatusBadge.text(for: .credentialRejected))) + } + + @Test("The identifier is preferred, the host is the fallback") + func disambiguationPrefersTheIdentifier() { + #expect( + DataContextSwitcherLabels.disambiguated("Work", username: "alice", host: "inv.example.com") + == "Work · alice@inv.example.com" + ) + #expect( + DataContextSwitcherLabels.disambiguated("Work", username: nil, host: "inv.example.com") + == "Work · inv.example.com" + ) + #expect(DataContextSwitcherLabels.disambiguated("Work", username: "alice", host: nil) == "Work · alice") + } + + @Test("A name that already carries the host does not repeat it") + func hostIsNotRepeated() { + #expect( + DataContextSwitcherLabels.disambiguated("inv.example.com", username: nil, host: "inv.example.com") + == "inv.example.com" + ) + + // The identifier still adds something the name does not have. + #expect( + DataContextSwitcherLabels.disambiguated("inv.example.com", username: "alice", host: "inv.example.com") + == "inv.example.com · alice@inv.example.com" + ) + } +} diff --git a/YatteeTests/InstancesManagerTests.swift b/YatteeTests/InstancesManagerTests.swift index 16d65213..ae1837da 100644 --- a/YatteeTests/InstancesManagerTests.swift +++ b/YatteeTests/InstancesManagerTests.swift @@ -71,6 +71,30 @@ struct InstanceFilteringTests { #expect(enabledInstances.count == 1) #expect(enabledInstances.first?.url.host == "enabled.example.com") } + + @Test("A disabled instance leaves the enabled listing without leaving the app") + func disablingPartitionsRatherThanRemoves() { + let manager = makeTestInstancesManager() + let instance = Instance(type: .invidious, url: URL(string: "https://inv.example.com")!) + manager.add(instance) + + #expect(manager.enabledInstances.map(\.id) == [instance.id]) + + manager.toggleEnabled(instance) + + // Every listing in the Sources tab is filtered to `enabledInstances`, + // so this is exactly the disappearance the recovery section undoes — + // and it can only undo it because the instance is still here. + #expect(manager.enabledInstances.isEmpty) + #expect(manager.instances.filter { !$0.isEnabled }.map(\.id) == [instance.id]) + + // Turning it back on takes the stale copy the row was drawn from: the + // row's instance is a value, and it stopped matching the stored one the + // moment it was switched off. + manager.toggleEnabled(instance) + + #expect(manager.enabledInstances.map(\.id) == [instance.id]) + } } // MARK: - Instance Type Tests