Skip to content

Add support for item information lazy loading - #459

Open
hagaygo wants to merge 3 commits into
jellyfin:masterfrom
hagaygo:lazy-load-info
Open

hagaygo wants to merge 3 commits into
jellyfin:masterfrom
hagaygo:lazy-load-info

Conversation

@hagaygo

@hagaygo hagaygo commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Addressing #455

Technically we can remove "Include people" settings and always do lazy load , since it is much faster, but some users might want to keep old behavior and wont like the side effect of info dialog is updating on the fly.

So i appended the settings text to include more information.

As mentioned on the issue, I used LLM at start for inspiration/feasibility check but re-tuned and checked the code later on.

Comment on lines +221 to +222
msgid "Include people for all items for each list request"
msgstr "Include people for all items for each list request"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change of wording really doesn't make sense, imo, and it will cause extra work of updating all the translations. I don't see any reason to change it like this.

I'd actually be in favor of removing the setting (or at least making it invisible for now in case we change our minds later), or if we are going to change it, then make the wording more like "Enable lazy loading for people" or "Load people in background." That way it would be more descriptive and easier for the layperson to understand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On my setup , I did not find any reason to keep old behavior.

If you are in favor of hiding/disabling it , I'll change it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverted the text back to original

Comment thread service.py Outdated
Comment on lines +103 to +105
include_people = settings.getSetting('include_people') == "false"
# Start the info dialog monitor (lazy load data when the info dialog is opened)
if include_people:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is arguably the most confusing way possible to write this check, and I spent several minutes trying to figure out why it wasn't behaving as expected before I realized that the setting value and check value are inverted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the code , if settings is hidden no need for this condition check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants