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.

Sign up for GitHub

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

Jump to bottom

Move grid direction option to DisplayPreferencesScreen #830

Merged
merged 8 commits into from
Apr 26, 2021
Merged

Move grid direction option to DisplayPreferencesScreen #830

merged 8 commits into from
Apr 26, 2021

Conversation

linetrimmer
Copy link
Contributor

@linetrimmer linetrimmer commented Apr 19, 2021 •
edited
Loading

Changes

  • Move the grid direction option from the developer options to DisplayPreferencesScreen so the grid direction can be set on a per-library basis
  • Enable the vertical grid by default
  • Remove the empty space above the first row of the vertical grid
  • Add sizes for libraries with square images as the primary image. I'm not sure of all the library types that use square images for the primary image—I know music does and from the demo server it looks like playlists also do.

@nielsvanvelzen nielsvanvelzen self-requested a review April 19, 2021 06:06
@nielsvanvelzen
Copy link
Member

I'm not sure if we should make it the default, after using the vertical grids for a bit I noticed that photo libraries in vertical mode don't work properly, you can scroll both vertically and horizontally.

Moving it to display preferences is a great idea though.

@linetrimmer
Copy link
Contributor Author

I'm not sure if we should make it the default, after using the vertical grids for a bit I noticed that photo libraries in vertical mode don't work properly, you can scroll both vertically and horizontally.

Moving it to display preferences is a great idea though.

Do photo libraries use a square aspect ratio? I only have tv, movies, collections, and a small music library, so those were the only ones I could properly test. The way I did it requires libraries with square cards to be explicitly stated, I just wasn’t sure which to include. Also, if there are any libraries that use a mix of different aspect ratios for the default image, we could hide the grid direction option for that library type, but I’m not sure which, if any, libraries do that.

@nielsvanvelzen
Copy link
Member

Do photo libraries use a square aspect ratio?

Nope, the width changes based on the aspect ratio of the image. For vertical grids this should be changed to change the height instead.

Also, if there are any libraries that use a mix of different aspect ratios for the default image, we could hide the grid direction option for that library type, but I’m not sure which, if any, libraries do that.

I'm not sure either what sizes all types of libraries use. That's why I think it's better to leave the default option to horizontal for now and when we're confident all libraries use correct sizes we could change it.

@linetrimmer linetrimmer changed the title Move grid direction option to DisplayPreferencesScreen, set default to vertical Move grid direction option to DisplayPreferencesScreen Apr 24, 2021
@github-actions github-actions bot added the merge conflict Conflicts prevent merging label Apr 26, 2021
@nielsvanvelzen
Copy link
Member

Btw, merge conflict was caused by #824 but should be easy to fix with a rebase.

@nielsvanvelzen nielsvanvelzen removed the merge conflict Conflicts prevent merging label Apr 26, 2021
@nielsvanvelzen nielsvanvelzen merged commit 9e4b622 into jellyfin:master Apr 26, 2021
@linetrimmer linetrimmer deleted the grid_option branch April 26, 2021 20:50
Bond-009 added a commit to Bond-009/jellyfin-androidtv that referenced this pull request Sep 23, 2024
@Bond-009
Fix aspect ratio specials
72f9ab9 
Don't overwrite the aspect ratio for videos
Change return value of ImageHelper.getImageAspectRatio to not be nullable

Fixes jellyfin#1192
Possibly a regression from jellyfin#830
@Bond-009 Bond-009 mentioned this pull request Sep 23, 2024
Merged
nielsvanvelzen pushed a commit that referenced this pull request Sep 23, 2024
@Bond-009 @nielsvanvelzen
Fix aspect ratio specials
90573f6 
Don't overwrite the aspect ratio for videos
Change return value of ImageHelper.getImageAspectRatio to not be nullable

Fixes #1192
Possibly a regression from #830
nielsvanvelzen pushed a commit that referenced this pull request Sep 28, 2024
@Bond-009 @nielsvanvelzen
Fix aspect ratio specials
eb869d2 
Don't overwrite the aspect ratio for videos
Change return value of ImageHelper.getImageAspectRatio to not be nullable

Fixes #1192
Possibly a regression from #830

(cherry picked from commit 90573f6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsvanvelzen nielsvanvelzen nielsvanvelzen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@linetrimmer @nielsvanvelzen