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

Add home sections #210

Open
wants to merge 2 commits into
base: master
from
Open

Conversation

@nielsvanvelzen
Copy link
Contributor

nielsvanvelzen commented Nov 25, 2019

I'm currently working on moving my personal library from Plex to Jellyfin and one of the things that annoyed me is that the sections I made in the web version weren't available in the TV app. So I implemented it!

I tried to make it look exactly the same as the web version but some things are still different:

  • The library buttons are not implemented as I tried but couldn't find a nice way to implement it
  • The "Latest Additions" section is split in the web version with one row for each media type, my implementation shows everything in a single row
  • Editing the sections is not supported (I might add it another time in a separate pull request)

It took quite some time to get familiar with this part of the code because I haven't used Java in a few years (I use Kotlin primarily) and the structure of the app doesn't make a lot of sense to me because there is strong cohesion between most classes.

@thornbill

This comment has been minimized.

Copy link
Member

thornbill commented Nov 25, 2019

Thanks for the PR! The code is certainly a bit of a mess structurally. We’ve been trying to clean it up some as we go, but it’s a slow process. Just a heads up it may be a few days before this gets reviewed.

@nielsvanvelzen nielsvanvelzen force-pushed the nielsvanvelzen:homesections branch from b77746c to 2fa75dd Dec 2, 2019
@nielsvanvelzen

This comment has been minimized.

Copy link
Contributor Author

nielsvanvelzen commented Dec 2, 2019

I rebased the branch as there were some conflicts in strings.xml.

@nielsvanvelzen

This comment has been minimized.

Copy link
Contributor Author

nielsvanvelzen commented Dec 4, 2019

I realized that the "latest additions" row is really annoying when all media types are showing. I also noticed that the feature of the web-home screen where you can select which (video-) libraries show in a single row doesn't work in my version.

This change should probably not be merged unless some more features are added, I'm not sure if I have time in the next few weeks to implement this as I expect it requires quite a lot of changes in the code to add this without using hacks to get it working.

@thornbill

This comment has been minimized.

Copy link
Member

thornbill commented Dec 4, 2019

@nielsvanvelzen I noticed a few minor issues in my testing also.

  • User settings for sections need merged with the defaults (the preference only seems to contain a value if it does not match the default)
  • Card heights are inconsistent in some rows
  • Selecting logout does not seem to work

If you don't mind, I may continue your work here as this is a really nice feature! I should have some availability this week to work on it.

@nielsvanvelzen

This comment has been minimized.

Copy link
Contributor Author

nielsvanvelzen commented Dec 5, 2019

I don't mind! I assume you have more knowledge about the code base than me, I'm curious how you would add the missing features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.