Skip to content

Unify FontPickerView#1935

Merged
LePips merged 4 commits intojellyfin:mainfrom
JPKribs:fontPickerView
Mar 10, 2026
Merged

Unify FontPickerView#1935
LePips merged 4 commits intojellyfin:mainfrom
JPKribs:fontPickerView

Conversation

@JPKribs
Copy link
Copy Markdown
Member

@JPKribs JPKribs commented Mar 10, 2026

Summary

Straight forward combination. These are both built on top of the SelectorView which is already both an iOS and tvOS target. Some cleanup to SelectorView to use the same ListRowCheckbox() so the image sizing is consistent across the whole Swiftfin. Same reasoning & changes found here: #1823

Videos

iOS

Simulator.Screen.Recording.-.iPhone.-.2026-03-09.at.21.55.06.mov

tvOS

Simulator.Screen.Recording.-.Apple.TV.-.2026-03-09.at.21.48.48.mov

@JPKribs JPKribs added developer Alters the developer experience iOS Impacts iOS or iPadOS tvOS Impacts tvOS labels Mar 10, 2026
@JPKribs
Copy link
Copy Markdown
Member Author

JPKribs commented Mar 10, 2026

This is the second to last "loose" view that isn't combined. The following views are not in folders and not merged with into tvOS that don't have an open/closed PR for their merge:

Screenshot 2026-03-09 at 22 03 57

At this point, I think only the SearchView needs to be merged in as the all of the other highlighted items are iOS only. I think that one is an easy enough PlatformView as iOS and tvOS are very different I don't think there is room to combine that in a more clever way.

SearchView is resolved via #1752!

ServerCheckView is iOS only right now but I'm a bit confused on whether we need it or now. So that might be another view but I'll get there when I get there. I haven't looked at this at all so this could be good to bring over but last time I looked at this it was something different with iOS routing vs tvOS that might not translate into anything from this unification stuff I've been up to.

I suppose also the AppIconSelectorView if that's coming over to tvOS as well but I don't have the talent to make icons so that seems like a later project.

Copy link
Copy Markdown
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.

In testing this, there's a bug where the changing of the user session isn't happening correctly. Signing into user A and then into user B, the accent color and settings values will still reflect user A's settings (and set new values to A's settings, rather than B's). This may have been a longer lasting thing than what I've just seen here, though.

@LePips LePips merged commit b658e21 into jellyfin:main Mar 10, 2026
4 checks passed
@JPKribs
Copy link
Copy Markdown
Member Author

JPKribs commented Mar 10, 2026

In testing this, there's a bug where the changing of the user session isn't happening correctly. Signing into user A and then into user B, the accent color and settings values will still reflect user A's settings (and set new values to A's settings, rather than B's).

My initial thought, one of the items we're using for tvOS and not iOS is userAccentColor which is what we set in the settings while we use accentColor in all of our views.

https://github.com/jellyfin/Swiftfin/blob/main/Shared/Views/SettingsView/SettingsView.swift

Line 10-27, sorry don't know how to grab lines on mobile

Likely, the easiest route would be a global type alias (if that's possible) so tvOS is userAccentColor while iOS is accentColor? I can test if that distinction is actually the cause of this but that was something I noticed and didn't question/change from the SettingsView switch


Ignore the above that's just tvOS but this is applicable to iOS as well. I think we're just not updating the AccentColor on user switch.

@JPKribs JPKribs deleted the fontPickerView branch March 10, 2026 14:45
@LePips
Copy link
Copy Markdown
Member

LePips commented Mar 11, 2026

The issue isn't with the usage of the userAccentColor as that's the setting for the user. It is observed elsewhere and then sets the global accent color.

There is an issue somewhere that is re-using views. For example, if user A presents the user settings and then switches to user B, when user B presents the user settings the views will not be brand new - they will still be views that hold keys to A's settings. We just may need to stick an id() somewhere in/on SettingsView, along with checking some other views to see if this is happening elsewhere.

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.

2 participants