[tvOS] Fix Indicators Setting Unreachable #1161
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…gsCoordinator. This is done with the goal of allowing Indicators + any future nested pages to be routable on tvOS.
5 tasks
JPKribs
pushed a commit
to JPKribs/Swiftfin
that referenced
this pull request
Aug 2, 2024
…ove tvOS Filter work since that's WIP.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Resolved by PR
In the Main branch, the Indicator button exists nested in the Settings > Customize. Because it is nested, the Settings.Router didn't seem to know what to do with it. On the current version of Main, selecting Indicators doesn't do anything. If you move the Indicators button to the main SettingsView, it's usable but it's not usable from Settings > Customize.
To resolve this, I created a CustomizeSettingsCoordinator that is activated in the same way that the existing SettingsCoordinator is when you call on SettingsViews. In this case, CustomizeSettingsCoordinator only contains the Indicators view at this time. But, in #1158 I am hoping to add a Home Sections menu to select which Tabs you want for tvOS. This CustomizeSettingsCoordinator works in this case as well.
Testing
Pull the current Main Branch and try selecting this button and notice that it does not route you to the Indicators selection view:
Settings > Customize > Indicators
Using this PR, you will be able to select this and it will bring you to this view:
Indicators View
Cause
I believe back in an earlier version, all of the settings that routed to other views existed on the main Settings Page. My guess is this was moved to the nested Settings > Customize Page. Since this version isn't on the App Store, the only people who would have noticed / tested probably aren't worried about setting Indicators so it went unnoticed.