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 - Active Devices Icons #1275

Merged
merged 11 commits into from
Oct 16, 2024

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Oct 15, 2024

This adds client icons when there they are active devices but have no NowPlayingItem. Also maps colors to the client to use as the background for the icons. Lastly, this forces the spacing & poster sizes. This prevents square posters from overflowing into the text when there are no other posters. This also makes sure the spacing between rows doesn't get weird when a square poster is present and all of the other posters shift up. See screenshots for details:

New Icon Examples Screenshot 2024-10-15 at 09 12 00
Spacing Changes Screenshot 2024-10-15 at 09 12 00
Force Albums into Size Screenshot 2024-10-15 at 09 14 24

@JPKribs
Copy link
Member Author

JPKribs commented Oct 15, 2024

I think the only weird part from this is that other clients are just parsing the DeviceName and ClientName to get the DeviceType and, if needed, browser type. I've recreated this logic in my DeviceType enum but I don't love how complicated it is. I'd much prefer fuzzier match where instead of "Swiftfin iOS, Swiftfin iPadOS, Swiftfin tvOS, Jellyfin iOS..." We could get apple with just "\bi(?:OS|PadOS)\b|\btvOS\b" + "Infuse" to capture all of these. I wasn't sure what the best route was so I went for the static route but I'd love to change this to be less maintenance.

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.

@LePips
Copy link
Member

LePips commented Oct 15, 2024

I did see your comment on the fuzzy matching and that sounds like a good general idea. Since that's just a nice-to-have I would want to keep it simple with natural regex matching, however a consensus wasn't made on its design.

https://forums.swift.org/t/se-0357-regex-string-processing-algorithms/57225/2

@JPKribs
Copy link
Member Author

JPKribs commented Oct 15, 2024

I did see your comment on the fuzzy matching and that sounds like a good general idea. Since that's just a nice-to-have I would want to keep it simple with natural regex matching, however a consensus wasn't made on its design.

https://forums.swift.org/t/se-0357-regex-string-processing-algorithms/57225/2

I tried to keep simple. Nothing crazy. I've tested on the following devices with no missed matches:

  • Internet Explorer
  • Google Chrome
  • MSFT Edge
  • FireFox
  • Swiftfin
  • Infuse
  • Samsung TV (Dev-Build of the Tizen)
  • Jellyfin Vue [This doesn't return a always return DeviceName as a Browser so the result is the generic other but working as expected]
  • Safari
  • Android Phone
  • LG TV (WebOS)

Let me know if you want to make any changes. My REGEX is functional but I'm not offended at all if there is any critique since I'm sure there are more efficient uses

@JPKribs
Copy link
Member Author

JPKribs commented Oct 15, 2024

Also, I added checks to parse out webOS (LG TVs) but for the life of me I couldn't find a webOS logo svg. I left a TODO for that. I don't feel a ton of pressure since other clients groups webOS as part of "Other" so we'd be the first if we had this parsed in Swiftfin.

My wife was able to help me get this into the same all-white format as the other logos. This should be good to go now:
Screenshot 2024-10-15 at 19 37 59

@JPKribs JPKribs requested a review from LePips October 16, 2024 01:59
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.

I haven't tested with all of the types but looks good to me. Thank your wife for helping with the webos logo!

@LePips LePips merged commit 50e0cfe into jellyfin:main Oct 16, 2024
4 checks passed
@JPKribs JPKribs deleted the adminDashboardDeviceIcons branch October 17, 2024 23:54
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.

2 participants