Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] Admin Dashboard - User Device & TV Access #1342

Merged
merged 16 commits into from
Dec 9, 2024

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 7, 2024

Summary

Adds a new section to the ServerUserDetailsView to limit device access. On web, this is in the Access section. This should roughly mirror our media access section but I've added some additional device details like the latest user on that device, when was it last seen, and the device icon to spruce it up a little.

I also re-odered the ServerUserDetailsView so the Media Access and Device Access options are both just in their own 'Access' section. Permissions are now in the 'Advanced' section where the last two sections will be added as well. So this will be: Permissions, Parental controls, & Live TV.

I've added LiveTV as well. The reason I did this was because it's just 2 toggles. The enabledChannels is NOT LiveTV Channels. So, this is a lot smaller and an easy implementation. This can probably just get rolled up into the Permissions section but it also makes sense for it to be in its own Access section as well 🤷‍♂️

Screenshots

User Details
Enable All Devices Enable All
Enable Individual Devices Enable Devices
Live TV Access Enable Devices

Translations/en.lproj/Localizable.strings Outdated Show resolved Hide resolved
@JPKribs
Copy link
Member Author

JPKribs commented Dec 7, 2024

I set up Live TV on Jellyfin only to find that none of the 'Channel' settings are related. I'm not able to find a web equivalent so I think I'm safe to skip those. This is for items like:

enabledChannels
blockedChannels

So, I am treating this PR as wrapping up the User Details 'Access' section. I'm very close to the end for this. I left TODOs for my last items in the Admin Dashboard which are:

TODOs Screenshot 2024-12-07 at 02 31 50

PR 1

  • Username Editing
  • Profile Pictures

I just need to figure out the PFP caching/refreshing the cache on change

PR 2

  • Allow Tags (10.10+)
  • Block Tags

I am going to make 1 view to be shared between both. Will likely reuse the tag searching from #1336 for the add screen.

PR 3

  • Access Schedules

I need the listView and an add screen. Should roughly mirror the EditServerTasksView & AddServerTaskView.

PR 4

  • Parental Rating (maxParentalRating & blockUnratedItems)

I already have this done but I don't like how it works. I don't like how it looks on web either but I'm not sure what is a good route to do this cleaner. See below.

Parental Rating Parental Rating

@JPKribs JPKribs changed the title [iOS] Admin Dashboard - User Device Access [iOS] Admin Dashboard - User Device & TV Access Dec 7, 2024
@JPKribs
Copy link
Member Author

JPKribs commented Dec 9, 2024

I've made the changes. The padding was getting weird when I tried using the DevicesView.DeviceRow and, in fixing that, I made the same changes to ServerUsersView.ServerUserRow since I made those at the same time and they mirrored eachother.

I had the RowInsets in the row instead of from the view. So, it looked good in a plain list and cramped in a non-plain list. I've moved to just using those from the Views above them so it would be reusable for both Device Access and Device List. Users mirrors that just for parity sake + if we ever need a user row somewhere in the future.

Last change I made was I make onDelete optional but I kept the onSelect (mostly because the button still allowed selecting it just didn't do anything). onSelect for ServerUserDeviceAccess also toggles the toggle.

@JPKribs JPKribs requested a review from LePips December 9, 2024 16:10
@JPKribs
Copy link
Member Author

JPKribs commented Dec 9, 2024

For consistency's sake, EditItemElementRow needed the same thing... The downside of doing this all in such quick succession is, any weird decision I made for 1 listRow, I made for all of them. Just checked though all the ItemEditing & AdminDashboard and we should be consistent now!

…is whole folder needs a cleanup which I'll move to another PR.
@JPKribs
Copy link
Member Author

JPKribs commented Dec 9, 2024

ugh, one more item. Should we separate out the Checkbox logic? We have 3 locations that use this so it might be nice to centralize this:

  • SelectUserView.UserRow
  • ServerUsersView.ServerUsersRow
  • DevicesView.DeviceRow

I've got a branch with these changes created if we want them but this might be better on another PR. See below:

import Defaults
import SwiftUI

struct ListRowCheckbox: View {

    @Default(.accentColor)
    private var accentColor

    // MARK: - Environment Variables

    @Environment(\.isEditing)
    private var isEditing
    @Environment(\.isSelected)
    private var isSelected

    // MARK: - Body

    @ViewBuilder
    var body: some View {
        if isEditing, isSelected {
            Image(systemName: "checkmark.circle.fill")
                .resizable()
                .backport
                .fontWeight(.bold)
                .aspectRatio(1, contentMode: .fit)
                .frame(width: 24, height: 24)
                .symbolRenderingMode(.palette)
                .foregroundStyle(accentColor.overlayColor, accentColor)

        } else if isEditing {
            Image(systemName: "circle")
                .resizable()
                .backport
                .fontWeight(.bold)
                .aspectRatio(1, contentMode: .fit)
                .frame(width: 24, height: 24)
                .foregroundStyle(.secondary)
        }
    }
}

@LePips
Copy link
Member

LePips commented Dec 9, 2024

That sounds like a good addition.

@JPKribs JPKribs requested a review from LePips December 9, 2024 20:30
@LePips
Copy link
Member

LePips commented Dec 9, 2024

I made the toggle mimic the inset grouped list style like we do for the header, which allows the list to be plain and give more room for the device rows like we have elsewhere. How do you think that looks?

@JPKribs
Copy link
Member Author

JPKribs commented Dec 9, 2024

I made the toggle mimic the inset grouped list style like we do for the header, which allows the list to be plain and give more room for the device rows like we have elsewhere. How do you think that looks?

Oh that looks way better! I was hesitant to try and figure out mixing standard & plain lists but I'm very happy with how that looks! Only change I made was resetting the development team back to None & org.jellyfin.swiftfin

Edit: I also got rid of the "Run Time" string in favor of "runtime" since that was there first, I created the Run Time, and I don't personally know why I did it lol. Should resolve that random strings.swift change that shows up in commits moving runtime up and down 3 lines.

@LePips LePips merged commit 174487a into jellyfin:main Dec 9, 2024
4 checks passed
@JPKribs JPKribs added the enhancement New feature or request label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants