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

Dynamically calculate episode row height on LazyEpisodeTable init #3842

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mikiher
Copy link
Contributor

@mikiher mikiher commented Jan 15, 2025

Brief summary

Instead of using a fixed size for episodeRowHeight, it is given a default value but recalculated when the episode table is mounted.

Which issue is fixed?

This fixes #3511. While this is marked as related to #3013, I believe they're not actually related.

In-depth Description

The bug was reproduced by manually setting the root font-size to a larger value (I suspected that this might be reason, because the reporter mentioned system scale changes).

The original code hardcodes episodeRowHeight value to 176 px (which is indeed the row height when the root font size is the default 16px). episodeRowHeight is used to make scroll calculations to decide which episodes to mount, and when it is different from the actual value, it results in mounting incorrect episodes.

The fix determines episodeRowHeight from the actual dom element of the first episode (or from the "no-episodes" element, if no episodes exist), which fixes the issue above.

How have you tested this?

Used Chrome settings to change the root font-size value, and ensured scrolling worked OK after reloading the page.

note: this fix doesn't automatically react to changes in the root font-size - it only recalculates episodeRowHeight in the init() function. I can make it reactive, but it seems unnecessary, as this is not something that users play with frequently.

@mikiher mikiher marked this pull request as ready for review January 15, 2025 09:00
@advplyr
Copy link
Owner

advplyr commented Jan 15, 2025

I don't think it is necessary to make this reactive. Works great, thanks!

@advplyr advplyr merged commit 6057930 into advplyr:master Jan 15, 2025
5 checks passed
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.

[Bug]: When scrolling through episodes in a podcast the listing of items disappears
2 participants