rework, optimize GridFragment #1812
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found 14 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 14 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 18 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 19 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Right now this is a mix of fixes and features. It will be a lot easier to review when split into multiple pull requests. Can you do that? |
|
To be more specific, can you split the "play from the grid view" feature to a separate PR. Everything else can stay in this one. |
ok will try, i know its not a ideal PR, but i had to-do quite some iterations, before i ended-up with this version, being new to the code and all. |
|
ok only has the grid changes/fixes now |
There was a problem hiding this comment.
Found 14 potential problems in the proposed changes. Check the Files changed tab for more details.
Ah ok, i remember there was special music poster handling, should be easy to fix. |
There was a problem hiding this comment.
Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.
|
ok music views are fixed, yet check the horizontal vs vertical card sizes/aspect. I understand that the horizontal special handling was to allow for slightly none 1.0 aspect posters or combined front+back covers. Yet i find the varying grid item offsets that are the result of this, kinda weird and visually unpleasing. So i personally would rather scale/cutoff those none 1.0 cases, instead of adapting the card to it? Just a idea. |
There was a problem hiding this comment.
Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Yeah I just noticed when viewing music artists that I have a single entry with a 1182x548 image, it shows as a square now in the vertical view but looks really bad and displaces all other items in the horizontal view. I think forcing all items to a single size would be best (even for single row). |
Ok will add the change to horizontal grids as well. PS: Btw off-topic, whats-up with the translations PR? Specifically with the seemingly, still auto-translations? |
|
For translations we only modify the The translations are made by actual people, not by machine. We'd rather have the app show an English string instead of a badly machine translated string. |
mhh ok, but depending on language and country, a bad google translation is often still better than the english one. In my fork i have to work with chinese documentation web-pages using google translate and this works surprisingly well, so i think google translate has a somewhat outdated "bad" reputation. |
|
ok using uniform aspect for all grid views now. |
There was a problem hiding this comment.
Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.
app/src/main/java/org/jellyfin/androidtv/ui/browsing/StdGridFragment.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/jellyfin/androidtv/ui/browsing/StdGridFragment.java
Show resolved
Hide resolved
app/src/main/java/org/jellyfin/androidtv/ui/browsing/StdGridFragment.java
Outdated
Show resolved
Hide resolved
add auto image size logic add/change Ui size names remove superfluous "Default" naming fix grid layout fragment to following screen layout/size fix card refresh not working after resume fix/optimize chunk size logic fix card padding/spacing remove vertical grid "hacks" remove unnecessary grid reload's reduce status-line update timer 400->250ms to feel more responsive
There was a problem hiding this comment.
Found 12 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Found 12 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Works perfect now, thanks! |
nielsvanvelzen
added
the
release-highlight
Changes
The grid now auto adapts to the actual screen size/dimensions, while the grid itself is density independent. So the size settings will always produce a consistent row/columns view result. The spacing's, padding, chunk size now also auto adapts. We now use the resource focus scaling % directly to determine padding/spacing.
The "default" naming, was also confusing and inconsistent. The user should directly see what the actual values are and not guess what "default" means.
My beta testers also reported that the "small" layout was still too big for large screens, so after feedback we decided on 2 new size options to address this.
Issues
fixes #1774