diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 89ebfd80..8fbedc2f 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -24,6 +24,11 @@ enum AccountStatus: Hashable, Sendable { /// The server rejected the credential. Account-scoped: a rejected login on /// one account never disables the instance or its sibling accounts. case credentialRejected + /// The account's source has been switched off. Not a credential state at + /// all — the login is untouched and comes back the moment the source is + /// turned on again — but nothing may be read from or written to it + /// meanwhile, so no surface may present the account as usable. + case instanceDisabled /// The server could not be reached. Never invalidates a credential. case unreachable } @@ -859,6 +864,20 @@ final class AccountsManager { accountStatuses[accountID] ?? (identityScope.isResolved ? .credentialMissing : .checking) } + /// The status as the user is shown it, which is not the same question. + /// + /// `status(for:)` answers what the *credential* is doing, and a source the + /// user switched off leaves that answer untouched — deliberately, so + /// turning the source back on restores whatever the login really was. What + /// no list may do is report that answer on its own: an account on a + /// disabled source cannot be read from, cannot be written to and is not + /// offered by the switcher, so a row saying "Signed in" beside it is the + /// app claiming health it will not deliver. + func displayStatus(for account: Account) -> AccountStatus { + guard instance(for: account)?.isEnabled != false else { return .instanceDisabled } + return status(for: account.id) + } + /// Asked to confirm an auth-shaped failure before anything is concluded /// from it. `nil` — tests, and the window before wiring completes — means /// nothing is concluded at all, which is the safe direction. diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index bcadea0b..66f9e079 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -420,6 +420,17 @@ } } }, + "accounts.status.instanceDisabled" : { + "comment" : "Status of an account whose source has been switched off", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Source switched off" + } + } + } + }, "accounts.status.instanceMissing" : { "localizations" : { "en" : { @@ -922,6 +933,17 @@ } } }, + "channel.menu.title" : { + "comment" : "Accessible name of the channel toolbar menu", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Subscription Options" + } + } + } + }, "channel.menu.unsubscribe" : { "localizations" : { "en" : { @@ -1004,6 +1026,28 @@ } } }, + "channel.subscribe.destination.subscribed" : { + "comment" : "Toast title after subscribing on a named destination", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Subscribed" + } + } + } + }, + "channel.subscribe.destination.unsubscribed" : { + "comment" : "Toast title after unsubscribing on a named destination", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Unsubscribed" + } + } + } + }, "channel.subscribe.error.title" : { "comment" : "Toast title when subscription fails", "localizations" : { @@ -4288,6 +4332,28 @@ } } }, + "home.history.metadataRetry" : { + "comment" : "Context menu item that asks again for a history row's metadata", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Try Again" + } + } + } + }, + "home.history.metadataUnavailable" : { + "comment" : "Title for a history row whose video details could not be fetched", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Details unavailable" + } + } + } + }, "home.history.remaining %@" : { "localizations" : { "en" : { @@ -5617,6 +5683,39 @@ } } }, + "login.footer.accountUnusable" : { + "comment" : "Footer of the account section when a listed account cannot be used and no more specific wording applies", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "An account here cannot be used right now. Its status is shown above." + } + } + } + }, + "login.footer.credentialMissing" : { + "comment" : "Footer of the account section when a listed account has no saved login on this device", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "An account here has no saved login on this device yet. Sign in to use it." + } + } + } + }, + "login.footer.credentialRejected" : { + "comment" : "Footer of the account section when the server rejected a listed account's login", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The server rejected an account's login. Sign in again to use it." + } + } + } + }, "login.footer.invidiousAccount %@" : { "localizations" : { "en" : { @@ -5670,6 +5769,17 @@ } } }, + "login.footer.unreachable" : { + "comment" : "Footer of the account section when the server cannot be reached", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The server could not be reached, so its accounts cannot be used right now." + } + } + } + }, "login.footer.yatteeServerAccount %@" : { "localizations" : { "en" : { @@ -18458,6 +18568,28 @@ } } }, + "subscriptions.feedUnavailable.description" : { + "comment" : "Feed state while the selected account cannot be reached", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The feed will load again once this account is available." + } + } + } + }, + "subscriptions.feedUnavailable.title" : { + "comment" : "Feed state while the selected account cannot be reached", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Feed Unavailable" + } + } + } + }, "subscriptions.goToChannel" : { "comment" : "Context menu action to go to channel", "localizations" : { diff --git a/Yattee/Services/Subscriptions/DataContextValidator.swift b/Yattee/Services/Subscriptions/DataContextValidator.swift index 2f07d215..8f86910b 100644 --- a/Yattee/Services/Subscriptions/DataContextValidator.swift +++ b/Yattee/Services/Subscriptions/DataContextValidator.swift @@ -112,6 +112,7 @@ final class DataContextValidator { case .checking: return .temporarilyUnavailable(.checking) case .credentialMissing: return .temporarilyUnavailable(.credentialMissing) case .credentialRejected: return .temporarilyUnavailable(.credentialRejected) + case .instanceDisabled: return .temporarilyUnavailable(.instanceDisabled) case .unreachable: return .temporarilyUnavailable(.unreachable) } } diff --git a/Yattee/Views/Components/AccountStatusBadge.swift b/Yattee/Views/Components/AccountStatusBadge.swift index 90f1560c..7cc5e3e0 100644 --- a/Yattee/Views/Components/AccountStatusBadge.swift +++ b/Yattee/Views/Components/AccountStatusBadge.swift @@ -28,19 +28,23 @@ struct AccountStatusBadge: View { case .signedIn: String(localized: "accounts.status.signedIn") case .credentialMissing: String(localized: "accounts.status.credentialMissing") case .credentialRejected: String(localized: "accounts.status.credentialRejected") + case .instanceDisabled: String(localized: "accounts.status.instanceDisabled") case .unreachable: String(localized: "accounts.status.unreachable") } } /// Only the states the user has to act on are coloured. A credential that /// never arrived on this device is ordinary — tvOS never receives them at - /// all — so it reads as secondary rather than as a fault. + /// all — so it reads as secondary rather than as a fault. A switched-off + /// source is the user's own doing, and the banner already reads it as + /// grey. static func color(for status: AccountStatus) -> Color { switch status { case .checking: .secondary case .signedIn: .secondary case .credentialMissing: .secondary case .credentialRejected: .red + case .instanceDisabled: .secondary case .unreachable: .orange } } diff --git a/Yattee/Views/Components/DataContextSwitcherMenu.swift b/Yattee/Views/Components/DataContextSwitcherMenu.swift index a68d510d..338f2a03 100644 --- a/Yattee/Views/Components/DataContextSwitcherMenu.swift +++ b/Yattee/Views/Components/DataContextSwitcherMenu.swift @@ -9,7 +9,7 @@ import SwiftUI // MARK: - Rows -/// The switchable contexts, as buttons. +/// The switchable contexts, as rows. /// /// Shared by every presentation of the switcher so a context that is offered in /// one place is offered — and named — identically in all of them. @@ -23,30 +23,50 @@ struct DataContextSwitcherRows: View { var body: some View { ForEach(validator?.availableContexts ?? [.local], id: \.self) { context in - Button { - appEnvironment?.switchDataContext(to: context) - onSwitch?() - } label: { - rowLabel(for: context) - } - #if os(tvOS) - .buttonStyle(TVSettingsButtonStyle()) - #endif + row(for: context) } } @ViewBuilder - private func rowLabel(for context: DataContext) -> some View { + private func row(for context: DataContext) -> some View { + // A menu row is a single label — a `VStack` inside one renders as + // nothing on macOS — so the status travels in the title rather than as + // a second line. let title = DataContextSwitcherLabels.title(for: context, in: appEnvironment) - // A menu row is a single label on macOS — a `VStack` inside one renders - // as nothing there — so the status travels in the title rather than as - // a second line. - if isCurrent(context) { - Label(title, systemImage: "checkmark") - } else { + #if os(macOS) + // The mark has to be the menu item's own. A `Label`'s `systemImage` is + // dropped by a macOS menu row, leaving the selected context + // indistinguishable from the rest; a `Toggle` is what SwiftUI renders + // as a checked item, mark character included. + Toggle(isOn: Binding( + get: { isCurrent(context) }, + set: { isOn in + guard isOn else { return } + switchTo(context) + } + )) { Text(title) } + #else + Button { + switchTo(context) + } label: { + if isCurrent(context) { + Label(title, systemImage: "checkmark") + } else { + Text(title) + } + } + #if os(tvOS) + .buttonStyle(TVSettingsButtonStyle()) + #endif + #endif + } + + private func switchTo(_ context: DataContext) { + appEnvironment?.switchDataContext(to: context) + onSwitch?() } private func isCurrent(_ context: DataContext) -> Bool { @@ -91,15 +111,18 @@ enum DataContextSwitcherLabels { var title = name - if isAmbiguous(name, accountsManager: accountsManager, validator: validator) { + let peers = peers(sharing: name, accountsManager: accountsManager, validator: validator) + if peers.count > 1 { title = disambiguated( name, - username: username(account), - host: accountsManager.instance(for: account)?.url.host + for: account, + among: peers, + accountsManager: accountsManager, + username: username ) } - let status = accountsManager.status(for: id) + let status = accountsManager.displayStatus(for: account) guard status != .signedIn else { return title } return "\(title) — \(AccountStatusBadge.text(for: status))" @@ -110,27 +133,76 @@ enum DataContextSwitcherLabels { /// 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)" + /// the identifier is not on this device — tvOS never receives one. Neither + /// is repeated when the name already carries it: the default name for an + /// account *is* `identifier@host`, so appending it unguarded produced two + /// rows reading `X · X`. + /// + /// The source is the last resort and the only one that can name what two + /// logins of the same server user on two copies of the same server differ + /// by — the thing the user chose when adding the second source. + static func disambiguated(_ name: String, username: String?, host: String?, source: String? = nil) -> String { + var parts: [String] = [] + + if let username, !username.isEmpty, !name.localizedCaseInsensitiveContains(username) { + if let host, !host.isEmpty, !name.localizedCaseInsensitiveContains(host) { + parts.append("\(username)@\(host)") + } else { + parts.append(username) + } + } else if let host, !host.isEmpty, !name.localizedCaseInsensitiveContains(host) { + parts.append(host) } - guard let host, !host.isEmpty, !name.localizedCaseInsensitiveContains(host) else { return name } - return "\(name) · \(host)" + if let source, !source.isEmpty, name.localizedCaseInsensitiveCompare(source) != .orderedSame { + parts.append(source) + } + + guard !parts.isEmpty else { return name } + return "\(name) · \(parts.joined(separator: " · "))" + } + + /// The identifier and the host first, and the source only once they have + /// failed: two logins on one server are told apart by who they are, and + /// naming the server as well would add a word that is the same on both + /// rows. It is when that suffix still collides — the same server user + /// signed in on two sources pointing at the same host — that the source is + /// the only thing left that differs. + @MainActor + private static func disambiguated( + _ name: String, + for account: Account, + among peers: [Account], + accountsManager: AccountsManager, + username: (Account) -> String? + ) -> String { + func withoutSource(_ peer: Account) -> String { + disambiguated( + name, + username: username(peer), + host: accountsManager.instance(for: peer)?.url.host + ) + } + + let own = withoutSource(account) + let collides = peers.contains { $0.id != account.id && withoutSource($0) == own } + guard collides else { return own } + + return disambiguated( + name, + username: username(account), + host: accountsManager.instance(for: account)?.url.host, + source: accountsManager.instance(for: account)?.displayName + ) } @MainActor - private static func isAmbiguous( - _ name: String, + private static func peers( + sharing name: String, accountsManager: AccountsManager, validator: DataContextValidator - ) -> Bool { - accountsManager.selectableAccounts - .filter { validator.displayName(for: $0) == name } - .count > 1 + ) -> [Account] { + accountsManager.selectableAccounts.filter { validator.displayName(for: $0) == name } } } @@ -202,17 +274,25 @@ enum DataContextSwitcherGlyph { } /// The switcher's icon, health included. +/// +/// Hidden from assistive technology on purpose: every presentation names the +/// switcher in words beside it, and an unlabelled `Image` announces its symbol +/// — a control that read "Account" while healthy started reading +/// "person.crop.circle.badge.exclamationmark" the moment it badged. struct DataContextSwitcherIcon: View { @Environment(\.appEnvironment) private var appEnvironment var body: some View { let image = Image(systemName: DataContextSwitcherGlyph.systemImage(in: appEnvironment)) - if let tint = DataContextSwitcherGlyph.tint(in: appEnvironment) { - image.foregroundStyle(tint) - } else { - image + Group { + if let tint = DataContextSwitcherGlyph.tint(in: appEnvironment) { + image.foregroundStyle(tint) + } else { + image + } } + .accessibilityHidden(true) } } @@ -233,7 +313,13 @@ struct DataContextSwitcherMenu: View { DataContextSwitcherIcon() } } - .help(DataContextSwitcherLabels.title(for: currentContext, in: appEnvironment)) + .help(contextTitle) + .accessibilityLabel(Text(String(localized: "accounts.switcher.title"))) + .accessibilityValue(Text(contextTitle)) + } + + private var contextTitle: String { + DataContextSwitcherLabels.title(for: currentContext, in: appEnvironment) } private var currentContext: DataContext { @@ -261,20 +347,39 @@ struct DataContextSwitcherSettingsRow: View { .fullScreenCover(isPresented: $showingPicker) { DataContextSwitcherPicker() } + #elseif os(macOS) + // A macOS menu label renders its first text and nothing else — the + // trailing name was dropped at every width, leaving the row silent + // about which account is selected. So the name is the menu's *own* + // label, and the row carries only what names the control. + LabeledContent { + Menu { + DataContextSwitcherRows() + } label: { + Text(contextTitle) + .lineLimit(1) + } + .menuStyle(.borderlessButton) + .help(contextTitle) + .accessibilityLabel(Text(String(localized: "accounts.switcher.title"))) + .accessibilityValue(Text(contextTitle)) + } label: { + Label { + Text(String(localized: "accounts.switcher.title")) + } icon: { + DataContextSwitcherIcon() + } + } #else Menu { DataContextSwitcherRows() } label: { rowLabel } - #if os(macOS) - // Both callers are full-width rows, so the default style's stretching - // is wanted here — only its chrome is not. - .menuStyle(.borderlessButton) - #endif #endif } + #if !os(macOS) private var rowLabel: some View { HStack { Label { @@ -283,11 +388,16 @@ struct DataContextSwitcherSettingsRow: View { DataContextSwitcherIcon() } Spacer(minLength: 8) - Text(DataContextSwitcherLabels.title(for: currentContext, in: appEnvironment)) + Text(contextTitle) .foregroundStyle(.secondary) .lineLimit(1) } } + #endif + + private var contextTitle: String { + DataContextSwitcherLabels.title(for: currentContext, in: appEnvironment) + } private var currentContext: DataContext { appEnvironment?.accountsManager.currentContext ?? .local diff --git a/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift b/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift index 902263f9..805fbd68 100644 --- a/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift +++ b/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift @@ -367,6 +367,7 @@ struct UnifiedPlaylistDetailView: View { } .frame(width: 26, height: 15) .clipShape(RoundedRectangle(cornerRadius: 3)) + .accessibilityHidden(true) Text(navigationTitleText) .font(.headline) diff --git a/Yattee/Views/Settings/AccountsListView.swift b/Yattee/Views/Settings/AccountsListView.swift index f9206e0b..3f30b6f1 100644 --- a/Yattee/Views/Settings/AccountsListView.swift +++ b/Yattee/Views/Settings/AccountsListView.swift @@ -180,7 +180,17 @@ struct AccountsListView: View { // mentions it. .id(credentialRevision) } header: { - Text(instance.displayName) + HStack(spacing: 6) { + Text(instance.displayName) + + // Said for the source as well as on each of its rows: without + // it the section reads like a healthy server whose logins + // happen to be unusable. + if !instance.isEnabled { + Text(String(localized: "sources.status.disabled")) + .foregroundStyle(.secondary) + } + } } } @@ -355,8 +365,11 @@ struct AccountsListView: View { return .forAccount(account, resolvedBy: validator) } + /// The status the user is shown, not the credential's: a login on a source + /// that has been switched off is not usable, whatever the Keychain says, + /// and this screen already withholds "Use This Account" from it. private func status(for account: Account) -> AccountStatus { - accountsManager?.status(for: account.id) ?? .checking + accountsManager?.displayStatus(for: account) ?? .checking } /// Asked of the validator rather than recomputed here: an account on a @@ -374,7 +387,13 @@ struct AccountsListView: View { /// What the backend behind this account lets it do. Deliberately a short /// sentence rather than a row of glyphs — the difference between two Piped /// logins is who they are, not what they support. + /// + /// Withheld while the source is switched off: none of it can be done until + /// the user turns the source back on, and a row promising subscriptions, + /// playlists and history directly under "Source switched off" contradicts + /// the line above it. private func capabilityHints(for account: Account) -> String? { + guard status(for: account) != .instanceDisabled else { return nil } guard let capabilities = accountsManager?.capabilities(for: account) else { return nil } var parts: [String] = [] diff --git a/YatteeTests/AccountsScreenTests.swift b/YatteeTests/AccountsScreenTests.swift index bc7fd0fa..faa17b7f 100644 --- a/YatteeTests/AccountsScreenTests.swift +++ b/YatteeTests/AccountsScreenTests.swift @@ -490,10 +490,77 @@ struct DataContextSwitcherLabelTests { == "inv.example.com" ) - // The identifier still adds something the name does not have. + // The identifier still adds something the name does not have — the + // host it is usually paired with does not. #expect( DataContextSwitcherLabels.disambiguated("inv.example.com", username: "alice", host: "inv.example.com") - == "inv.example.com · alice@inv.example.com" + == "inv.example.com · alice" + ) + } + + @Test("A name that already is the identifier and the host is not doubled") + func defaultNameIsNotRepeated() { + #expect( + DataContextSwitcherLabels.disambiguated( + "alice@inv.example.com", + username: "alice", + host: "inv.example.com" + ) == "alice@inv.example.com" + ) + + #expect( + DataContextSwitcherLabels.disambiguated( + "alice@inv.example.com", + username: "alice", + host: "inv.example.com", + source: "Backup" + ) == "alice@inv.example.com · Backup" + ) + + // The source names nothing the name does not already say. + #expect( + DataContextSwitcherLabels.disambiguated("Work", username: nil, host: nil, source: "Work") == "Work" + ) + } + + @Test("The same login on two copies of one source is told apart by the source") + func duplicateSourceAccountsAreToldApart() { + let (accounts, instances, validator) = makeFixture() + var first = makeInstance("inv.example.com") + first.name = "Invidious" + var second = makeInstance("inv.example.com") + second.name = "Backup" + instances.add(first) + instances.add(second) + + let a = accounts.addAccount(instanceID: first.id, displayName: "alice@inv.example.com") + let b = accounts.addAccount(instanceID: second.id, displayName: "alice@inv.example.com") + accounts.setStatus(.signedIn, for: a.id) + accounts.setStatus(.signedIn, for: b.id) + + let usernames = [a.id: "alice", b.id: "alice"] + + let labelA = title(a, accounts, validator, usernames: usernames) + let labelB = title(b, accounts, validator, usernames: usernames) + + #expect(labelA == "alice@inv.example.com · Invidious") + #expect(labelB == "alice@inv.example.com · Backup") + #expect(labelA != labelB) + } + + @Test("An account on a switched-off source does not read as signed in") + func disabledSourceIsNamedInTheLabel() { + let (accounts, instances, validator) = makeFixture() + var instance = makeInstance("inv.example.com") + instance.isEnabled = false + instances.add(instance) + + let account = accounts.addAccount(instanceID: instance.id, displayName: "Work") + accounts.setStatus(.signedIn, for: account.id) + + #expect( + title(account, accounts, validator) + == "Work — \(AccountStatusBadge.text(for: .instanceDisabled))" ) } }