fix: keep spinner up until client-drawn widgets have painted - #1
Open
ivokwee wants to merge 1 commit into
Open
Conversation
'shiny:value' fires when a value reaches the browser, not when the widget has drawn it. For plotly and iheatmapr the value *is* the drawing instruction, so hiding the spinner on that event leaves the container blank for the entire client-side draw -- which for a large scatter or heatmap is the slow part, and is the whole of the wait when a plot is redrawn without a server round-trip. Keep the spinner visible for those widget types until the output element actually contains an svg/canvas, capped at ~10s so a widget that never paints cannot leave it spinning. Tables, HTML and server-rendered plot images are complete at 'shiny:value' and are unaffected. Version bumped so the htmlDependency path changes and browsers do not serve a cached spinner.js. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
spinner.jshides the spinner onshiny:value. That event fires when the value reaches the browser — which for a client-drawn widget is before it draws anything. For plotly and iheatmapr the value is the drawing instruction, so the spinner disappears and the container sits blank for the entire client-side draw.For a large scatter or heatmap that draw is the slow part. It is also the whole of the wait whenever a plot is redrawn without a server round-trip.
Noticed in Omics Playground, where every
PlotModuleUI()plot goes throughuseSpinner(), so it affects all plotly cards app-wide.Fix
For plotly/iheatmapr outputs only, keep the spinner up until the output element actually contains an
svg/canvas, polled onrequestAnimationFrame. Capped at ~10s so a widget that never paints (empty data, a render error swallowed by the library) cannot leave the spinner running forever.Everything else — tables,
htmlOutput, server-rendered plot images — is complete atshiny:valueand takes the original immediate-hide path, unchanged.showSpinner/hideSpinnerare untouched, so thevisibilityhandling from 7efb55b still applies: the container keeps its layout box while hidden, which plotly needs in order to measure itself while drawing.Version bump
addBigLoaderDeps()builds thehtmlDependencyfrompackageVersion(), and that version is part of the served asset path. Bumped 0.0.2 → 0.0.3 so browsers don't serve a cachedspinner.js.Testing
Verified in Omics Playground (
components/app_qsee): opening a board with a heavy plotly figure now shows a continuous spinner through both the server compute and the client draw, with no blank gap before the plot appears. Tables and non-widget outputs behave as before.@ESCRI11 — you wrote this package, so you're the right reviewer. The main thing worth your eye is the widget-detection list in
isClientDrawnWidget(): I covered plotly and iheatmapr because those are what Playground uses. If there are other client-drawn widget types you'd expect bigLoaders to handle (DT, visNetwork, ggiraph), say the word and I'll extend it.🤖 Generated with Claude Code