Let every menu mark the current context and a disabled source admit it

MAC1: switcher rows are Toggles on macOS so the checkmark really renders,
and the settings row names its context via LabeledContent with a tooltip.
MAC4: accessible names are words, never raw SF Symbol identifiers.
MAC14: new displayStatus(for:) surfaces instanceDisabled on account rows,
badges, and the switcher tooltip without disturbing status(for:) consumers.
MAC16: disambiguated labels guard the username branch and fall back to the
source name only when rows would still collide.
Also carries the new Localizable strings for the whole macOS fix batch.
This commit is contained in:
Arkadiusz Fal
2026-09-11 08:02:26 +02:00
parent cf07236cf7
commit e661635079
8 changed files with 404 additions and 51 deletions

View File

@@ -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.

View File

@@ -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" : {

View File

@@ -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)
}
}

View File

@@ -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
}
}

View File

@@ -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

View File

@@ -367,6 +367,7 @@ struct UnifiedPlaylistDetailView: View {
}
.frame(width: 26, height: 15)
.clipShape(RoundedRectangle(cornerRadius: 3))
.accessibilityHidden(true)
Text(navigationTitleText)
.font(.headline)

View File

@@ -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] = []

View File

@@ -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))"
)
}
}