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 ItemRowAdapter.retrieveNext() Query StartIndex #1783

Merged
merged 1 commit into from Jun 18, 2022

Conversation

Andy2244
Copy link
Contributor

Changes
Make sure we reset the StartIndex, since the Query object is reused, otherwise all following queries will break.

Issues
Issue steps:

  • start a library/grid view with a lib > 50 entries
  • change focus until retrieveNext() is triggered via loadMoreItemsIfNeeded()
  • now enable any filter like "watched" or "favorites"
  • the "new" query will have the StartIndex, from the last retrieveNext() call and either display nothing or some single items or a mix of other stuff depending on the set "sortby" filter

PS: @nielsvanvelzen just a quick fix that drove me crazy, since i though my refactor stuff was the cause, yet turned out its a master branch bug.

@nielsvanvelzen nielsvanvelzen added this to In progress in v0.14.0 via automation Jun 17, 2022
@nielsvanvelzen
Copy link
Member

Wouldn't it make more sense to reset the start index when the filters change instead?

@Andy2244
Copy link
Contributor Author

Wouldn't it make more sense to reset the start index when the filters change instead?

Still mainly working on the Grid related classes, so have no clue if other stuff may want to set a persistent start index. Just noticed that retrieveNext is changing stuff willy-nilly.

My first instinct was to add a copy constructor to ItemQuery, so queries that only need temporal changes can use a copy. Yet seemed overkill so i opted for a quick fix.

@Andy2244
Copy link
Contributor Author

set a persistent start index

mhh just noticed, so actually we would want to save/restore the index state tobe 100% consistent?
The issue i see is setFilters() has nothing todo with setStartIndex(), so we cant really safeguard those.

*  make sure we reset the StartIndex, since the Query object is reused
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

I think this behavior is fine for now. We can always tweak it some more. Thanks!

v0.14.0 automation moved this from In progress to Reviewer approved Jun 18, 2022
@nielsvanvelzen nielsvanvelzen merged commit 0e3a07c into jellyfin:master Jun 18, 2022
v0.14.0 automation moved this from Reviewer approved to Done Jun 18, 2022
@Andy2244 Andy2244 deleted the fix_querys branch June 19, 2022 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v0.14.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants