-
-
Notifications
You must be signed in to change notification settings - Fork 382
[iOS] Admin Dashboard - Device Management #1277
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
Conversation
There was a problem hiding this 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.
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/Components/DeviceRow.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
…deleteDevice core + deleteDevices. Delete All Devices now just uses the more generic deleteDevices
… to pass in the viewModel.devices as a 'Delete All' function
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:
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. |
…st. Hopefully this is good.
There was a problem hiding this 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.
...ngsView/UserDashboardView/DeviceDetailsView/Components/Sections/CompatibilitiesSection.swift
Outdated
Show resolved
Hide resolved
...gsView/UserDashboardView/DeviceDetailsView/Components/Sections/CustomDeviceNameSection.swift
Outdated
Show resolved
Hide resolved
...gsView/UserDashboardView/DeviceDetailsView/Components/Sections/CustomDeviceNameSection.swift
Outdated
Show resolved
Hide resolved
...Views/SettingsView/UserDashboardView/DeviceDetailsView/Components/Sections/UserSection.swift
Outdated
Show resolved
Hide resolved
… 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.
...gsView/UserDashboardView/DeviceDetailsView/Components/Sections/CustomDeviceNameSection.swift
Outdated
Show resolved
Hide resolved
...Views/SettingsView/UserDashboardView/DeviceDetailsView/Components/Sections/UserSection.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/Components/DeviceRow.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/Components/DeviceRow.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
Outdated
Show resolved
Hide resolved
#Conflicts: # Swiftfin/Views/SettingsView/UserDashboardView/DevicesView/DevicesView.swift
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! |
I came to perform the final review/finishing touches and came into a design conundrum, realizing that the I was able to make a |
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! |
I can do the rest of the work necessary for this. The renaming from |
Sounds good. I appreciate your help getting this (and many other PRs) over the finish line! |
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 |
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
Attempt to Delete Own Session
Delete 1 Session
Delete All Sessions
Showing off the Device Icons in Action