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

[WIP] Customize Home Tabs / Hide Tab Labels #1158

Closed
wants to merge 0 commits into from

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Jul 26, 2024

Relies on: #1034

This PR sets out to add some customization to the Navigation Bar. This feature is supposed to help with scenarios like 'I only have movies, why do I need a TV Shows section?' or 'I don't want the Home screen first.' This exists in both iOS and tvOS.

Additionally, this PR adds the ability to turn on/off the section labels, mirroring the setting in Jellyfin-Expo.

Screenshots

Setting Location

Simulator Screenshot - Apple TV 4K (3rd generation) - 2024-08-13 at 17 49 32

Settings
Simulator.Screen.Recording.-.Apple.TV.4K.3rd.generation.-.2024-08-13.at.17.50.45.mp4
Re-Arranged With Labels

Simulator Screenshot - Apple TV 4K (3rd generation) - 2024-08-13 at 17 52 29

Re-Arranged With Labels Turned Off

Simulator Screenshot - Apple TV 4K (3rd generation) - 2024-08-13 at 17 52 13

Outstanding Items

  • Resolve Merge Conflicts
  • Enable iOS Tab Changes
  • Refresh tvOS & iOS Home Views on Change
  • Localization
  • Remove/Replace Work Done Below:
    I've added a check to refresh the MainTabCoordinator on tvOS when the HomeLabels or HomeSections are changed. This seems to work well but I don't know if I did it the right way... I did this by making the MainCoordinator observable and making it refresh the MainTabCoordinator underneath it. This seems to work but to make it refresh the home view, I added a **self.objectWillChange.send()** as part of the refresh. Let me know if there is a cleaner way to do this!

@JPKribs JPKribs changed the title [Looking for Feedback] tvOS & iOS Navigation Bar Customizations Feedback Welcome - [tvOS] Customize Home Tabs / Remove Tab Labels Aug 2, 2024
@JPKribs JPKribs changed the title Feedback Welcome - [tvOS] Customize Home Tabs / Remove Tab Labels [tvOS] Customize Home Tabs / Remove Tab Labels Aug 2, 2024
@JPKribs JPKribs changed the title [tvOS] Customize Home Tabs / Remove Tab Labels [tvOS] Customize Home Tabs / Hide Tab Labels Aug 2, 2024
@JPKribs JPKribs marked this pull request as ready for review August 2, 2024 05:45
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'm glad this is taking shape and thank you for starting this.

1 - It's good that this could initially be done without #1034 but I would prefer we do that first so that tabs can be updated live instead of requiring a restart of the app. The attempt at adding Observable (just adding the protocol doesn't do anything) with Published is incorrect and shouldn't be/can't be mixed together.

2 - I would actually like this on iOS as well. Users would be able to put Live TV options as tabs, specific libraries, or other views I'm sure we'll make in the future. This specifically would be more complex since we would have to check Live TV and other access permissions if they are no longer valid. I will be implementing something similar for the SuperUser and local user customization, so I'll be hand-wavy and say let's wait to see what I do there.

3 - I had the thought to make the first tab, regardless of what it could ever be, to have the settings button. This would remove its strong relationship with Home.

4 - I'll have to think about how we implement the re-arranging menu.

@JPKribs
Copy link
Member Author

JPKribs commented Aug 3, 2024

1 - It's good that this could initially be done without #1034 but I would prefer we do that first so that tabs can be updated live instead of requiring a restart of the app.

I will go ahead and leave this as a draft until #1034 is addressed! This PR will be more of a reference since I'm sure there will be some other changes required to make this work when that is changed.

2 - I would actually like this on iOS as well. Users would be able to put Live TV options as tabs, specific libraries, or other views I'm sure we'll make in the future. This specifically would be more complex since we would have to check Live TV and other access permissions if they are no longer valid. I will be implementing something similar for the SuperUser and local user customization, so I'll be hand-wavy and say let's wait to see what I do there.

I had this completed and in up until 55a3d25. I'll go back and re-add this to this PR so it's there when I pick this back up again.

4 - I'll have to think about how we implement the re-arranging menu.

Outside of the one bug/workaround, I like the re-purposed implementation of OrderedSectionSelectorView for this purpose. I'd like to get that view in tvOS regardless of this PR since I think it's going to be very helpful for things like Enabling/Disabling filters on tvOS.

@JPKribs JPKribs closed this Aug 3, 2024
@JPKribs JPKribs reopened this Aug 3, 2024
@JPKribs JPKribs marked this pull request as draft August 3, 2024 23:58
@JPKribs
Copy link
Member Author

JPKribs commented Aug 13, 2024

Alright! This should be ready. Localizations and live updating the main tabs need to be done still but I will wait on #1034 before I pick this up again.

@JPKribs JPKribs changed the title [tvOS] Customize Home Tabs / Hide Tab Labels Customize Home Tabs / Hide Tab Labels Oct 14, 2024
@JPKribs JPKribs changed the title Customize Home Tabs / Hide Tab Labels [WIP] Customize Home Tabs / Hide Tab Labels Oct 14, 2024
@chickdan
Copy link
Contributor

IMO this is a small enough change to not need to wait on #1034, having to restart the app is a little annoying but theoretically a user will only be making changes once or twice. Including Settings as a tab option and adding an alert to inform users the app will need restarted would make for a solid MVP. Though I'm someone who prefers to get things in and make iterative improvements.

@JPKribs
Copy link
Member Author

JPKribs commented Oct 15, 2024

IMO this is a small enough change to not need to wait on #1034, having to restart the app is a little annoying but theoretically a user will only be making changes once or twice. Including Settings as a tab option and adding an alert to inform users the app will need restarted would make for a solid MVP. Though I'm someone who prefers to get things in and make iterative improvements.

@chickdan

I wouldn't personally be opposed to that. That being said, I think this PR is pretty out of date now so resolving conflicts on in it's current PR is gonna be a chore. In addition to this, I think one of the direction @LePips was referring to was the ability to pin something like a specific library. Which, I didn't account for in this version.

I'm game to pick this back up but I'm going to be throwing a lot of my time of Admin Dashboard items (#1271) for a bit so if you wanted to take anything valuable from this PR and work on this I would not be offended at all! Just lemme know and I can close this one out to avoid confusion. Otherwise, once I get all of #1271 completed, split out, and done I can take another look at this.

@JPKribs JPKribs closed this Nov 19, 2024
@JPKribs JPKribs force-pushed the tvOSTabConfiguration branch from 6d96117 to 687cfa6 Compare November 19, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants