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 #1230

Merged
merged 43 commits into from
Oct 5, 2024
Merged

[iOS] Admin Dashboard #1230

merged 43 commits into from
Oct 5, 2024

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Sep 5, 2024

Issue

Resolves: #608

Summary

This PR Creates a User/Admin Dashboard. This view replaces the current EditServerView that is accessible from the SettingsView. For now, this feature will only exist on iOS and iPadOS. If there is an appetite for adding this to tvOS I can look into it but, on a personal level, I don't see a lot of value in adding this to tvOS.

When clicking into this view, the user will always be provided with name of the server and the ability to switch to another URL for the same server. For Non-Admin Users, this PR adds an additional section that shows all Active Devices.

Active Devices are handled by the Server. With this, the Server only returns Sessions that the user is allowed to see. For Non-Admins, this means they can only see their own Sessions. For Admins, they are able to see all Sessions. All filtering/security for this section is handled by the Server API.

Clicking an Active Device will show the user Active Device Details which includes:

  • Content currently being played
  • Current progress of the Content
  • Stream details including transcoding
  • Transcoding reasons (if transcoding is occurring)

Both the Active Devices List and Active Device Details update every second with the status and progress of the Active Device. If the device is active but not streaming, both view will show the last time that the device interacted with the Jellyfin Server.

If the User accessing the Dashboard is an Admin they will see 2 additional Administration buttons. These buttons are:

  • Scan All Libraries
  • Scheduled Tasks

Scan All Libraries is the same function as Jellyfin-Web. When triggered, it returns the current progress of the process. There is a Red X on the left side while this task is running which can be selected to cancel the Libraries Scan. The progress percentage pings every second, even if not triggered from the Server, so this shows Library scans that are triggered from other devices as well with the ability to cancel them too.

Scheduled Tasks takes the Admin User to another View that contains all of the available Tasks on the Server. If selected, this runs these tasks. The same as the Scan All Libraries, these tasks can be cancelled and progress is showed. In addition to the Scheduled Tasks, the Shutdown/Restart Server tasks are included on this page at the top. These prompt the user with a confirmation prior to running.

The only difference between iOS and iPadOS is the Active Devices section contains 1 column on iOS and 2 columns on iPadOS.

Please find all Screenshots in the Screenshots Section below.

Difficult to Recreate Issues

  • The container in the Active Device Details > Streams Section comes from SessionInfo.NowPlayingItem.Container. On my server, it's always 100% correct but for some reason demo.jellyfin.org/stable is giving me 7 containers for every item.

Screenshots

User Dashboard User Dashboard
User Dashboard - URL Switching User Dashboard - URL Switching
Admin Dashboard Admin Dashboard
Admin Dashboard - URL Switching Admin Dashboard - URL Switching
Admin Dashboard - Scan in Progress Admin Dashboard - Scan in Progress
Admin Dashboard - Scan Cancelling Admin Dashboard - Scan Cancelled
Admin Dashboard - iPad Admin Dashboard - iPad
Shutdown/Restart Warnings Restart Warning Shutdown Warning
Active Device Details - DirectPlay Active Device Details - DirectPlay
Active Device Details - Transcode Active Device Details - Transcode
Active Device Details - Idle Sessions This ticks up every second. If the user starts playback, the view populates with the new session details. Active Device Details - Idle
Active Device Details - Offline Session This will only happen when you are viewing a User Session Detail and the user goes offline while you are looking at it. Active Device Details - Offline
Scheduled Tasks Scheduled Tasks
Scheduled Tasks - Many Running Scheduled Tasks - Many Running

@JPKribs
Copy link
Member Author

JPKribs commented Sep 5, 2024

@LePips Absolutely no rush on this but I wanted to get your thought on this PR and make sure I was going in the right direction for what the Admin Dashboard would include.

@JPKribs JPKribs marked this pull request as ready for review September 6, 2024 04:23
…change the image will stay the same. Use a random image found for Portrait/Landscape.
@sjorge
Copy link

sjorge commented Sep 6, 2024

When it's transcoding, does it also show the reason/format ? That generally the most useful thing I sometimes check on the go.

I don't think we should fully duplicate all functionality of the web interface, even vue is struggling with that.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 6, 2024

I have the type broken out like Jellyfin-Web:

static func getDisplayPlayMethod(_ session: SessionInfo) -> SessionPlaybackMethod {
        if let transcodingInfo = session.transcodingInfo {
            if (
                transcodingInfo.isVideoDirect ?? false ||
                    transcodingInfo.videoCodec == nil
            ) &&
                transcodingInfo.isAudioDirect ?? false
            {
                return .remux
            } else if transcodingInfo.isVideoDirect ?? false {
                return .directStream
            } else if session.playState?.playMethod == .transcode {
                return .transcode
            }
        }
        if session.playState?.playMethod == .directStream {
            return .directStream
        } else if session.playState?.playMethod == .directPlay {
            return .directPlay
        }

        return .unknown
    }

But I'm guessing you mean: Example

I could look at adding this in but I originally skipped it since phone screen real estate is a lot smaller and I was worried it was starting to get cluttered.

@sjorge
Copy link

sjorge commented Sep 6, 2024

Correct, it seems those activity tiles already can already be expanded based on the little V arrow.

At least the transcode reason would be nice to have as an extra item there.

Target audio and video code would be useful too, but IMHO not as important.

Although both are definitely things that can be added as a follow up at a later time.

Also will this get a tvOS variant? It seems less useful there.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 6, 2024

I still need to reorganize since this has gotten pretty messy behind the scenes but is this the type of information we're looking for or is the reason string important too?
Screenshot 2024-09-06 at 10 31 36

@sjorge
Copy link

sjorge commented Sep 6, 2024

I still need to reorganize since this has gotten pretty messy behind the scenes but is this the type of information we're looking for or is the reason string important too?
Screenshot 2024-09-06 at 10 31 36

What is show here would generally be sufficient. (For me at least)

With the info in the screenshot we see what the client is and what is being transcoded vs copied, in this case AC3 -> AAC for an iOS client makes sense as they generally do not play dolby surround.

So we know it's a lightweight transcode in this case.

The reason string would take the guess work out of it, but it does tend to be rather 'lengthy'. But it would also tell the transcode is because of audio stream incompatibly. It might be less clear for a video example though, a HVEC to h264 transcode could be because: a) the device does not support the HVEC profile b) subtitles are being burned-in because they are not supported c) original bitrate is too high. (I think b, can be covered by showing if the subtitle stream is copy or not though)

I'm not sure what the right thing to do is, we need to weigh the clutter vs providing information vs code complexity.
e.g. we could make it so tapping the 3 lines showing the stream detail would swap between this display and just the reason string so the user could easily switch without it taking up a lot of extra space but that would make things more complex code wise (I assume, swift/object C is confusing for me) or be too intuitive.

Also adding the reason below, would probably make it take up to much space.

Maybe we can have Erik our ui-ux guy chime in?

Edit: I just spotted the transcode fps, nice.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 6, 2024

I'd be game for any insight someone may have related to what gets included here.

This is my current version:
Screenshot 2024-09-06 at 11 56 39

@sjorge
Copy link

sjorge commented Sep 6, 2024

That looks fine to me, but I'll link Erik to this PR to see if he wants to chime in.

@LePips
Copy link
Member

LePips commented Sep 6, 2024

Since these can be very detailed views, I think a better design would be to have the list of sessions with bare details and we push to a session with more details instead of expanding.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 6, 2024

I can do that. What should be included in the high level view? My thought is:

  • Username
  • Content Name
  • Playback State ⏯️
  • Playback Progress

Then I can add everything else to the details page.

Also, for the progress bar, do we want the color to indicate Play/Pause or should it use the accent color?

@sjorge
Copy link

sjorge commented Sep 6, 2024

I can do that. What should be included in the high level view? My thought is:

* Username
* Content Name
* Playback State ⏯️

I think maybe we can use play and pause icon instead of the text? That should save some space

* Playback Progress

2 Colored progress bar like in web? It shows playback and transcoding progress in 1 bar.

Edit: like it's on the first one in the screenshot here #1230 (comment)

Then I can add everything else to the details page.

Also, for the progress bar, do we want the color to indicate Play/Pause or should it use the accent color?

I think accent color, using color to indicate playback is going to be less obvious than using just a string or icon.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 6, 2024

What do people think of this version:

Active Devices [v3]

Simulator Screenshot - iPhone 15 Pro - 2024-09-06 at 19 11 39

Active Device Details [v1]

Simulator Screenshot - iPhone 15 Pro - 2024-09-06 at 23 44 59

@JPKribs JPKribs marked this pull request as draft September 6, 2024 23:45
@JPKribs
Copy link
Member Author

JPKribs commented Sep 7, 2024

I'm going to call it for now. I've pushed my latest changes so anyone who wants to look it over / recommend anything this should be the latest version. There is still a TON of reorganization that I need to do. I've also overloaded the Details Page with just about every detail I could find. If there is anything else needed, I can add it but I think I've hit the point where it's more likely that we start removing things from it. Screenshots can be found below:

Active Devices

Simulator Screenshot - iPhone 15 Pro - 2024-09-07 at 01 35 23

Active Device Details 1

Simulator Screenshot - iPhone 15 Pro - 2024-09-07 at 01 36 11

Active Device Details 2

Simulator Screenshot - iPhone 15 Pro - 2024-09-07 at 01 36 15

Video In Action
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-09-07.at.01.32.59.mp4

I have some outstanding issues I want to cleanup but it may be a bit:

  • ViewModel / Stateful Changes - I might need help with this
  • Live Updating ViewModel in the Details Tab
  • Ensure the Container/Codecs are accurate since some look weird in testing
  • Reorganize Files
  • Localization

@sjorge
Copy link

sjorge commented Sep 7, 2024

The video shows it without the poster/thumbnail, I think that looks much cleaner! Also what happens when there is only a poster (Primary) available?

@JPKribs
Copy link
Member Author

JPKribs commented Sep 7, 2024

I think I'm calling images a wrong way? I need to look at that. I get images from my server but demo.jellyfin.org doesn't seem to pull images. I'm leaning towards no images on the main screen with posters on the details? I'm only able to get 4 sessions on a single screen with these posters.

Right now, I am looking at the Library Customization for Poster Type to determine with poster is used for this. I think I need to look at where I get images from since there should be some sort of placeholder if the image isn't there. I had one prior but I moved to use the ImageView and I haven't taken the time to dig into it.

@sjorge
Copy link

sjorge commented Sep 7, 2024

I think I'm calling images a wrong way? I need to look at that. I get images from my server but demo.jellyfin.org doesn't seem to pull images. I'm leaning towards no images on the main screen with posters on the details? I'm only able to get 4 sessions on a single screen with these posters.

No images on the main screen would be good to fit more, and also no need to download a bunch of extra images. 👍

Right now, I am looking at the Library Customization for Poster Type to determine with poster is used for this. I think I need to look at where I get images from since there should be some sort of placeholder if the image isn't there. I had one prior but I moved to use the ImageView and I haven't taken the time to dig into it.

I think the image type you want is Primary it's either the Show Poster, Movie Poster, or album cover for music.
The compiled URI should be /Items/<itemId>/Images/Primary, optionally you can also get them resize by adding ?fillHeight=400&fillWidth=240&quality=70 Starting in 10.9.10 it is send as attachment so it no longer displays just loading the URL in a browser window.

@erikbucik
Copy link
Member

Why are there two sections (STREAMS & ONLINE)? What's the difference?

@sjorge
Copy link

sjorge commented Sep 7, 2024

Why are there two sections (STREAMS & ONLINE)? What's the difference?

Streams are those clients actively streaming something. Online is clients marked online in jellyfin, IIRC recent activity in the last X minutes.

Also welcome 😃

@JPKribs
Copy link
Member Author

JPKribs commented Sep 7, 2024

I split this into people are are connected to Jellyfin and people who are connected and streaming something. Personally, on Jellyfin-Web, I'm always more interested in what's being streamed opposed to whose only online. So splitting these out is partially a preference from me. See an example below. In this case, the iPhone is ONLINE and the Android TV would be STREAMING.

I'm not sold on that verbiage so if there is a better way to name that I'm all ears!

image

@LePips
Copy link
Member

LePips commented Sep 27, 2024

I've had time to take a look at this and have started on my changes. Some main changes:

1 - I have split the "server connection" and "Dashboard" settings views as they serve different purposes. The former being how the client is connected to the server.
2 - I have removed the CurrentUserViewModel as we already retrieve the latest user data when we do the "server check". This is done here. While we only get this data when we perform the check, a better way in the future would be implement a background task system that runs periodically.
3 - Some changes to the task progress tracking and most views for tasks.

I have really only taken a look at the tasks and haven't gotten to the active devices yet.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 29, 2024

I'm doing a bit of travel for work but I'd love to assist when I'm back. Is there anything you'd like me to work on for this or is this mostly cleanup/design to make sure it mirrors the rest of Swiftfin?

1 - I have split the "server connection" and "Dashboard" settings views as they serve different purposes. The former being how the client is connected to the server.

Since this view only has one function (Switch URL) would it make sense to just make the existing Chevron button for this view into the Menu to select the URL instead? Flatten out the hierarchy a little? Then again, I know that view is re-used so the benefit might not be there.

@LePips
Copy link
Member

LePips commented Sep 30, 2024

No, as that view is also used for the deletion and could gain more functionality with stuff like #508.

You've made a great foundation and I'm almost done with my changes. After I am done I'll make comments on the major things I changed as well as some things I may have misguided on.

@JPKribs
Copy link
Member Author

JPKribs commented Sep 30, 2024

Sounds great! Thank you (once again) for helping me get this over the finish line!

Feel free to let me know if there's anything that I can do to help out. If you'd like, I'd be more than happy to go through and localize all of the strings when you're done with it. I was looking through your commit, and I saw that you TODO'd localizing the transcoding reasons. I'd be more than happy to do that part before we have our final merge.

@LePips
Copy link
Member

LePips commented Sep 30, 2024

Still have some work to do but most visual things should be done or has relevant TODOs.

Major changes:

  • Copying the Settings app headers, have descriptive headers in some areas where I feel is applicable. This header should not be used necessarily everywhere, but we should have in more places.
  • Instead of having the single tasks view model track the tasks, each task tracks its own progress since it can query itself.
  • Tasks can be pushed to a detail view to show more information about the task and where to edit the triggers. Most of this is still left as a TODO.
  • I misled on having a grid and row on the active devices, thinking to match the libraries for items. However the amount of information doesn't/shouldn't differ in each view type unlike item libraries, so it seemed redundant to have both. However, this can change with the issue I describe below when more space is allowed.
  • For the session rows, I switched to using ListRow since that view is made for these view contexts.
  • For sessions, we don't need to see a lot of detail on the item itself, even though the information is available. It may be desirable in the future to have the ability to deep link to that item.
  • I was under a previous assumption that devices could have multiple sessions, so then that would cause the previous reused session observer to point to wrong sessions. However that assumption was incorrect and I didn't check before I over-engineered a session updating/removing. I'll be taking a look at that again to reuse the session observer like you had it.
  • For polling, I've now learned that we want to instead delay calls after successful calls instead of view polling. Polling causes issues for cancelling previous tasks and we can live with tasks taking longer than the polling interval. The QuickConnect object also does a delay instead of polling and I haven't really had to think about these constant updates, so now in the future we have more guidance.
  • It was quick for me to add the ability to see server logs.
  • Handle music art for sessions. However I haven't been able to test with other media like books.

For many of the changes I would recommend learning from in future work:

  • Instead of making strings that are Title:, use a TextPairView. These should be in their own rows in List/Form contexts instead of in a VStack.
  • For formatted text/dates, use Text(_:format:) instead. I have been moving to this primarily in Video Player Manager Refactor #1203 and was able to bring over some of my work done there. I would recommend learning more about FormatStyle and the many (admittedly confusing) provided formats in SwiftUI.
  • There are many uses of foregroundColor. Please use foregroundStyle instead. There are still uses of it throughout the project, however they should all be replaced (and are in my other PR).
  • Most other changes are small to match designs throughout the app. Like proper usage of secondary styles, fonts, spacing, and animations. A lot of this stuff I've developed myself over my years of SwiftUI.

When testing on the iPad, I realized using the dashboard can be rather small. The user is performing administrative work and we are restricting their viewport. This is a side effect iOS and iPadOS having the same presentations but we may need to change that once we take a look at replacing our navigation stack. Meaning, iPadOS would have some entirely different views to allow them to be full screen. However, that is something I will reevaluate after we drop iOS 15.

@JPKribs
Copy link
Member Author

JPKribs commented Oct 4, 2024

I am still traveling but I had some time today at the airport to pick this back up. All I did was fix the CodeFactor issue and localize all of the Strings. I can take a look at the Edit Tasks screens this weekend. Just to confirm, is our goal there to allow updating the crons for scheduled tasks?

Other than that, I will follow the TODOs!

I'm loving the changes you made for this! It looks great!

Edit: Changing the PR Name back to just Admin Dashboard. In a previous version of this, the User could see their own active sessions but this is no longer the case. Hence, back to just admin.

@JPKribs JPKribs changed the title [iOS] Admin/User Dashboard [iOS] Admin Dashboard Oct 4, 2024
@JPKribs
Copy link
Member Author

JPKribs commented Oct 4, 2024

@LePips Do we want to mirror Jellyfin-Web like this (see below) or should an editView? The TaskTriggers don't have an ID so you can't really edit them. I assume that's why you can't do that on Jellyfin-Web. That being said, I am able to find and remove a trigger then replace it with a new one from the array as a psuedo edit. Do we want to have edit functionality or do we just want to mirror what's available on web?

Edit Task View

Simulator Screenshot - iPhone 16 Pro - 2024-10-04 at 17 08 37

@LePips
Copy link
Member

LePips commented Oct 4, 2024

Thank you for doing that work. I have completed the work that I feel needs to be done except for a few things I am willing to kick down the road:

  • I had a hard time thinking about the timer/delay change for the active sessions/tasks and it works now, so I wasn't going to worry about it. Currently, if you are trying to view the active sessions and the server is shut down/response takes a really long time the view will go to a "cancelled error" because on the poll we cancel the previous request. I'm fine with that for now and I'm sure we'll come up with a universal solution for these kind of active updates/polling in the future.
  • I thought about reverting back the active sessions observation like I said in my previous comment, but actually think my solution works better since the list constantly updates. I frankly didn't know what to do in this situation but will have to think about it.
  • Added a few TODOs for polish investigations.

However, I ran into issues of testing the tasks since I wasn't able to run a task for a long amount of time. I'm somewhat confident in the observation of the tasks so I think it's okay.


For updating a Task, we should have the Save button in the navigation bar like in Security and Custom Profile. That then posts to Paths.UpdateTask with the list of triggers that we have.

However, this PR has laid the groundwork for the dashboard view and think is done. TODOs like editing tasks and having the device logo for inactive sessions can be saved for later PRs.

@LePips LePips self-requested a review October 4, 2024 23:29
@JPKribs
Copy link
Member Author

JPKribs commented Oct 5, 2024

Sounds good. I'll hold onto what I'm doing for editing the task trigger and throw that into a new PR. I agree what exists now exceeds a lot of the original issue request so I'm happy.

I'll work on the Edit Task Triggers and look at some of the TODOs to see if I can wrap those up over the coming weeks.

@LePips LePips merged commit bc9eaca into jellyfin:main Oct 5, 2024
4 checks passed
@JPKribs JPKribs deleted the AdminDashboardSessions branch October 7, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin Dashboard
4 participants