Skip to content

Fix lazy loading on the genre pages #6799

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

Merged
merged 5 commits into from
Apr 29, 2025

Conversation

Jorgensonnj
Copy link
Contributor

Changes

Changed the query object's SortBy value to be SortName from Random. This change keeps the ordering of the shows consistent when scrolling to and from a genre section of the genre tab.

Issues

Fixes #6779 Genre tab's sorting gets applied each time a row is scrolled into view

@Jorgensonnj Jorgensonnj requested a review from a team as a code owner April 23, 2025 19:08
@jellyfin-bot
Copy link
Collaborator

jellyfin-bot commented Apr 23, 2025

Cloudflare Pages deployment

Latest commit b3df9f8
Status ✅ Deployed!
Preview URL https://dde51c7a.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs

@viown
Copy link
Member

viown commented Apr 23, 2025

Using Random here is intended in order to avoid showing the same items over and over again. The issue is that the requests will still be made when the rows are scrolled into view.

I'm assuming this issue is a side-effect from the lazy loader which loads the rows as the user scrolls down. If we want to fix this we should prevent loading rows that have already been loaded.

@Jorgensonnj
Copy link
Contributor Author

Changes

  • Undid my previous commit per the previous comment from @viown.
  • Added logic around the callback function of the intersectionObserver that executes the callback and then prevents it from future execution.

Notes

This change will impact movie genres, show genres, and images.

@viown
Copy link
Member

viown commented Apr 26, 2025

Awesome, it looks like the lazy loader never even worked as intended for the genres pages and your change fixes it. However, the image loader relies on the multiple execution of the callback in order to fill and empty the image as it intersects and goes out of view.

I believe after your change, it'll never get emptied, so I'm thinking we could add an option to the lazy loader e.g invokeOnce to implement your behaviour and use that on the movies and tv show genre pages.

@viown viown added the bug Something isn't working label Apr 26, 2025
@Jorgensonnj
Copy link
Contributor Author

Third time is the charm!

Changes

  • The creation of the intersectionObserver within the context of the lazyLoader will now pass along the observer to the callback function. This will allow control of the observer from within the callback function.
  • Lazy loaded movie and show items now explicitly unobserve the item after they have been loaded.

Notes

Image lazy loading functionality seemed to be unaffected by these changes, but please let me know if I missed something.

@@ -124,6 +128,8 @@ export default function (view, params, tabContent) {
if (result.Items.length >= query.Limit) {
tabContent.querySelector('.btnMoreFromGenre' + id + ' .material-icons').classList.remove('hide');
}
}).then(() => {
observer.unobserve(elem);
Copy link
Member

Choose a reason for hiding this comment

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

Calling unobserve after the items were fetched can leave a small window where fillItemsContainer could be called more than once. I think you can just call unobserve closer to the beginning of the function and that would be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just made a change to move up the unobserve closer to the beginning of the function.

@viown viown changed the title Fix genre tab from sorting shows randomly Fix lazy loading on the genre pages Apr 29, 2025
@thornbill thornbill added this to the v10.11.0 milestone Apr 29, 2025
@thornbill thornbill merged commit d190fdb into jellyfin:master Apr 29, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Genre tab's sorting gets applied each time a row is scrolled into view
5 participants