diff --git a/Yattee/Views/Channel/ChannelView.swift b/Yattee/Views/Channel/ChannelView.swift index f53233d9..3460bc05 100644 --- a/Yattee/Views/Channel/ChannelView.swift +++ b/Yattee/Views/Channel/ChannelView.swift @@ -34,6 +34,10 @@ struct ChannelView: View { /// previous one is dropped instead of answering for the channel on screen. @State private var subscriptionGeneration = 0 @State private var showingUnsubscribeConfirmation = false + /// What the destination rows know, kept here rather than inside the menu: + /// nothing in a platform menu's content is ever given a lifecycle to load + /// from. + @State private var destinationMembership = SubscriptionDestinationMembershipLoader() #if os(tvOS) @State private var showingSubscriptionDestinations = false #endif @@ -192,6 +196,15 @@ struct ChannelView: View { .task { await loadChannel() } + // Asked for here, where the answers arrive before the menu is built: + // the rows themselves live inside a platform menu, which runs no + // lifecycle of its own, and a mark that is not known by the time the + // menu opens is a mark the user never sees. + .task(id: channelToSubscribe?.id) { + guard let channelToSubscribe, + SubscriptionDestinations.areOffered(in: appEnvironment) else { return } + await destinationMembership.load(for: channelToSubscribe, in: appEnvironment) + } .onAppear { // Update Handoff activity for this specific channel appEnvironment?.handoffManager.updateActivity(for: .channel(channelID, source)) @@ -825,7 +838,10 @@ struct ChannelView: View { } .buttonStyle(.bordered) .fullScreenCover(isPresented: $showingSubscriptionDestinations) { - SubscriptionDestinationPicker(channel: channelToSubscribe) { context, subscribed in + SubscriptionDestinationPicker( + channel: channelToSubscribe, + membership: destinationMembership + ) { context, subscribed in destinationDidToggle(context, subscribed: subscribed) } } @@ -1249,7 +1265,10 @@ struct ChannelView: View { // never presented there. #if !os(tvOS) if let channelToSubscribe { - SubscriptionDestinationMenu(channel: channelToSubscribe) { context, subscribed in + SubscriptionDestinationMenu( + channel: channelToSubscribe, + membership: destinationMembership + ) { context, subscribed in destinationDidToggle(context, subscribed: subscribed) } } @@ -1275,6 +1294,7 @@ struct ChannelView: View { } label: { Image(systemName: channelMenuIcon) } + .accessibilityLabel(Text(String(localized: "channel.menu.title"))) } // MARK: - Subscription Actions diff --git a/Yattee/Views/Components/SubscriptionDestinationMenu.swift b/Yattee/Views/Components/SubscriptionDestinationMenu.swift index 884fff26..de4f5172 100644 --- a/Yattee/Views/Components/SubscriptionDestinationMenu.swift +++ b/Yattee/Views/Components/SubscriptionDestinationMenu.swift @@ -40,6 +40,158 @@ struct SubscriptionDestinationMembership: Equatable { func isSubscribed(on context: DataContext) -> Bool? { known[context] } } +// MARK: - Loading + +/// Asks every destination whether the channel is subscribed there, and holds +/// the answers the rows render. +/// +/// Deliberately owned by the view that *presents* the rows rather than by the +/// rows themselves. A `Menu`'s content is handed to the platform's own menu — +/// an `NSMenu` on macOS — and SwiftUI runs no lifecycle inside one: `.task`, +/// `.onAppear` and `.onReceive` attached to rows there are never called. A +/// round asked for from inside the menu is a round that never happens, leaving +/// every row unknown; unknown reads as "not subscribed", so each row would +/// subscribe again instead of doing the unsubscribe it promises. +@MainActor +@Observable +final class SubscriptionDestinationMembershipLoader { + private(set) var membership = SubscriptionDestinationMembership() + + /// Destinations with a write in flight. + private(set) var busy: Set = [] + + @ObservationIgnored private weak var appEnvironment: AppEnvironment? + @ObservationIgnored private var channel: Channel? + @ObservationIgnored private var reloadTask: Task? + @ObservationIgnored private var observers: [any NSObjectProtocol] = [] + + init() { + // The credential behind an account was replaced, the accounts + // themselves changed, or an instance was switched back on and added a + // destination on the spot: every answer held here describes the logins + // that were there before. + for name: Notification.Name in [.accountCredentialDidChange, .accountsDidChange, .instancesDidChange] { + observers.append( + NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in + Task { @MainActor in self?.reload() } + } + ) + } + } + + deinit { + observers.forEach { NotificationCenter.default.removeObserver($0) } + } + + func isSubscribed(on context: DataContext) -> Bool? { membership.isSubscribed(on: context) } + + func isBusy(_ context: DataContext) -> Bool { busy.contains(context) } + + /// Asks about `channel`, from a place the SwiftUI lifecycle reaches. + /// + /// Answered before the menu is opened rather than while it is open: a + /// platform menu is built from the values it is handed, so a mark that + /// arrives afterwards is a mark the user does not see. + func load(for channel: Channel, in appEnvironment: AppEnvironment?) async { + self.channel = channel + self.appEnvironment = appEnvironment + + reloadTask?.cancel() + await ask() + } + + /// Forgets what is known and asks again. + /// + /// Forgetting first, rather than waiting for the replacement answers: a row + /// toggled in between would otherwise toggle against the previous login's + /// membership and do the opposite of what its mark promises. + private func reload() { + guard channel != nil else { return } + + membership.invalidate() + reloadTask?.cancel() + reloadTask = Task { [weak self] in await self?.ask() } + } + + private func ask() async { + guard let channel, let service = appEnvironment?.subscriptionService else { return } + + membership.invalidate() + let generation = membership.generation + let channelID = channel.id.channelID + + // Serially rather than in a group: each cold destination answers by + // fetching its account's whole list, and firing every instance at once + // is a burst the user never asked for. + for context in SubscriptionDestinations.available(in: appEnvironment) { + let subscribed = await service.isSubscribed(to: channelID, in: context) + membership.record(subscribed, for: context, generation: generation) + } + } + + // MARK: - Toggle + + func toggle( + _ context: DataContext, + of channel: Channel, + in appEnvironment: AppEnvironment?, + onToggled: ((DataContext, Bool) -> Void)? + ) { + guard let appEnvironment else { return } + + self.channel = channel + self.appEnvironment = appEnvironment + + // Unknown counts as not subscribed: the row carries no mark, so + // subscribing is what the user just asked for. Asking the server again + // at press time would be the other way round — it could unsubscribe + // under a row that showed no mark at all. + let wasSubscribed = membership.isSubscribed(on: context) == true + let generation = membership.generation + let channelID = channel.id.channelID + + busy.insert(context) + + Task { + do { + if wasSubscribed { + try await appEnvironment.subscriptionService.unsubscribe(from: channelID, in: context) + } else { + try await appEnvironment.subscriptionService.subscribe(to: channel, in: context) + } + + membership.record(!wasSubscribed, for: context, generation: generation) + onToggled?(context, !wasSubscribed) + + // A menu closes on the press that made the write, so the mark + // the row will carry on its next open is not something the user + // is looking at. The toast is what tells them it landed, and + // where — the main button speaks for the account being browsed, + // which a destination write usually is not. + appEnvironment.toastManager.showSuccess( + String( + localized: wasSubscribed + ? "channel.subscribe.destination.unsubscribed" + : "channel.subscribe.destination.subscribed" + ), + subtitle: DataContextSwitcherLabels.title(for: context, in: appEnvironment) + ) + } catch { + appEnvironment.toastManager.showError( + String( + localized: wasSubscribed + ? "channel.unsubscribe.error.title" + : "channel.subscribe.error.title" + ), + subtitle: error.localizedDescription + ) + } + + busy.remove(context) + } + } +} + // MARK: - Destinations /// Where a channel can be subscribed. @@ -95,6 +247,9 @@ struct SubscriptionDestinationRows: View { /// The channel as it must be stored, with the id the calling view resolved. let channel: Channel + /// What is known about the destinations, asked for outside the menu. + let membership: SubscriptionDestinationMembershipLoader + /// The destination that was toggled and whether it is now subscribed, for /// the view that owns the main subscribe button. var onToggled: ((DataContext, Bool) -> Void)? @@ -102,138 +257,69 @@ struct SubscriptionDestinationRows: View { /// Called after any tap, for presentations that have to close themselves. var onSelect: (() -> Void)? - @State private var membership = SubscriptionDestinationMembership() - @State private var busy: Set = [] - /// Bumped when the logins change, so the answers are asked for again. - @State private var membershipRound = 0 - - /// What a round of membership answers belongs to. - /// - /// The channel alone is not enough: the same channel on the same rows means - /// something different once a credential has been replaced, and the picker - /// deliberately stays open across such a change on tvOS. - private struct MembershipRequest: Equatable { - let channel: Channel.ID - let round: Int - } - var body: some View { ForEach(destinations, id: \.self) { context in - Button { - toggle(context) - onSelect?() - } label: { - rowLabel(for: context) - } - .disabled(busy.contains(context)) - #if os(tvOS) - .buttonStyle(TVSettingsButtonStyle()) - #endif + row(for: context) } - // Membership is only asked for once the rows are on screen: an unwarmed - // provider answers by fetching that account's whole subscription list, - // which is far too much to spend on a menu nobody opened. - .task(id: MembershipRequest(channel: channel.id, round: membershipRound)) { - await loadMembership() - } - // The credential behind an account was replaced, or the accounts - // themselves changed: every checkmark on screen describes the logins - // that were there before. - .onReceive(NotificationCenter.default.publisher(for: .accountCredentialDidChange)) { _ in - invalidateMembership() - } - .onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in - invalidateMembership() - } - // An instance switched on adds a row to `destinations` on the spot, and - // the round that would have asked about it has already run. Unknown - // reads as unsubscribed, so an account the channel is already on gets a - // checkmark-less row whose tap subscribes again instead of doing the - // unsubscribe it promises. - .onReceive(NotificationCenter.default.publisher(for: .instancesDidChange)) { _ in - invalidateMembership() - } - } - - /// Forgets what is known and asks again. - /// - /// Forgetting first, rather than waiting for the replacement answers: a row - /// toggled in between would otherwise toggle against the previous login's - /// membership and do the opposite of what its checkmark promises. - private func invalidateMembership() { - membership.invalidate() - membershipRound += 1 } @ViewBuilder - private func rowLabel(for context: DataContext) -> some View { + private func row(for context: DataContext) -> some View { let title = DataContextSwitcherLabels.title(for: context, in: appEnvironment) - if membership.isSubscribed(on: context) == true { - Label(title, systemImage: "checkmark") - } else { + #if os(tvOS) + Button { + toggle(context) + onSelect?() + } label: { + if membership.isSubscribed(on: context) == true { + Label(title, systemImage: "checkmark") + } else { + Text(title) + } + } + .buttonStyle(TVSettingsButtonStyle()) + .disabled(isRefusing(context)) + #else + // The mark is the menu's own, not a drawn glyph: an image inside a + // macOS menu row is dropped, and a toggle is what both platforms render + // — and report to VoiceOver — as a checked row. + Toggle(isOn: subscribed(on: context)) { Text(title) } + .disabled(isRefusing(context)) + #endif + } + + private func subscribed(on context: DataContext) -> Binding { + Binding( + get: { membership.isSubscribed(on: context) == true }, + set: { _ in + toggle(context) + onSelect?() + } + ) } private var destinations: [DataContext] { SubscriptionDestinations.available(in: appEnvironment) } - // MARK: - Membership - - private func loadMembership() async { - guard let service = appEnvironment?.subscriptionService else { return } - - membership.invalidate() - let generation = membership.generation - let channelID = channel.id.channelID - - // Serially rather than in a group: each cold destination answers by - // fetching its account's whole list, and firing every instance at once - // is a burst the user never asked for. - for context in destinations { - let subscribed = await service.isSubscribed(to: channelID, in: context) - membership.record(subscribed, for: context, generation: generation) - } + /// Whether the destination would refuse the write it is offering. + /// + /// The same gate the main subscribe button uses, asked about the row's own + /// target rather than the account being browsed. A rejected or unreachable + /// account is listed — it names its state, and hiding it would read as a + /// login that had been deleted — but pressing it can only cost a doomed + /// request and an error toast, so it is not pressable. + private func isRefusing(_ context: DataContext) -> Bool { + if membership.isBusy(context) { return true } + guard let appEnvironment else { return !context.isLocal } + return !appEnvironment.dataContextValidator.validity(of: context).isValid } - // MARK: - Toggle - private func toggle(_ context: DataContext) { - guard let appEnvironment else { return } - - // Unknown counts as not subscribed: the row shows no checkmark, so - // subscribing is what the user just asked for. - let wasSubscribed = membership.isSubscribed(on: context) == true - let generation = membership.generation - let channelID = channel.id.channelID - - busy.insert(context) - - Task { - do { - if wasSubscribed { - try await appEnvironment.subscriptionService.unsubscribe(from: channelID, in: context) - } else { - try await appEnvironment.subscriptionService.subscribe(to: channel, in: context) - } - - membership.record(!wasSubscribed, for: context, generation: generation) - onToggled?(context, !wasSubscribed) - } catch { - appEnvironment.toastManager.showError( - String( - localized: wasSubscribed - ? "channel.unsubscribe.error.title" - : "channel.subscribe.error.title" - ), - subtitle: error.localizedDescription - ) - } - - busy.remove(context) - } + membership.toggle(context, of: channel, in: appEnvironment, onToggled: onToggled) } } @@ -245,12 +331,17 @@ struct SubscriptionDestinationMenu: View { @Environment(\.appEnvironment) private var appEnvironment let channel: Channel + let membership: SubscriptionDestinationMembershipLoader var onToggled: ((DataContext, Bool) -> Void)? var body: some View { if SubscriptionDestinations.areOffered(in: appEnvironment) { Menu { - SubscriptionDestinationRows(channel: channel, onToggled: onToggled) + SubscriptionDestinationRows( + channel: channel, + membership: membership, + onToggled: onToggled + ) } label: { Label( String(localized: "channel.menu.subscribeOn"), @@ -271,6 +362,7 @@ struct SubscriptionDestinationPicker: View { @Environment(\.dismiss) private var dismiss let channel: Channel + let membership: SubscriptionDestinationMembershipLoader var onToggled: ((DataContext, Bool) -> Void)? var body: some View { @@ -286,7 +378,11 @@ struct SubscriptionDestinationPicker: View { // The picker stays open across a toggle: checkmarks are // the whole point of the list, and closing it would hide // the change the user just made. - SubscriptionDestinationRows(channel: channel, onToggled: onToggled) + SubscriptionDestinationRows( + channel: channel, + membership: membership, + onToggled: onToggled + ) } }