Swiftfin
Swiftfin copied to clipboard
[iOS] Home - Activity Indicator
Originally Discussed
https://github.com/jellyfin/Swiftfin/discussions/1269#discussioncomment-10926588
Summary
Creates an optional indicator to the HomeView that shows how many active sessions are currently active on the Server. Active sessions are online sessions where there is a nowPlayingItem.
This indicator has an action tied to it. Eventually, I want this to directly take the user to the Admin Dashboard when selected. I want to get all of the Admin Dashboard items into their own coordinator first. So, all of my other outstanding Admin Dashboard items will need to be merged first. Likely, after https://github.com/jellyfin/Swiftfin/pull/1278.
Considerations
- I baked the viewModel into the ActiveSessionsIndicator. The reason I did this was because I didn't want to add an extra viewModel to the HomeView if the setting was disabled or the user was a non-admin. This way, only when the indicator is present is the viewModel attached to the HomeView.
- This viewModel loads onAppear and every 60 seconds. The video below I changed this to every 4 seconds to capture what that looks like better.
- The loading circle around this indicator shows when the viewModel is loading new session data. It shows for a minimum of 3 seconds. The reason I set a minimum indication time was because it was looking really weird/janky when it loaded for .5 seconds then disappeared.
- The indicator & loading circle will be Color.secondary when there are no sessions. If there are 1 or more sessions, this changes to the Accent Color and a number appears to the left with a count of the number of sessions active.
Screenshot
Inactive Indicator
Active Indicator
New Setting (Only Visible when User.isAdministrator)
I had to inject currentUserSession to get this. I hope that was right?Video
Indicator (Loading Ramped to Every 4 Seconds) Video (at 1/2 speed for some reason)
Apologies, but I don't think I want the loading indicator. At least with its design, it doesn't have precedent anywhere else and is more Material than native iOS. I think we can do away with a loading design entirely and not worry about it.
Too bad .badge isn't implemented for navigation toolbar items currently, but I'll have to think about how we want the session count to look. These aren't as pressing as notifications, but it would at least match other native design.
https://ondrej-kvasnovsky.medium.com/how-display-a-notification-count-badge-in-swiftui-f4fd243f557
No worries! That was quick to throw together and I'm not in love with that design. If I ever stopped the circle spinning, it'd break next time it started spinning (at least on emulator) so I just had to swap it out for a hidden non-circle...
If we aren't worried about a loading indicator for this item I'd be more than happy to pull this part!
I'll take a crack at what you sent and I'll throw in a screenshot of what that looks like to see if this is a better fit!
Too bad .badge isn't implemented for navigation toolbar items currently, but I'll have to think about how we want the session count to look. These aren't as pressing as notifications, but it would at least match other native design.
https://ondrej-kvasnovsky.medium.com/how-display-a-notification-count-badge-in-swiftui-f4fd243f557
I'm a fan of this! I think this is an improvement. Well, minus the colors I picked. Should I change the indicator color or the badge color? I was thinking about a make just accentColor with .75 opacity white on top but I thought you'd probably have something a little less custom as a solution.
Also, I think I am going to remove the spacing 0 from the HomeView for this since it's getting a little too close together. I've included what that looks like as the last image. See below:
The badge, IMO, looks like an improvement. How do we feel about the colors. This is .fill(secondary)
I don't think that has enough contrast with itself. I think we will have to have a "divider" outside of the badge.
- https://github.com/JPKribs/Swiftfin/compare/activeSessionsIndicator...LePips:SwiftFin:activitySessionsIndicator
I don't think that has enough contrast with itself. I think we will have to have a "divider" outside of the badge.
- https://github.com/JPKribs/Swiftfin/compare/activeSessionsIndicator...LePips:SwiftFin:activitySessionsIndicator
That's a big improvement as well! Are we fine with both being accent color variations? I like it but I'm never going to claim I have good taste
Honestly probably not, but I will go through some designs.
I think them both being a variation looks fine as is, it also nice that aside from posters nearly everything somehow follows the accent color set by the user.
On the otherhand the contrast might be a bit to low between them, I wonder how it looks with either solid white or black depending on on dark or light theme. The border+text is obvious inverted from the fill color. That should give a big contrast and because it's on the grayscale spectrum like the other non accented colors, it should still look fine. 🤞
I think my plan is hold off on anything new (from my end) for AdminDashboard until https://github.com/jellyfin/Swiftfin/pull/1284 & https://github.com/jellyfin/Swiftfin/pull/1287 are merged.
Then, move all of the AdminDashboard from the SettingsView > UserDashboard into their own standalone AdminDashboard folder. From there, I want to put all of AdminDashboard views on their own coordinator and I can reference that here as the onSelect action for the indicator.
So, I think we have some time for formatting/style choice! For the time being, I think I'm going to move this to draft. I'd like to just get the indicator done in one merge rather than adding a 1 line merge after the coordinator changes.
I'm going to place this on a more permanent hold until https://github.com/jellyfin/Swiftfin/issues/1289 is resolved. I think this is going to be related to something that changed in 10.10 that would be reflected in future SDK changes. The good news, I now have a good confirmation that the error icon is working on this PR:
The bad news, with Swiftfin being on 10.8 as a base, I would guess that we're a ways off from getting on getting to a base 10.10. So this change might be a longer term ambition.
I'm going to keep this in draft for now and we'll play it by ear moving forward!
New take. What if we just put the number badge over the user profile picture instead over the EKG logo?
It's opt-in as a feature so the badge shouldn't be a shock to anyone. It stops the need for a second item in the navigation bar. Finally, the EKG button only saves 1 additional click so routing directly to the admin dashboard isn't that beneficial vs just putting this over the profile.
The only downside to this route is that it may look a little like a notification. This also would cause issues if we every need notifications in the future. But I also have no idea what notifications for Jellyfin would even be?
Edit: Just for reference/clarity, this is on pause as this will not work until this is resolved: https://github.com/jellyfin/Swiftfin/issues/1289
This branch is out of date. I am going to close this and reopen it at a later date after we update the API. At that time, I'll likely want to just rewrite this so I'm going to get this out of the open PRs until then