Mirrored_Repos/yattee
1
0
Fork 0
mirror of https://github.com/yattee/yattee.git synced 2026-10-23 21:22:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Send a creation where the button said it would go

Browse Source 
Creating a playlist while an Invidious account was the selected context
wrote it to the device. No request left the app, the row never appeared in
the account's list, and the form had come up without its visibility picker
— the two symptoms of the same thing: the sheet took the local branch.

The destination was decided correctly and then read wrongly. The button
resolved the account and put it in a companion `@State`, and the sheet's
content closure — created by an earlier body evaluation — read that
property as it stood *then*, which was nil. Measured on device: the create
resolved the account and validity fine, and one millisecond later the sheet
rendered the local form while every live value around it still named the
account. Whether the stale read is corrected depends on whether anything
re-renders the view before the form is saved, which is why the same steps
against Piped had landed on the server.

The destination now travels with the presentation as
`PlaylistCreationTarget`, so the sheet is handed the account rather than
looking one up. One resolver answers for both the create affordances and
the row menus, and it is where the rule lives: a context reading an
account's data creates on that account or creates nothing at all — a
toolbar item can outlive the condition that put it there, and a write with
nowhere to go is not a write to somewhere else. A context that fell back to
local reads is the one case that does create locally, and it is honest:
those are the rows on screen, with the banner above them saying why.

Two more from the same pass:

- Subscriptions stopped giving two answers at once. Under a fallback the
  local feed asks for a Yattee Server nobody has configured, so "Add a
  subscriptions account in Settings" was stacking under a banner saying
  this account needs signing in again. The banner names the cause, so the
  feed error stands down while it is up.

- The playlists banner no longer lands on the search field. A navigation
  bar carrying a search field keeps its full height over content that
  cannot scroll, and the empty, loading and error states could not — they
  scroll now, which also gives pull to refresh somewhere to live.
This commit is contained in:
Arkadiusz Fal
2026-09-05 23:41:25 +02:00
parent d41c6938f2
commit 6bf3942529
4 changed files with 273 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
Yattee/Views/Home/PlaylistCreationTarget.swift Normal file
View File

@@ -0,0 +1,72 @@
//
// PlaylistCreationTarget.swift
// Yattee
//
// Where a newly created playlist goes.
//
import Foundation
/// Where the playlists screen would put a playlist it is asked to create.
///
/// Kept as a value rather than left to be re-derived at save time: a creation
/// travels with the sheet that collects it, so the destination cannot move
/// while the form is open — and cannot be read one update late, which is how a
/// creation on an account ended up in local data with its visibility picker
/// missing.
enum PlaylistCreationTarget: Identifiable, Equatable {
/// The device's own playlists.
case local
/// A server-side playlist on this account.
case account(Account)
var id: String {
switch self {
case .local: "local"
case let .account(account): account.id.uuidString
}
}
/// The account to create on, or `nil` when the device is the destination.
var account: Account? {
switch self {
case .local: nil
case let .account(account): account
}
}
/// Where a creation started right now would go, or `nil` when there is
/// nowhere to put one and the affordance must not be offered.
///
/// Follows the *effective* context, which is what the rows on screen came
/// from: a selected account that cannot be used falls back to local reads,
/// and creating locally is then the same answer the list is already giving
/// — with the banner above it saying why. What never happens is the other
/// direction: an account context whose server cannot be written to right
/// now creates nothing at all, rather than quietly retargeting the write at
/// local data the user did not choose.
///
/// - Parameters:
/// - account: the account `effectiveContext` names, when it resolves.
/// - allowsServerMutations: the validator's write gate.
/// - capabilities: what that account's backend can do.
static func resolve(
effectiveContext: DataContext,
account: Account?,
allowsServerMutations: Bool,
capabilities: AccountCapabilities?
) -> PlaylistCreationTarget? {
switch effectiveContext {
case .local:
return .local
case .account:
guard let account,
allowsServerMutations,
capabilities?.supportsPlaylistWrites == true
else { return nil }
return .account(account)
}
}
}

82
Yattee/Views/Home/PlaylistsListView.swift
View File

@@ -12,7 +12,6 @@ struct PlaylistsListView: View {
@Namespace private var sheetTransition
@State private var localPlaylists: [LocalPlaylist] = []
@State private var searchText = ""
@State private var showingNewPlaylist = false
@State private var playlistToEdit: LocalPlaylist?
#if os(tvOS)
@FocusState private var focusedPlaylistID: PlaylistListItemID?
@@ -21,9 +20,15 @@ struct PlaylistsListView: View {
// MARK: - Remote state
/// The account a creation is for, captured when the button was pressed —
/// the form must not follow a context that moves while it is open.
@State private var createAccount: Account?
/// The creation being collected, and where it goes — captured when the
/// button was pressed.
///
/// The destination travels with the presentation instead of standing beside
/// it in a second `@State`: a companion value is read by the sheet's own
/// closure one update behind the button that set it, so a creation on an
/// account took the local branch — writing to the device and losing the
/// visibility picker on the way.
@State private var creation: PlaylistCreationTarget?
@State private var remoteEdit: RemotePlaylistEdit?
@State private var remoteDeletion: RemotePlaylistDeletion?
/// Why the account's list could not be fetched, when it could not.
@@ -110,23 +115,36 @@ struct PlaylistsListView: View {
return appEnvironment.accountsManager.account(id: id)
}
/// Where a playlist created right now would go, or `nil` when there is
/// nowhere to put one.
///
/// The single answer both the create affordances and the row actions are
/// gated on, so the button that offers a write and the write it performs
/// can never have decided differently.
private var creationTarget: PlaylistCreationTarget? {
guard let appEnvironment else { return nil }
let account = contextAccount
return PlaylistCreationTarget.resolve(
effectiveContext: appEnvironment.dataContextValidator.effectiveContext,
account: account,
allowsServerMutations: appEnvironment.dataContextValidator.allowsServerMutations,
capabilities: account.flatMap { appEnvironment.accountsManager.capabilities(for: $0) }
)
}
/// The account these playlists may be changed on.
///
/// Both halves are asked even though reads only ever follow a valid context
/// today: the write gate is what has to hold if that ever stops being true.
private var writableAccount: Account? {
guard let appEnvironment,
let account = contextAccount,
appEnvironment.dataContextValidator.allowsServerMutations,
appEnvironment.accountsManager.capabilities(for: account)?.supportsPlaylistWrites == true
else { return nil }
guard case let .account(account) = creationTarget else { return nil }
return account
}
/// Whether the create affordances have somewhere to create.
private var canCreatePlaylist: Bool {
contextAccount == nil || writableAccount != nil
creationTarget != nil
}
private var contextUnavailability: DataContextUnavailability? {
@@ -232,18 +250,18 @@ struct PlaylistsListView: View {
.liquidGlassSheetContent(sourceID: "playlistsViewOptions", in: sheetTransition)
}
#endif
.sheet(isPresented: $showingNewPlaylist) {
if let createAccount {
.sheet(item: $creation) { creation in
if let account = creation.account {
PlaylistFormSheet(
mode: .create,
showsPrivacy: supportsPrivacy(createAccount)
showsPrivacy: supportsPrivacy(account)
) { title, description, privacy in
Task {
await createRemotePlaylist(
title: title,
description: description,
privacy: privacy,
on: createAccount
on: account
)
}
}
@@ -328,12 +346,18 @@ struct PlaylistsListView: View {
@ViewBuilder
private var content: some View {
if isLoadingRemoteList {
ProgressView()
.frame(maxWidth: .infinity, maxHeight: .infinity)
scrollableState {
ProgressView()
.frame(maxWidth: .infinity)
}
} else if let remoteErrorMessage, items.isEmpty {
remoteErrorView(remoteErrorMessage)
scrollableState {
remoteErrorView(remoteErrorMessage)
}
} else if filteredItems.isEmpty {
emptyView
scrollableState {
emptyView
}
} else {
switch layout {
case .list:
@@ -344,6 +368,18 @@ struct PlaylistsListView: View {
}
}
/// A state with nothing to scroll, put in a scroll view anyway.
///
/// Two things need one. Pull to refresh has nowhere to live without it —
/// and a navigation bar carrying a search field keeps its full height over
/// content that cannot scroll, which is what the banner was landing under.
private func scrollableState(@ViewBuilder _ state: () -> some View) -> some View {
ScrollView {
state()
.containerRelativeFrame(.vertical)
}
}
#if os(tvOS)
// MARK: - tvOS Content
@@ -714,9 +750,13 @@ struct PlaylistsListView: View {
// MARK: - Creating
/// Opens the form over the destination as it stands at this moment.
///
/// Silent when there is nowhere to create: a toolbar item can outlive the
/// condition that put it there, and a write with nowhere to go is not a
/// write to somewhere else.
private func beginCreate() {
createAccount = writableAccount
showingNewPlaylist = true
creation = creationTarget
}
private func createRemotePlaylist(

51
Yattee/Views/Subscriptions/SubscriptionsView.swift
View File

@@ -165,11 +165,24 @@ struct SubscriptionsView: View {
}
#endif
/// The feed error worth showing, if any.
///
/// Nothing while the context banner is up: a selected account that cannot be
/// used reads local data, and the local feed then reports the Yattee Server
/// it has no reason to have — "add a subscriptions account in Settings"
/// under a banner saying *this* account needs signing in again is two
/// answers to one question. The banner names the cause, so it is the one
/// that stays.
private var displayedFeedError: FeedLoadState.FeedLoadError? {
guard contextUnavailability == nil,
case let .error(error) = feedCache.feedLoadState
else { return nil }
return error
}
private var isShowingFullScreenError: Bool {
if case .error = feedCache.feedLoadState, feedCache.videos.isEmpty {
return true
}
return false
displayedFeedError != nil && feedCache.videos.isEmpty
}
private var dataManager: DataManager? { appEnvironment?.dataManager }
@@ -249,19 +262,21 @@ struct SubscriptionsView: View {
.background(Color.black.opacity(0.3))
#endif
case .error(let error):
HStack(spacing: 8) {
Image(systemName: "exclamationmark.triangle")
Text(errorMessage(for: error))
.font(.caption)
case .error:
if let error = displayedFeedError {
HStack(spacing: 8) {
Image(systemName: "exclamationmark.triangle")
Text(errorMessage(for: error))
.font(.caption)
}
.foregroundStyle(.red)
.padding(.horizontal)
.padding(.vertical, 8)
.frame(maxWidth: .infinity)
#if os(tvOS)
.background(Color.black.opacity(0.3))
#endif
}
.foregroundStyle(.red)
.padding(.horizontal)
.padding(.vertical, 8)
.frame(maxWidth: .infinity)
#if os(tvOS)
.background(Color.black.opacity(0.3))
#endif
case .loadingMore:
HStack(spacing: 8) {
@@ -640,7 +655,7 @@ struct SubscriptionsView: View {
/// Feed content rows or empty/loading states.
@ViewBuilder
private var feedContentRows: some View {
if case .error(let feedError) = feedCache.feedLoadState, feedCache.videos.isEmpty {
if let feedError = displayedFeedError, feedCache.videos.isEmpty {
// Show specific error states
switch feedError {
case .yatteeServerRequired:
@@ -730,7 +745,7 @@ struct SubscriptionsView: View {
#endif
// Content
if case .error(let feedError) = feedCache.feedLoadState, feedCache.videos.isEmpty {
if let feedError = displayedFeedError, feedCache.videos.isEmpty {
// Show specific error states
switch feedError {
case .yatteeServerRequired:

107
YatteeTests/PlaylistCreationTargetTests.swift Normal file
View File

@@ -0,0 +1,107 @@
//
// PlaylistCreationTargetTests.swift
// YatteeTests
//
// Tests for where a newly created playlist is sent.
//
import Testing
import Foundation
@testable import Yattee
@Suite("Playlist Creation Target")
struct PlaylistCreationTargetTests {
// MARK: - Helpers
private static let account = Account(instanceID: UUID())
private static func capabilities(playlistWrites: Bool) -> AccountCapabilities {
AccountCapabilities(
supportsSubscriptions: true,
supportsFeed: true,
supportsPlaylistReads: true,
supportsPlaylistWrites: playlistWrites,
supportsWatchHistory: false,
supportsPlaylistPrivacy: false
)
}
private static func resolve(
effectiveContext: DataContext,
account: Account? = account,
allowsServerMutations: Bool = true,
capabilities: AccountCapabilities? = capabilities(playlistWrites: true)
) -> PlaylistCreationTarget? {
PlaylistCreationTarget.resolve(
effectiveContext: effectiveContext,
account: account,
allowsServerMutations: allowsServerMutations,
capabilities: capabilities
)
}
// MARK: - Local
@Test("The device's own context creates locally")
func localContextCreatesLocally() {
#expect(Self.resolve(effectiveContext: .local, account: nil) == .local)
}
/// The fallback case: the selected account is unusable, reads have moved to
/// local, and the rows on screen are local — so a creation is local too.
@Test("A context that fell back to local creates locally")
func fallbackCreatesLocally() {
#expect(
Self.resolve(
effectiveContext: .local,
account: nil,
allowsServerMutations: false,
capabilities: nil
) == .local
)
}
// MARK: - Account
@Test("An account context creates on that account")
func accountContextCreatesOnAccount() {
#expect(Self.resolve(effectiveContext: .account(Self.account.id)) == .account(Self.account))
}
/// The defect this exists to prevent: an account context must never quietly
/// become a local write.
@Test("An account that may not be written to creates nowhere")
func mutationsDisallowedCreatesNowhere() {
#expect(Self.resolve(effectiveContext: .account(Self.account.id), allowsServerMutations: false) == nil)
}
@Test("A backend without playlist writes creates nowhere")
func withoutPlaylistWritesCreatesNowhere() {
#expect(
Self.resolve(
effectiveContext: .account(Self.account.id),
capabilities: Self.capabilities(playlistWrites: false)
) == nil
)
}
@Test("An account whose capabilities are unknown creates nowhere")
func unknownCapabilitiesCreateNowhere() {
#expect(Self.resolve(effectiveContext: .account(Self.account.id), capabilities: nil) == nil)
}
@Test("An account context that resolves to no account creates nowhere")
func unresolvedAccountCreatesNowhere() {
#expect(Self.resolve(effectiveContext: .account(Self.account.id), account: nil) == nil)
}
// MARK: - Identity
@Test("Each destination presents as itself")
func identityDistinguishesDestinations() {
#expect(PlaylistCreationTarget.local.id != PlaylistCreationTarget.account(Self.account).id)
#expect(PlaylistCreationTarget.account(Self.account).account == Self.account)
#expect(PlaylistCreationTarget.local.account == nil)
}
}