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 - Parental Ratings #1353

Merged
merged 11 commits into from
Dec 11, 2024
Merged

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 10, 2024

Summary

Allows settings the maximum Parental Rating for a User. Also allows changing how unrated or unrecognized ratings are handled based on Type.

The way this works on Server is ratings have a name and a number associated to them. This way, TV-14 and PG-13 are roughly the same value. This is why they are grouped together like this:

Screenshot 2024-12-09 at 19 02 14

I've done the same thing for us. It's a bit cluttered looking but I'm not sure how else to show this.

Screenshots

Dropdown Screenshot 2024-12-09 at 18 57 09
Other Options Screenshot 2024-12-09 at 18 57 23

@JPKribs JPKribs changed the title Main into Parental Controls [iOS] Admin Dashboard - Parental Ratings Dec 10, 2024
@LePips
Copy link
Member

LePips commented Dec 10, 2024

Hm, I think that there is something better we can do than just a Menu and these offerings will only ever get larger. I'll have to think about it.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 10, 2024

Looking at the other media servers, they handle this groupings like 'Teen' or 'Kid' that are more ambiguous but cleaner. On our end, just from the US one looks like:

Rating Age
Approved 0
G 0
TV-G 0
TV-Y 0
TV-Y7 7
TV-Y7-FV 7
PG 10
TV-PG 10
TV-PG-D 10
TV-PG-L 10
TV-PG-S 10
TV-PG-V 10
TV-PG-DL 10
TV-PG-DS 10
TV-PG-DV 10
TV-PG-LS 10
TV-PG-LV 10
TV-PG-SV 10
TV-PG-DLS 10
TV-PG-DLV 10
TV-PG-DSV 10
TV-PG-LSV 10
TV-PG-DLSV 10
PG-13 13
TV-14 14
TV-14-D 14
TV-14-L 14
TV-14-S 14
TV-14-V 14
TV-14-DL 14
TV-14-DS 14
TV-14-DV 14
TV-14-LS 14
TV-14-LV 14
TV-14-SV 14
TV-14-DLS 14
TV-14-DLV 14
TV-14-DSV 14
TV-14-LSV 14
TV-14-DLSV 14
NC-17 17
R 17
TV-MA 17
TV-MA-L 17
TV-MA-S 17
TV-MA-V 17
TV-MA-LS 17
TV-MA-LV 17
TV-MA-SV 17
TV-MA-LSV 17

We could just break this out into like... Anyone, kid=7, otherkid=10, teen=13, somethingteen=14, adult=17? I'm terrible at naming but this could be the cleanest route then we can move the details of this into a Learn More where we say

Kid Ratings:

  • TV-Y7
  • TV-Y7-FV

Adult Ratings:

  • R
  • NC-17
  • etc.

For reference, this is where the localized ratings live:

https://github.com/jellyfin/jellyfin/tree/master/Emby.Server.Implementations/Localization/Ratings

Here's a version of this:

Simulator Screenshot - iPhone 16 Pro - 2024-12-10 at 20 22 56

@LePips
Copy link
Member

LePips commented Dec 11, 2024

I had thought about something like that as well: to just show the shared ages of the parental ratings. I think that's the way go since we've both come across it.

I think we can make it better with a Learn More where each age lists what ratings are a part of that age cap. That will be dynamic if it ever gets more granular.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 11, 2024

I think we can make it better with a Learn More where each age lists what ratings are a part of that age cap. That will be dynamic if it ever gets more granular.

Here is my working version. Let me know what you think!

0 goes to "All Audiences". Anything 100+ I just use the name of the rating.

I wanted to use the bulletList but LearnMore takes [TextPair] and TextPair requires both title and subtitle are String. I could extend TextPair to allow subtitle to be a View.

Dropdown Dropdown Screenshot
LearnMore LearnMore Screenshot

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.

Learn More without a bulleted list looks great. However, I think the way the ages are presented is odd. The ages are for the maximum age allowed whereas we are presenting them like "this range and up aren't allowed", or could even be misinterpreted as "this range is allowed".

We can simplify that this is the Max parental rating and don't need the + in the strings.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 11, 2024

We can simplify that this is the Max parental rating and don't need the + in the strings.

Here is my latest commit. Made the StateObject change + the String change:
Screenshot 2024-12-11 at 11 40 08

LePips
LePips previously approved these changes Dec 11, 2024
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 just did some cleanup and switched LearnMoreButton to use SeparatorVStack.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 11, 2024

I just did some cleanup and switched LearnMoreButton to use SeparatorVStack.

That's a good change! Only note, without the foregroundStyle, the LearnMore inherits the style from the footer and looks like this:

simulator_screenshot_6ABDAF78-7CED-4D88-B394-BE02F95F72F5

Opposed to this:

#1353 (comment)

Not an issue just so you're aware before we merge if we want to change the section headers to Color.primary instead.

@LePips
Copy link
Member

LePips commented Dec 11, 2024

Thanks for pointing that out. I've fixed it on the LearnMoreButton level so that passing in the foregroundStyle on the callers isn't necessary.

@LePips LePips merged commit ba5c037 into jellyfin:main Dec 11, 2024
4 checks passed
@JPKribs JPKribs deleted the serverUserPC branch December 11, 2024 20:34
@JPKribs
Copy link
Member Author

JPKribs commented Dec 11, 2024

Thanks for pointing that out. I've fixed it on the LearnMoreButton level so that passing in the foregroundStyle on the callers isn't necessary.

Perfect! I'm a big fan of doing that there since I don't know if we'll need to vary that anywhere else.

Thanks for your help and feedback getting this over the finish line!

ddrccw added a commit to ddrccw/Swiftfin that referenced this pull request Jan 26, 2025
* upstream/main: (392 commits)
  [tvOS] Add pin prompt to sign-in screen (jellyfin#1383)
  [iOS] Admin Dashboard - User Access Tags (jellyfin#1377)
  [Meta] 2025 Disclaimer (jellyfin#1381)
  [tvOS] Delete User from User Selection Screen (jellyfin#1359)
  [iOS] Media Item Menu - Identify Media Item (jellyfin#1369)
  [iOS] Admin Dashboard - User Profiles (jellyfin#1328)
  [iOS] Select all Users When Editing (jellyfin#1373)
  [Meta] Automatic String Organization (jellyfin#1372)
  [iOS & tvOS] Unused Localization Cleanup (jellyfin#1362)
  [tvOS] SelectServerView Change to Menu (jellyfin#1363)
  [tvOS] Update ConnectToServerView & UserSignInView (jellyfin#1365)
  Trim Fastlane Options (jellyfin#1367)
  Update Fastlane Runner (jellyfin#1366)
  [iOS & tvOS] Localize Existing Strings (jellyfin#1361)
  [iOS] Admin Dashboard - User Access Schedules (jellyfin#1358)
  [iOS] Admin Dashboard - Parental Ratings (jellyfin#1353)
  [iOS & tvOS] Error Cleanup (jellyfin#1357)
  update (jellyfin#1356)
  Fix possible duplicate ids (jellyfin#1354)
  [tvOS] Media Item Menu - Refresh / Delete Items (jellyfin#1348)
  ...

Signed-off-by: ddrccw <[email protected]>
@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