-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
Video Player Settings Cleanup #1199
Conversation
…ayer Sections into their own files. Make tvOS mirror iOS for VideoPlayerSettings with the exception of the ColorPickers and the Gestures. Remove the Native Player Settings in favor of a single settings screen with a switch case.
I apologize, but this is not my intended goal. tvOS will be less customizable and more simple, mainly due to the fact that I don't see the need for more player or app customization and tvOS is already a pain to develop for so it makes it easy. For iOS, I have been more generous with player customization because it was easy but even I think I went too far for our own good. |
That makes sense to me. Are there any specifics you want me to pull from the tvOS side? My goal is to get all the shared elements laid out in the same way but anything we don't need on tvOS I'd be happy to strip out of this PR. |
…ter in the Sections where Routing is required.
I haven't taken a deeper look, but the only options that should be customizable are:
We shouldn't have any settings regarding gesture or UI (labels, button/menu placement). Since mpv provides more in-player customization options (subtitle position offset, color grading, etc.) I think those will have more impact than overlay customizations. |
…re no longer in tvOS
…PR. Will likely re-introduce this at a later time based on need.
@LePips I've removed the unwanted tvOS sections. In PlayerSettings, iOS and tvOS should mirror each other in layout and descriptions with the exception of items that are not found in tvOS. I've updated the screenshots to reflect the current state of this PR. All labels should be the same minus the Subtitle description no longer mentions color on tvOS. All strings should be localized. Let me know if there are any that need to get changed out |
I'm going to close this out. I think it's a good idea eventually go clean up the settings / customize settings but this PR is pretty out of date and the amount of work to copy/paste sections around really isn't that hard. I will approach this and #1222 after I'm done making new settings as part of #1271. I want to focus more on getting the Admin Dashboard functionality first! |
I was going to look at this more after #1203 since I'm removing/reworking many settings and have ideas for previewing certain settings as well as Apologies for not being more explicit on that. |
No worries! Let me know if this is the wrong direction, but do we eventually want all settings in StoredValues instead of Defaults? I feel like if I'm moving things around, I should be looking at migrating those to stored values while I'm at in? Plus, for anything new, should it be in StoredValues instead like the customDeviceProfiles? |
Probably. However, we will first have to think about how we will perform our own manual versioned migrations. We will need to migrate the values from defaults to stored values. Or, not all values necessarily need to be moved over. We will see. However, I consider that low priority. |
Summary
The goal of this PR is to add descriptors to the Video Player Settings and make sure that iOS and tvOS Video Player Settings mirror each other as closely as possible. Since the only Native Player setting is the Resume Offset, the Video Player Settings should switch based on which player is selected.
Native Player Settings is a section in iOS that changes the same Resume Offset variable as the one in the VideoPlayerSettings. Since that's the only real Native Player Setting, I have just set it up to use a single VideoPlayerSettings page with a switch case to hide everything not in use when Native Player is selected.
tvOS currently has a lot of settings missing vs iOS. That being said, I don't know how many of these settings are in-use or will be in use after #1081. Let me know if there are any settings that need to be removed. For now, the goal of this PR is to just add everything from iOS to tvOS and make sure it works. I'm more than happy to trim that down.
tvOS resumeOffset was calling an overlay which meant that any other setting that used the same number stepper couldnt get called. I moved these settings into their own views and called them using the router. I found that the number stepper used to have an issue where it checked if the value was inside the range before stepping so you could step outside the desired range (-1 or 31). I've resolved this as well.
Under the hood, I noticed that most of the sections in the Video Player Settings are called from other views but there were still a couple settings in iOS that existed inside the main view. I moved everything to its own section file and called it from the extensions. Since tvOS didn't have this structure, I mirrored this layout in tvOS as well.
Still In Progress
Figure Out a ColorPicker Solution for tvOSUnder the Hood
tvOS/iOS Section Layout
VideoPlayerSettings Switch Case
iOS
Video Player Player Pt. 1
Video Player Player Pt. 2
Video Player Player - Native Player
tvOS
Video Player Player Pt. 1
Video Player Player Pt. 2
Resume Offset View
Video Player Player - Native Player