Repository navigation
fix(convert/html): fix flickering on slide change with --one-file - #663
mmustafasenoglu wants to merge 3 commits into
Conversation
When using --one-file mode, base64-encoded video data URIs are split into multiple <source> elements by RevealJS. The existing fix only ran on the 'ready' event, so dynamically created video elements (when navigating to slides outside the initial viewDistance) were never fixed. This adds a 'slidechanged' handler that re-runs the fix on slide transitions, using a WeakSet to avoid duplicate processing of already-fixed backgrounds. Closes jeertmans#426
for more information, see https://pre-commit.ci
Up to standards ✅🟢 Issues
|
jeertmans
left a comment
There was a problem hiding this comment.
Hi @mmustafasenoglu, thanks for your contribution! I'll properly review this later, but can you make sure to address my small request change, and also sync the changes to docs/source/_static/template.html and docs/source/_static/firebase_sync_demo.html. Thanks!
- firebase_sync.html: same WeakSet + slidechanged handler - docs/source/_static/template.html: sync with main template - docs/source/_static/firebase_sync_demo.html: sync with firebase template - revealjs.html: var -> const (review feedback)
|
Done! Changes applied:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #663 +/- ##
==========================================
- Coverage 79.23% 79.09% -0.14%
==========================================
Files 23 23
Lines 2177 2177
==========================================
- Hits 1725 1722 -3
- Misses 452 455 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @mmustafasenoglu, my apologies for the delay. I have reviewed your changes and tested locally, and I cannot observe any repeatable, visual change. I rendered the Do you have a set of example slides on which I could validate your contribution? |
|
Hi @mmustafasenoglu, nice fix! I have PR #678 open that also addresses #426 from a different angle — it pre-converts data URIs to Blob URLs at load time so the browser doesn't re-decode massive base64 strings on every transition. Your approach (listening to slidechanged + WeakSet tracking) handles the root cause for dynamically created slides, while the Blob URL pre-conversion is a complementary performance optimization. Both can coexist without conflicts since they modify non-overlapping sections of the templates. FYI @jeertmans — these two PRs are complementary and can be merged together. |
Closes #426
When using
--one-filemode, base64-encoded video data URIs are split into multiple<source>elements by RevealJS. The existingfixBase64VideoBackgroundonly ran on thereadyevent, so dynamically created video elements (when navigating to slides outside the initialviewDistance) were never fixed, causing a visible blank frame flicker on slide transitions.This adds a
slidechangedhandler that re-runs the fix on every slide transition. AWeakSettracks already-processed backgrounds to avoid duplicate MutationObserver setup.