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

Fix home image size bug. Update all home view sections - including those previously hidden #1530

Merged
merged 21 commits into from
Dec 5, 2023
Merged

Conversation

1hitsong
Copy link
Member

@1hitsong 1hitsong commented Nov 22, 2023

Changes

Fixes home section image size bug.

Reworks update logic so it refreshes all data (including sections that previously had no data and were hidden). This will add/remove sections as needed based on current data.

When returning to the home view, we attempt to refocus the item the user was previously focused on. If the item is gone, then the content of the row will shift left and the new item in its place will gain focus. If there is no item to take its place, then the first item in the row will gain focus. If the row no longer exists, then the rows will shift up and the new row that took its place will gain focus. If there is no row to take its place, focus will remain on the 1st item of the 1st row.

Creates a new user setting to use the web's home section arrangement. This setting is off for users who upgraded from the previous version of the client, and automatically on for new users.

Issues

Fixes #1493
Fixes #1534

@1hitsong 1hitsong requested a review from a team as a code owner November 22, 2023 14:35
@1hitsong 1hitsong added the bug-fix This fixes a bug. label Nov 22, 2023
@1hitsong 1hitsong changed the title Fix home image size Fix home image size bug. Update all home view sections - including those previously hidden Nov 22, 2023
Copy link
Member

@cewert cewert left a comment

Choose a reason for hiding this comment

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

The main nasty bug was fixed but there is a new focus bug. When hitting back to return to the home screen, focus is always at the top row(my media for me) instead of the row you came from.

@cewert
Copy link
Member

cewert commented Nov 23, 2023

Adding this here so we don't forget... We talked in chat about using brighterscript constants or enums for the different items size arrays. There are 3 I think:

  • Wide poster
  • Movie poster
  • Music album

components/home/HomeRows.bs Outdated Show resolved Hide resolved
components/ImageSizes.bs Outdated Show resolved Hide resolved
components/home/HomeRows.bs Outdated Show resolved Hide resolved
settings/settings.json Outdated Show resolved Hide resolved
source/utils/session.bs Outdated Show resolved Hide resolved
locale/en_US/translations.ts Outdated Show resolved Hide resolved
Fix Roku component reuse bug
Don't repeat loadingTimer
Move ImageSizes enum to source folder
Update setting text & translation
Check selectedRowItem is valid and not empty
@1hitsong
Copy link
Member Author

1hitsong commented Nov 26, 2023

Current State: this addresses the review items, but the 1st load users VS upgrade user setting still isn't working correctly.

I pushed my commits up so if someone has time to look at it, they can.

@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be reviewed. Thanks!

@jellyfin-bot jellyfin-bot added the merge-conflict This PR has a merge conflict label Nov 30, 2023
Copy link
Member

@cewert cewert left a comment

Choose a reason for hiding this comment

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

  • Rows aren't being added when returning to the home page. Load the home page with an empty continue watching row, open a movie, use a client to play the movie at a random postion then pause the movie, now hit back in roku client and notice the continue watching row doesnt get created.
  • The text showing the number of items for each row (1 of 9) "flickers" when the page is being updated. On prod client, the numbers mostly don't flicker at all but sometimes will flicker once. This PR flickers at least twice it seems, sometimes 3 times.

@jellyfin-bot jellyfin-bot removed the merge-conflict This PR has a merge conflict label Dec 2, 2023
components/home/HomeRows.bs Outdated Show resolved Hide resolved
@1hitsong 1hitsong merged commit 3e73f8d into jellyfin:unstable Dec 5, 2023
9 checks passed
@1hitsong 1hitsong deleted the fixHomeImageSizes branch December 5, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix This fixes a bug.
Projects
Status: Done
3 participants