Skip to content

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Oct 16, 2024

Summary

Creates a DeviceView that allows for viewing all hardware that has connected to the Jellyfin Server and allows for assigning a custom name to them or deleting them.

On both the ViewModel and on the View, I do not allow the user to delete their own session. I think if were allowed to happen, then you would get 401'd and need to remove and re-add the user to get back in. 'Delete All' skips your device session. Not only does this mirror other clients with this functionality but it also avoids some weird interactions.

This should bring Swiftfin up to feature parity with Jellyfin-Web for the All Devices page features.

Ideas

I think this is where I want this. The only feature I was realizing would be valuable is instead of a Delete All function, making the list so you can multselect, with a select all function on the list. Then, use that selection as the delete population. API-wise, there is no "Delete All" endpoint. It's always just been a forLoop to delete all Devices. There is no reason we couldn't make this a list pick instead. I stuck with Delete All to keep things matching but I'd be happy to look at the list idea too.

Weird Behavior

Custom names on this SDK version are a little weird since we can update the customName but we cannot see the custom name. I have a TODO to update the deviceName to use CustomName when available. For now, you can update the custom name but it doesn't really take affect on the Swiftfin side so visually it's a little weird but it's all working fine behind the scene.

Screenshots

DevicesView Simulator Screenshot - iPhone 16 Pro - 2024-10-15 at 23:22:58
Attempt to Delete Own Session Simulator Screenshot - iPhone 16 Pro - 2024-10-15 at 23:23:15
Delete 1 Session Simulator Screenshot - iPhone 16 Pro - 2024-10-15 at 23:23:19
Delete All Sessions Simulator Screenshot - iPhone 16 Pro - 2024-10-15 at 23:23:29
Showing off the Device Icons in Action Simulator Screenshot - iPhone 16 Pro - 2024-10-15 at 23:24:51

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an initial skim.

@JPKribs
Copy link
Member Author

JPKribs commented Oct 16, 2024

I've cleaned this all up from my end and added the DeviceDetailsView. I left a TODO with the lines that need to be updated:

Swiftfin/Swiftfin/Views/SettingsView/UserDashboardView/DeviceDetailsView/Components/Sections/CustomDeviceNameSection.swift

// TODO: Enable for CustomNames for Devices: 27, 50, & 53

The version of the SDK we are on does not have CustomName available from DeviceInfo but it IS available on the Main version of the SDK. This TODO is just to uncomment out those lines and this should be good to go.

Lastly, I added DeviceCompatibilities to the view since it was looking a little bare. I don't think it's anything revolutionary but some people might find it valuable...

I split this out into Sections so, down the road, if we do have activity logs that we can join to Device/UserId, it should be pretty easy to slot that in here.|

I'd still be interested in doing #1277 (comment). building the logic was super simple and I have a working version locally but the UI was getting super cluttered. I might need a second set of eyes on just figuring out what that would look like.

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another look, I'll take a closer look at all strings later.

JPKribs and others added 2 commits October 17, 2024 13:26
… when in EditMode. Also, it's EDITMODE not SELECTMODE... Finally, make sure the SelectedDevice and SelectedDevices are both empty if the user tries to delete themselves and fails. Change how the single device delete works to confirm deleting from an array still works as needed.
@JPKribs
Copy link
Member Author

JPKribs commented Oct 18, 2024

I have these review changes merged in. I believe this should be all good! Thank you for your help and patience @LePips. I feel like this addition ended up in a really good place!

Let me know if there is anything else you need from me regarding this!

@JPKribs JPKribs requested a review from LePips October 18, 2024 03:05
@LePips
Copy link
Member

LePips commented Oct 21, 2024

I came to perform the final review/finishing touches and came into a design conundrum, realizing that the Active Devices and Devices have different list styles, which I didn't like. We want the header information which we achieve with List but we use CollectionVGrid for Active Devices. We aren't able to introspect the layout to achieve the inset grouped for the header and the plain list style for the rows, which means we will have to roll our own layout.

I was able to make a UICollectionViewCompositionalLayout that should be able to achieve what we want. I will take a look at incorporating this into my CollectionVGrid package and have this view use it here, and we may want to change Active Devices to use that as well. I may also look at renaming that action to Sessions.

@JPKribs
Copy link
Member Author

JPKribs commented Oct 21, 2024

I can make the name change and flip this to use the collection V grid (for now without the header). I just wanted to make sure you didn't have changes on a branch before I started making changes on my end and screwing that up that commit?

Lemme know and I can swap these around if needed!

@LePips
Copy link
Member

LePips commented Oct 21, 2024

I can do the rest of the work necessary for this. The renaming from Active Devices to Sessions can be done and approved separately.

@JPKribs
Copy link
Member Author

JPKribs commented Oct 21, 2024

I can do the rest of the work necessary for this.

Sounds good. I appreciate your help getting this (and many other PRs) over the finish line!

@LePips
Copy link
Member

LePips commented Oct 21, 2024

I did implement what I said I would implement: LePips/CollectionVGrid@65fbe9b, however there are issues when performing the editing that would require some more significant refactoring.

Instead, I just made the list header look like it's inset grouped but the colors are inverted in light mode when compared to the normal inset grouped style, which is okay. We will still use List and luckily these are still all list-styled, however this may change on iPadOS if we want to change the design to be full-screen and have a grid layout. That's an issue for later.

@LePips LePips merged commit a04f97e into jellyfin:main Oct 21, 2024
4 checks passed
@JPKribs JPKribs deleted the adminDashboardDevices branch October 24, 2024 20:09
@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