[tvOS] Delete User from User Selection Screen

[chickdan]

This PR adds the capability to delete local users on tvOS. There may need some tweaks such as the use of FocusGuide (which I'm not yet familiar with). Also strangely, I am unable to get the advanced menu to work if there is a conditional surrounding the edit users button (noted with a TODO).

Demo:

recording.mov

Delete via context menu:

Screen.Recording.mov

Notes:
When investigating the bottom bar moving to the bottom edge when only buttons are present, I discovered that the Menu component provides it's own bottom padding. So I added a padding of 50 to the button group to mostly match the spacing of the ServerSelectionMenu and AdvancedMenu.

[JPKribs]

I hope @LePips has a better answer but I've found this issue is consistent when you put a Menu in the NavigationBar on tvOS. If it's not possible to use Menus there in the NavigationBar then this is what I am thinking:

    @ViewBuilder
    private var advancedMenu: some View {
        Button(L10n.advanced, systemImage: "gearshape.fill") {
            isPresentingAdvancedMenu.toggle()
        }
    }

    private var menuSheet: some View {
        Section {
            if gridItems.count > 1 {
                Button(L10n.editUsers, systemImage: "person.crop.circle") {
                    isEditingUsers.toggle()
                    isPresentingAdvancedMenu.toggle()
                }
            }
        }
    }

...

    var body: some View {
        ZStack {
            if viewModel.servers.isEmpty {
                emptyView
            } else {
                userView
            }
        }
        .sheet(isPresented: $isPresentingAdvancedMenu) {
            menuSheet
        }
    }

This being said, this works but I hope there is a way to make that menu work.

We could mirror something like how the LearnMoreModal works. This is really rough but:

Like I said, Menu in the NavigationBar would be ideal but something like this as workaround isn't the worst thing, right?

What I have there looks like this:

//
// Swiftfin is subject to the terms of the Mozilla Public
// License, v2.0. If a copy of the MPL was not distributed with this
// file, you can obtain one at https://mozilla.org/MPL/2.0/.
//
// Copyright (c) 2024 Jellyfin & Jellyfin Contributors
//

import SwiftUI

struct MenuBox<Content: View>: View {

    private let title: String?
    private let content: Content

    // MARK: - Initializer

    init(title: String?, @ViewBuilder content: () -> Content) {
        self.title = title
        self.content = content()
    }

    // MARK: - Body

    var body: some View {
        VStack(alignment: .leading, spacing: 16) {
            if let title = title {
                Text(title)
                    .font(.title2)
                    .foregroundStyle(.primary)
            }

            content
        }
        .padding(64)
        .background {
            RoundedRectangle(cornerRadius: 16)
                .fill(Material.regular)
        }
    }
}

[LePips]

Due to overall issues with the "navigation bar" on tvOS with design, platform and SwiftUIs [lack of] implementation, and long standing focus issues, I'm more inclined to forego it and go with alternative designs and interactions. That does require more work due to the focus system as well.

For this view, we can make our own "bar" at the bottom that would hold all buttons that would typically go on the navigation bar on iOS. Refinements aside, I think we would do better with stuff like:

Single	Double

This custom "bottom navigation bar" would probably be best split out into a separate View, where it would also implement the Delete, Cancel, and probably Select/Remove All while editing users (just hasn't been implemented on iOS yet). We would have to take into account custom focus for this bar as well, utilizing FocusGuide to ideally always focus on the server selection menu when not editing users. This would be necessary in case the currently focused button is far to the left or right and moving down with the remote wouldn't focus a button by the natural implementation.

Here is my branch to recreate those screenshots:
main...LePips:SwiftFin:tvos-select-user-bottom-bar

[chickdan]

Thanks for the input! I'll work on moving the button to the bottom for the time being and work with @JPKribs on #1360

[JPKribs]

Working on #1360 and I realized we already have exactly what I am looking at in FullScreenMenu. Would this be a good route to take for this PR? Put the settings gear icon in the top right and use that to open a FullScreenMenu with our user options? It would be the same way we are handling the "menu" from the ServerSelectionMenu so it would feel pretty cohesive IMO.

[chickdan]

Working on #1360 and I realized we already have exactly what I am looking at in FullScreenMenu. Would this be a good route to take for this PR? Put the settings gear icon in the top right and use that to open a FullScreenMenu with our user options? It would be the same way we are handling the "menu" from the ServerSelectionMenu so it would feel pretty cohesive IMO.

I think that should work for the use case here, those changes do look much better too! I'll keep an eye on the conversation happening over there and make changes accordingly.

[chickdan]

Here is my progress so far, still some work left and I need to determine why the delete/cancel buttons hug the bottom, but it's functional!

in-small.mov

[JPKribs]

Here is my progress so far ... it's functional!

Nice work!!!

I need to determine why the delete/cancel buttons hug the bottom

My guess is some kind of padding issue? It looks like the user icons drop down a little with it? In my experience, fiddle with it for half an hour and it will randomly align how you need it to haha.

It looks like the viewModel is stateful so it might make some sense to add error handling in case the deletion fails. I did this for tvOS for item deletion. It should almost be a copy/paste from here if you're interested:

https://github.com/jellyfin/Swiftfin/blob/main/Swiftfin%20tvOS/Views/ItemView/Components/ActionButtons/ActionButtonHStack.swift#L135C1-L143C34

[LePips]

While editing user selection we currently don't show stuff like "select/remove all" like we have developed during the dashboard implementations. While not necessary for this round of implementation, it would possibly be nice if that was implemented as well. iOS should also receive that before 1.3 drops on the App Store and should be a relatively small change (within an ugly file...).

[LePips]

Sorry, another thing that I would like changed though. Repeating the message of #1365 (comment), a lot has been added in tvOS since I've last looked at it to make buttons better. UserGridButton and AddUserButton look like they will need to be slightly refactored to use the new hoverEffect(.highlight) and structure to make the label move automatically.

This would allow the following where UserGridButton has been refactored but the AddUserButton is the current implementation.

Sample commit: 4611587

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.at.1080p.-.2024-12-17.at.15.58.18.mp4

[chickdan]

It looks like the viewModel is stateful so it might make some sense to add error handling in case the deletion fails.

Please correct me if I'm wrong but it looks like this view already has .onReceive(viewModel.events) and is handling errors with

case let .error(eventError):
    self.error = eventError

[JPKribs]

Please correct me if I'm wrong but it looks like this view already has .onReceive(viewModel.events) and is handling errors with

You are correct! I missed that sorry about that!

[LePips]

I did push a different workaround for the weird Menu layout with the custom label. However there are still focus issues when editing with even just a single user. I haven't tested with a larger grid and know there would be focus issues there as well when trying to select the server selection menu.

Since Delete is disabled if the user isn't selected the focus cannot get to Cancel. My little FocusGuide that I've made should help here (but doesn't work 100%), where the following should be the behavior for the focus:

• keep track of last focused item (user, add)
• if not editing and attempting to focus bottom bar, go to server selection menu
  • if attempting to focus back to users, go to last focused item
  • if server selection changed, change last focused item to first item regardless if the items actually changed since the focus has been on the bar
• if editing and attempting to focus bottom bar:
  • if no users are selected, focus on Cancel
  • if users have been selected, focus on Delete
  • if attempting to focus back on users while still editing, go to last focused item
  • after cancellation or deletion, manually focus on first item

[chickdan]

@LePips I added .focusSection() to the bottom bar and the user grid, in my testing this works quite well for vertical focus. It obviously doesn't have the logic for conditionally focusing various elements but it at least gets this feature into a fully functional and usable state.

If you'd like I can add the focus logic but wanted your thoughts on this simpler approach first.

[LePips]

Yes, a focusSection is necessary and used in my FocusGuide but for now it's just fine. I thought I would have more time to work with my FocusGuide but I need to spend some time to rework it a bit and won't let that stop this.