Skip to content

Deprecate SplitFormWindowView & LearnMore Types#1838

Merged
LePips merged 32 commits intojellyfin:mainfrom
JPKribs:comboSettings
Dec 16, 2025
Merged

Deprecate SplitFormWindowView & LearnMore Types#1838
LePips merged 32 commits intojellyfin:mainfrom
JPKribs:comboSettings

Conversation

@JPKribs
Copy link
Member

@JPKribs JPKribs commented Dec 6, 2025

Summary

Starts the process for: #1222

SplitFormWindowView currently causes issues with building for both iOS and tvOS as it only exists in tvOS. Additionally, I understand that we are moving away from the builder pattern in favor of generics. My goal with this PR is to build the foundation so I can start migrating all of the settings over to a unified configuration.

I've created a PlatformForm that mirrors our existing SplitFormWindowView for tvOS and a standard form layout for iOS. I've added some logic for initializing the SplitFormWindowView.descriptionView since we have a pattern we re-used with the systemImage and 400 width but this builds this in as an init. I've also added our recurring logic in at this level so it's easier to maintain as we change things.

I was trying to figure out the right way of doing this. Whether I should do a struct where I basically retain SplitFormWindowView as the tvOSView of a PlatformView or if this should be a protocol like how I did this now. I preferred the protocol since I believe this is cleaner to just have a single contentView instead of body { SplitFormWindowView }. Let me know if I went the wrong way with this!

I think I will want to move CaseIterablePicker to shared and remove ListRowMenu or vice versa to have a single type for this.

If this all looks good, I would like to start moving form views over to a shared state when applicable in iOS and tvOS. This should resolve #1222 as part of this as well.

No screenshots as this is visually the same on both platforms.

@JPKribs JPKribs added developer Alters the developer experience iOS Impacts iOS or iPadOS tvOS Impacts tvOS labels Dec 6, 2025
…re this is used.

TODO: Figure out a way to unify LearnMoreButton and LearnMoreButton to pull from the same source
@JPKribs
Copy link
Member Author

JPKribs commented Dec 6, 2025

Let me know if I am off base with this. I just also added the LearnMoreModal since this is the only place we use this. I am trying to wrap my head around something where you pass in a single like @LabeledContentBuilder or maybe like [Section: LabeledContent] that then applies both the LearnMoreButton under the correct iOS section and applies the LearnMoreModal to the correct focused section on tvOS.

This might not be possible. For now, I have this version where the learn more context is an optional input as a @LabeledContentBuilder that applies to tvOS but iOS would still need to put it in the sections. This is an improvement but I feel I can do better.

My only other thought is like a FormSection struct that handles focus for elements for tvOS and just adds a @LabeledContentBuilder. Then, on iOS this would add a LearnMoreButton under the section footer. For this SettingsView, there currently is not a LearnMore... that is required but I think it would be valuable to add one to the VideoPlayerType that just explains Swiftfin v Native.

@LePips
Copy link
Member

LePips commented Dec 7, 2025

You're not too far off of what I've thought about to consolidate these views. To share the Learn More functionality, we would use FocusedValue. Here's a little sample I've whipped up for reference.

SharedPlatformSettings.zip

@JPKribs
Copy link
Member Author

JPKribs commented Dec 7, 2025

You're not too far off of what I've thought about to consolidate these views. To share the Learn More functionality, we would use FocusedValue. Here's a little sample I've whipped up for reference.

SharedPlatformSettings.zip

Oh perfect! I'll take a look at this tonight

@JPKribs
Copy link
Member Author

JPKribs commented Dec 7, 2025

Okay! This is all functioning how I would expect this to now! I'm happy with where this stands but I'm not sure about naming/deprecation. Some of the items that I moved around are basically just our existing SplitFormWindowView, LearnMoreButton, and LearnMoreModal but now in different places. I feel the functionality is there but the naming doesn't make a lot of sense.

Finally, I've added a LearnMore for the VideoPlayerType. This serves as a demo/confirmation that this works how we want and I think this is some valuable information to more casual users who might not look at our documentation. If we don't like this part, I am happy to remove it but I believe this serves as a good demo of this.

The description text for these LearnMore values is also up for debate. I tend to be a little wordy so feel free to lemme know if this should be reduced/changed.

Only other question. Do we like the section that leads to CustomizeSettingsView and, on iOS, AccentColor is called "Accessibility". IMO, this should be like "Customization" and the button that goes to CustomizeSettingsView should change from "Customize" to "Advanced" but I'm up for whatever. Just a good time to change this while we're changing it up.


iOS

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2025-12-07.at.01.13.24.mov

tvOS

Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2025-12-07.at.01.07.21.mov

@LePips
Copy link
Member

LePips commented Dec 7, 2025

I've changed the usage of Learn More sections to a Section overload, similar to how I've done the SecureField overload for mask toggling. We don't need to introduce a PlatformForm protocol, we can instead make a Form overload, that takes an image, and then based on the platform makes the relevant form view.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 7, 2025

Ohhh that's a much cleaner way of this! Good call, I'll work on that this afternoon. Thank you for the suggestion!

@JPKribs JPKribs changed the title Deprecate SplitFormWindowView in Favor of a Unified PlatformView for Form Data Deprecate SplitFormWindowView & LearnMore Types Dec 8, 2025
@JPKribs JPKribs requested a review from LePips December 8, 2025 02:35
@JPKribs
Copy link
Member Author

JPKribs commented Dec 8, 2025

I should have all of these changes included now. Videos should be unchanged. This should be good to go for a review now.

I moved the overloads to extensions for Form and Section. Let me know if that isn't the right place for this.


I might also want to do something similar for SplitLoginWindowView in a later PR. Something similar but for List. I will want to focus with moving all the settings over first. Probably after #1823 and #1834 as well since those are done except for migrating to these new Form and Sections.

Co-authored-by: Ethan Pippin <ethanpippin2343@gmail.com>
LePips
LePips previously approved these changes Dec 15, 2025
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've done my final cleanup. Something we'll have to consider in the future is to allow the Learn More on tvOS be selectable and present a scrollable view, in the event that the learn more is too long to be presented within its view.

@JPKribs
Copy link
Member Author

JPKribs commented Dec 15, 2025

Good call. I'll make an issue for that otherwise I'll forget. Lots to do haha

@LePips LePips enabled auto-merge (squash) December 16, 2025 03:19
@LePips LePips merged commit 43e07fe into jellyfin:main Dec 16, 2025
4 checks passed
@JPKribs JPKribs deleted the comboSettings branch December 16, 2025 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer Alters the developer experience iOS Impacts iOS or iPadOS tvOS Impacts tvOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reorganize Customization Settings

2 participants