From b0b604c4adf14c2f56320401d052ae87dfbcc507 Mon Sep 17 00:00:00 2001 From: Joe Kribs Date: Thu, 23 Jan 2025 20:25:08 -0700 Subject: [PATCH] [tvOS] Login Flow Cleanup - Second Pass (#1403) * Background on Server User Signin. Button Sizing. More visible deletion notice. Menu ListView insets. * wip * Change Highlight. Move Add User Button. Remove Add User inline option. * Take 2 * Undo user changes. * Remove all changes. * "selectServer" = "Select Server"; * Recommendations Co-authored-by: Ethan Pippin * Update ServerDetailView.swift Co-authored-by: Ethan Pippin * Update ServerDetailView.swift Co-authored-by: Ethan Pippin * build strings --------- Co-authored-by: Ethan Pippin --- Shared/Strings/Strings.swift | 2 + Swiftfin tvOS/Components/ListRowButton.swift | 28 ++++++-- .../Components/SplitLoginWindowView.swift | 21 ++++++ .../Components/AddUserButton.swift | 66 ++++++++++++------ .../Components/SelectUserBottomBar.swift | 69 +++++++++---------- .../Components/UserGridButton.swift | 53 +++++++------- .../Views/SelectUserView/SelectUserView.swift | 56 +++++++-------- Swiftfin tvOS/Views/ServerDetailView.swift | 7 +- .../Views/UserSignInView/UserSignInView.swift | 3 +- .../Components/AddUserButton.swift | 2 +- .../Components/AddUserRow.swift | 2 +- Translations/en.lproj/Localizable.strings | 3 + 12 files changed, 185 insertions(+), 127 deletions(-) diff --git a/Shared/Strings/Strings.swift b/Shared/Strings/Strings.swift index fe6e0071..dec35a28 100644 --- a/Shared/Strings/Strings.swift +++ b/Shared/Strings/Strings.swift @@ -1130,6 +1130,8 @@ internal enum L10n { internal static let selectAll = L10n.tr("Localizable", "selectAll", fallback: "Select All") /// Select Image internal static let selectImage = L10n.tr("Localizable", "selectImage", fallback: "Select Image") + /// Select server + internal static let selectServer = L10n.tr("Localizable", "selectServer", fallback: "Select server") /// Series internal static let series = L10n.tr("Localizable", "series", fallback: "Series") /// Series Backdrop diff --git a/Swiftfin tvOS/Components/ListRowButton.swift b/Swiftfin tvOS/Components/ListRowButton.swift index 1774fb62..7b964e3d 100644 --- a/Swiftfin tvOS/Components/ListRowButton.swift +++ b/Swiftfin tvOS/Components/ListRowButton.swift @@ -11,10 +11,12 @@ import SwiftUI struct ListRowButton: View { let title: String + let role: ButtonRole? let action: () -> Void - init(_ title: String, action: @escaping () -> Void) { + init(_ title: String, role: ButtonRole? = nil, action: @escaping () -> Void) { self.title = title + self.role = role self.action = action } @@ -23,15 +25,33 @@ struct ListRowButton: View { action() } label: { ZStack { - Rectangle() - .foregroundStyle(.secondary) + RoundedRectangle(cornerRadius: 10) + .fill(secondaryStyle) Text(title) + .foregroundStyle(primaryStyle) .font(.body.weight(.bold)) - .foregroundStyle(.primary) } } .buttonStyle(.card) .frame(height: 75) } + + // MARK: - Styles + + private var primaryStyle: some ShapeStyle { + if role == .destructive { + return AnyShapeStyle(Color.red) + } else { + return AnyShapeStyle(.primary) + } + } + + private var secondaryStyle: some ShapeStyle { + if role == .destructive { + return AnyShapeStyle(Color.red.opacity(0.2)) + } else { + return AnyShapeStyle(.secondary) + } + } } diff --git a/Swiftfin tvOS/Components/SplitLoginWindowView.swift b/Swiftfin tvOS/Components/SplitLoginWindowView.swift index 5cc6ce50..dfdc7e91 100644 --- a/Swiftfin tvOS/Components/SplitLoginWindowView.swift +++ b/Swiftfin tvOS/Components/SplitLoginWindowView.swift @@ -22,6 +22,10 @@ struct SplitLoginWindowView: View { private let trailingTitle: String private let trailingContentView: () -> Trailing + // MARK: - Background Variable + + private let backgroundImageSource: ImageSource? + // MARK: - Body var body: some View { @@ -52,6 +56,21 @@ struct SplitLoginWindowView: View { } } .navigationBarBranding(isLoading: isLoading) + .background { + if let backgroundImageSource { + ZStack { + ImageView(backgroundImageSource) + .aspectRatio(contentMode: .fill) + .id(backgroundImageSource) + .transition(.opacity) + .animation(.linear, value: backgroundImageSource) + + Color.black + .opacity(0.9) + } + .ignoresSafeArea() + } + } } } @@ -61,6 +80,7 @@ extension SplitLoginWindowView { isLoading: Bool = false, leadingTitle: String, trailingTitle: String, + backgroundImageSource: ImageSource? = nil, @ViewBuilder leadingContentView: @escaping () -> Leading, @ViewBuilder trailingContentView: @escaping () -> Trailing ) { @@ -69,5 +89,6 @@ extension SplitLoginWindowView { self.trailingTitle = trailingTitle self.leadingContentView = leadingContentView self.trailingContentView = trailingContentView + self.backgroundImageSource = backgroundImageSource } } diff --git a/Swiftfin tvOS/Views/SelectUserView/Components/AddUserButton.swift b/Swiftfin tvOS/Views/SelectUserView/Components/AddUserButton.swift index 7b4165da..48878ba4 100644 --- a/Swiftfin tvOS/Views/SelectUserView/Components/AddUserButton.swift +++ b/Swiftfin tvOS/Views/SelectUserView/Components/AddUserButton.swift @@ -42,37 +42,59 @@ extension SelectUserView { self.servers = servers } + @ViewBuilder + private var content: some View { + ZStack { + Color.secondarySystemFill + + RelativeSystemImageView(systemName: "plus") + .foregroundStyle(.secondary) + } + .clipShape(.circle) + .aspectRatio(1, contentMode: .fill) + .hoverEffect(.highlight) + + Text(L10n.addUser) + .font(.title3) + .fontWeight(.semibold) + .foregroundStyle(isEnabled ? .primary : .secondary) + + if serverSelection == .all { + // For layout, not to be localized + Text("Hidden") + .font(.footnote) + .hidden() + } + } + var body: some View { - VStack { + if serverSelection == .all { + Menu { + Text(L10n.selectServer) + + ForEach(servers) { server in + Button { + action(server) + } label: { + Text(server.name) + Text(server.currentURL.absoluteString) + } + } + } label: { + content + } + .buttonStyle(.borderless) + .buttonBorderShape(.circle) + } else { Button { if let selectedServer { action(selectedServer) } } label: { - ZStack { - Color.secondarySystemFill - - RelativeSystemImageView(systemName: "plus") - .foregroundStyle(.secondary) - } - .clipShape(.circle) - .aspectRatio(1, contentMode: .fill) - .hoverEffect(.highlight) - - Text(L10n.addUser) - .font(.title3) - .fontWeight(.semibold) - .foregroundStyle(isEnabled ? .primary : .secondary) - - if serverSelection == .all { - Text(L10n.hidden) - .font(.footnote) - .hidden() - } + content } .buttonStyle(.borderless) .buttonBorderShape(.circle) - .disabled(!isEnabled) } } } diff --git a/Swiftfin tvOS/Views/SelectUserView/Components/SelectUserBottomBar.swift b/Swiftfin tvOS/Views/SelectUserView/Components/SelectUserBottomBar.swift index 8147d44d..ece7b9b1 100644 --- a/Swiftfin tvOS/Views/SelectUserView/Components/SelectUserBottomBar.swift +++ b/Swiftfin tvOS/Views/SelectUserView/Components/SelectUserBottomBar.swift @@ -31,13 +31,26 @@ extension SelectUserView { @ViewBuilder private var advancedMenu: some View { - Menu(L10n.advanced, systemImage: "gearshape.fill") { - + Menu { Button(L10n.editUsers, systemImage: "person.crop.circle") { isEditing.toggle() } + // TODO: Advanced settings on tvOS? + // + // Divider() + // + // Button(L10n.advanced, systemImage: "gearshape.fill") { + // router.route(to: \.advancedSettings) + // } + } label: { + Label(L10n.advanced, systemImage: "gearshape.fill") + .font(.body.weight(.semibold)) + .foregroundStyle(Color.primary) + .labelStyle(.iconOnly) + .frame(width: 50, height: 50) + } - // TODO: Do we want to support a grid view and list view like iOS? + // TODO: Do we want to support a grid view and list view like iOS? // if !viewModel.servers.isEmpty { // Picker(selection: $userListDisplayType) { // ForEach(LibraryDisplayType.allCases, id: \.hashValue) { @@ -51,40 +64,20 @@ extension SelectUserView { // } // .pickerStyle(.menu) // } - - // TODO: Advanced settings on tvOS? -// Section { -// Button(L10n.advanced, systemImage: "gearshape.fill") { -// router.route(to: \.advancedSettings) -// } -// } - } - .labelStyle(.iconOnly) } + // MARK: - Delete User Button + private var deleteUsersButton: some View { - Button { + ListRowButton(L10n.delete, role: .destructive) { onDelete() - } label: { - ZStack { - Color.red - - Text(L10n.delete) - .font(.body.weight(.semibold)) - .foregroundStyle(areUsersSelected ? .primary : .secondary) - - if !areUsersSelected { - Color.black - .opacity(0.5) - } - } - .frame(width: 400, height: 65) - .clipShape(RoundedRectangle(cornerRadius: 10)) } + .frame(width: 400, height: 50) .disabled(!areUsersSelected) - .buttonStyle(.card) } + // MARK: - Initializer + init( isEditing: Binding, serverSelection: Binding, @@ -103,6 +96,8 @@ extension SelectUserView { self.toggleAllUsersSelected = toggleAllUsersSelected } + // MARK: - Content View + @ViewBuilder private var contentView: some View { HStack(alignment: .center) { @@ -113,16 +108,20 @@ extension SelectUserView { toggleAllUsersSelected() } label: { Text(areUsersSelected ? L10n.removeAll : L10n.selectAll) - .font(.body.weight(.semibold)) .foregroundStyle(Color.primary) + .font(.body.weight(.semibold)) + .frame(width: 200, height: 50) + .clipShape(RoundedRectangle(cornerRadius: 10)) } Button { isEditing = false } label: { - L10n.cancel.text - .font(.body.weight(.semibold)) + Text(L10n.cancel) .foregroundStyle(Color.primary) + .font(.body.weight(.semibold)) + .frame(width: 200, height: 50) + .clipShape(RoundedRectangle(cornerRadius: 10)) } } else { ServerSelectionMenu( @@ -130,13 +129,13 @@ extension SelectUserView { viewModel: viewModel ) - if userCount > 1 { - advancedMenu - } + advancedMenu } } } + // MARK: - Body + var body: some View { // `Menu` with custom label has some weird additional // frame/padding that differs from default label style diff --git a/Swiftfin tvOS/Views/SelectUserView/Components/UserGridButton.swift b/Swiftfin tvOS/Views/SelectUserView/Components/UserGridButton.swift index 35911a54..a9e0a715 100644 --- a/Swiftfin tvOS/Views/SelectUserView/Components/UserGridButton.swift +++ b/Swiftfin tvOS/Views/SelectUserView/Components/UserGridButton.swift @@ -28,6 +28,8 @@ extension SelectUserView { private let action: () -> Void private let onDelete: () -> Void + // MARK: - Initializer + init( user: UserState, server: ServerState, @@ -42,49 +44,52 @@ extension SelectUserView { self.onDelete = onDelete } + // MARK: - Label Foreground Style + private var labelForegroundStyle: some ShapeStyle { guard isEditing else { return .primary } return isSelected ? .primary : .secondary } - @ViewBuilder - private var personView: some View { - ZStack { - Color.secondarySystemFill + // MARK: - User Portrait - RelativeSystemImageView(systemName: "person.fill", ratio: 0.5) - .foregroundStyle(.secondary) - } - .clipShape(.circle) - .aspectRatio(1, contentMode: .fill) - } - - @ViewBuilder - private var userImage: some View { + private var userPortrait: some View { UserProfileImage( userID: user.id, source: user.profileImageSource( client: server.client, maxWidth: 120 - ) + ), + pipeline: .Swiftfin.local ) - .aspectRatio(1, contentMode: .fill) .overlay { if isEditing { - Color.black - .opacity(isSelected ? 0 : 0.5) - .clipShape(.circle) + ZStack(alignment: .bottom) { + Color.black + .opacity(isSelected ? 0 : 0.5) + + if isSelected { + Image(systemName: "checkmark.circle.fill") + .resizable() + .aspectRatio(contentMode: .fill) + .frame(width: 75, height: 75) + .symbolRenderingMode(.palette) + .foregroundStyle(accentColor.overlayColor, accentColor) + } + } } } } + // MARK: - Body + var body: some View { VStack { Button { action() } label: { - userImage + userPortrait .hoverEffect(.highlight) Text(user.username) @@ -107,16 +112,6 @@ extension SelectUserView { } } } - .overlay { - if isEditing && isSelected { - Image(systemName: "checkmark.circle.fill") - .resizable() - .aspectRatio(contentMode: .fit) - .frame(width: 40, height: 40, alignment: .bottomTrailing) - .symbolRenderingMode(.palette) - .foregroundStyle(accentColor.overlayColor, accentColor) - } - } } } } diff --git a/Swiftfin tvOS/Views/SelectUserView/SelectUserView.swift b/Swiftfin tvOS/Views/SelectUserView/SelectUserView.swift index 3290a90c..c96a54a7 100644 --- a/Swiftfin tvOS/Views/SelectUserView/SelectUserView.swift +++ b/Swiftfin tvOS/Views/SelectUserView/SelectUserView.swift @@ -306,34 +306,22 @@ struct SelectUserView: View { @ViewBuilder private var emptyView: some View { - ZStack { - VStack { - Image(.jellyfinBlobBlue) - .resizable() - .aspectRatio(contentMode: .fit) - .frame(height: 100) - .edgePadding() + VStack(spacing: 50) { + L10n.connectToJellyfinServerStart.text + .font(.body) + .frame(minWidth: 50, maxWidth: 500) + .multilineTextAlignment(.center) - Color.clear - } - - VStack(spacing: 50) { - L10n.connectToJellyfinServerStart.text - .font(.body) - .frame(minWidth: 50, maxWidth: 500) - .multilineTextAlignment(.center) - - Button { - router.route(to: \.connectToServer) - } label: { - L10n.connect.text - .font(.callout) - .fontWeight(.bold) - .frame(width: 400, height: 75) - .background(Color.jellyfinPurple) - } - .buttonStyle(.card) + Button { + router.route(to: \.connectToServer) + } label: { + L10n.connect.text + .font(.callout) + .fontWeight(.bold) + .frame(width: 400, height: 75) + .background(Color.jellyfinPurple) } + .buttonStyle(.card) } } @@ -350,13 +338,20 @@ struct SelectUserView: View { } } - // change splash screen selection if necessary -// selectUserAllServersSplashscreen = serverSelection + setSplashScreenImageSource() } + // MARK: - Did Appear + private func didAppear() { viewModel.send(.getServers) + setSplashScreenImageSource() + } + + // MARK: - Set Splash Screen Image Source + + private func setSplashScreenImageSource() { splashScreenImageSource = makeSplashScreenImageSource( serverSelection: serverSelection, allServersSelection: .all @@ -385,10 +380,7 @@ struct SelectUserView: View { .onChange(of: serverSelection) { _, newValue in gridItems = makeGridItems(for: newValue) - splashScreenImageSource = makeSplashScreenImageSource( - serverSelection: newValue, - allServersSelection: .all - ) + setSplashScreenImageSource() } .onChange(of: isPresentingLocalPin) { _, newValue in if newValue { diff --git a/Swiftfin tvOS/Views/ServerDetailView.swift b/Swiftfin tvOS/Views/ServerDetailView.swift index c57205f6..d6af5224 100644 --- a/Swiftfin tvOS/Views/ServerDetailView.swift +++ b/Swiftfin tvOS/Views/ServerDetailView.swift @@ -75,14 +75,17 @@ struct EditServerView: View { .foregroundColor(.secondary) } } + .listRowBackground(Color.clear) + .listRowInsets(.zero) } if isEditing { Section { - ListRowButton(L10n.delete) { + ListRowButton(L10n.delete, role: .destructive) { isPresentingConfirmDeletion = true } - .foregroundStyle(.primary, .red.opacity(0.5)) + .listRowBackground(Color.clear) + .listRowInsets(.zero) } } } diff --git a/Swiftfin tvOS/Views/UserSignInView/UserSignInView.swift b/Swiftfin tvOS/Views/UserSignInView/UserSignInView.swift index 3a51e1ab..45ffd272 100644 --- a/Swiftfin tvOS/Views/UserSignInView/UserSignInView.swift +++ b/Swiftfin tvOS/Views/UserSignInView/UserSignInView.swift @@ -166,7 +166,8 @@ struct UserSignInView: View { SplitLoginWindowView( isLoading: viewModel.state == .signingIn, leadingTitle: L10n.signInToServer(viewModel.server.name), - trailingTitle: L10n.publicUsers + trailingTitle: L10n.publicUsers, + backgroundImageSource: viewModel.server.splashScreenImageSource() ) { signInSection } trailingContentView: { diff --git a/Swiftfin/Views/SelectUserView/Components/AddUserButton.swift b/Swiftfin/Views/SelectUserView/Components/AddUserButton.swift index c5686602..5ebfa214 100644 --- a/Swiftfin/Views/SelectUserView/Components/AddUserButton.swift +++ b/Swiftfin/Views/SelectUserView/Components/AddUserButton.swift @@ -80,7 +80,7 @@ extension SelectUserView { if serverSelection == .all { Menu { - Text("Select Server") + Text(L10n.selectServer) ForEach(servers) { server in Button { diff --git a/Swiftfin/Views/SelectUserView/Components/AddUserRow.swift b/Swiftfin/Views/SelectUserView/Components/AddUserRow.swift index 122c3098..c1826586 100644 --- a/Swiftfin/Views/SelectUserView/Components/AddUserRow.swift +++ b/Swiftfin/Views/SelectUserView/Components/AddUserRow.swift @@ -99,7 +99,7 @@ extension SelectUserView { if serverSelection == .all { Menu { - Text("Select Server") + Text(L10n.selectServer) ForEach(servers) { server in Button { diff --git a/Translations/en.lproj/Localizable.strings b/Translations/en.lproj/Localizable.strings index e84c9255..3212d174 100644 --- a/Translations/en.lproj/Localizable.strings +++ b/Translations/en.lproj/Localizable.strings @@ -1618,6 +1618,9 @@ /// Select Image "selectImage" = "Select Image"; +/// Select server +"selectServer" = "Select server"; + /// Series "series" = "Series";