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 support for vertical grids #475

Merged
merged 4 commits into from
May 22, 2020

Conversation

nielsvanvelzen
Copy link
Member

@nielsvanvelzen nielsvanvelzen commented May 14, 2020

This adds some more poetry and spaghetti to the grid code to allow vertical grids! A user preference is added and will default to the new vertical view.

Changes

  • Add new preference "grid direction"
  • Can be vertical or horizontal
  • Add new vertical grid direction

Issues

@AndreasGB
Copy link
Contributor

Grade A poetry, love the juxtaposition of horizontal and vertical in the instanceof repetitio.

@nielsvanvelzen
Copy link
Member Author

Unfortunately the grid presenters don't implement an interface for their shared functionality :(

@nielsvanvelzen nielsvanvelzen marked this pull request as ready for review May 15, 2020 19:48
@nielsvanvelzen nielsvanvelzen requested a review from thornbill May 18, 2020 06:56
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems to be working well!

The only other issue I noticed is the horizontal layout no longer has horizontal margins (or they are just too small).

large horizontal grid

@nielsvanvelzen nielsvanvelzen requested a review from thornbill May 21, 2020 11:30
@jellyfin jellyfin deleted a comment from anthonylavado May 21, 2020
app/src/main/res/xml/preferences.xml Outdated Show resolved Hide resolved
@nielsvanvelzen nielsvanvelzen requested a review from thornbill May 22, 2020 06:30
@thornbill
Copy link
Member

Looks good now! Just a minor conflict that needs fixed!

Copy link
Contributor

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- app/src/main/java/org/jellyfin/androidtv/ui/GridFragment.java  9
         

Clones removed
==============
+ app/src/main/java/org/jellyfin/androidtv/browsing/StdGridFragment.java  -1
         

See the complete overview on Codacy

@thornbill thornbill merged commit 3bca99d into jellyfin:master May 22, 2020
@nielsvanvelzen nielsvanvelzen deleted the vertical-grids branch May 30, 2020 09:53
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.

BrowseGridFragment doesn't feel native to the app because of wrong scroll direction
4 participants