diff --git a/CHANGELOG.md b/CHANGELOG.md index a319b07..3ca1abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +* Fixed an issue where Plotly `FigureWidget` outputs in filling layouts could briefly render at the wrong height and then visibly jump to their final size on initial render or rerender. Plotly outputs now wait for Plotly's own resize cycle before being revealed, and Playwright coverage was added for the first visible Plotly paint. (#208, #236) * Fixed an issue where browser-originated widget buffers were not decoded back into bytes on the server before being forwarded to ipywidgets, which broke binary `update` and `custom` comm traffic from widgets running in the browser. Added Playwright regression coverage for both paths. (#151, #230) * Hardened widget teardown for re-rendered views so replacing a widget no longer emits known cleanup noise like `Widget is not attached` or dead-comm sync errors. Added Playwright regression coverage for repeated rerenders across plotly, altair, bokeh, and ipyleaflet, and wired that suite into GitHub Actions on Python 3.12. (#223) * Fixed an issue where parent widget updates could arrive before newly introduced child widget models were opened client-side, causing one-interaction lag or dropped updates in cases like `ipyleaflet` marker insertion. (#225) diff --git a/js/src/output.ts b/js/src/output.ts index 8cf3cf1..52041c0 100644 --- a/js/src/output.ts +++ b/js/src/output.ts @@ -1,5 +1,6 @@ import { HTMLManager, requireLoader } from '@jupyter-widgets/html-manager'; import { ShinyComm } from './comm'; +import { findPlotlyGraphDiv, waitForPlotlyReadyToReveal } from './plotly'; import { jsonParse } from './utils'; import type { ErrorsMessageValue } from 'rstudio-shiny/srcts/types/src/shiny/shinyapp'; @@ -76,18 +77,23 @@ class IPyWidgetOutput extends Shiny.OutputBinding { } onValueError(el: HTMLElement, err: ErrorsMessageValue): void { Shiny.unbindAll(el); + el.style.visibility = "inherit"; this.renderError(el, err); } async renderValue(el: HTMLElement, data): Promise { + const hiddenVisibility = "hidden"; + const revealVisibility = "inherit"; + const renderToken = this._nextRenderToken(el); // Allow for a None/null value to hide the widget (css inspired by htmlwidgets) if (!data) { - el.style.visibility = "hidden"; + el.style.visibility = hiddenVisibility; return; - } else { - el.style.visibility = "inherit"; } + const isPlotlyWidget = data.widget_pkg === "plotly"; + el.style.visibility = isPlotlyWidget ? hiddenVisibility : revealVisibility; + // Only forward the potential to fill if `output_widget(fillable=True)` // _and_ the widget instance wants to fill const fill = data.fill && el.classList.contains("html-fill-container"); @@ -117,7 +123,42 @@ class IPyWidgetOutput extends Shiny.OutputBinding { if (fill) { this._onImplementation(lmWidget, () => this._doAddFillClasses(lmWidget)); } - this._onImplementation(lmWidget, this._doResize); + if (!isPlotlyWidget) { + this._onImplementation(lmWidget, () => this._doResize()); + } else { + this._onImplementation(lmWidget, () => { + if (!this._isCurrentRenderToken(el, renderToken)) { + return; + } + + const plotEl = findPlotlyGraphDiv(lmWidget); + if (!plotEl) { + this._doResize(); + if (this._isCurrentRenderToken(el, renderToken)) { + el.style.visibility = revealVisibility; + } + return; + } + + // Plotly FigureWidget may first render at its internal 360px fallback, + // then resize after paint. Keep it hidden until a direct Plotly resize + // completes so the first visible paint is already settled. + void waitForPlotlyReadyToReveal(plotEl, () => this._doResize()).finally(() => { + if (this._isCurrentRenderToken(el, renderToken)) { + el.style.visibility = revealVisibility; + } + }); + }); + } + } + _nextRenderToken(el: HTMLElement): number { + const trackedEl = el as HTMLElement & { __shinywidgetsRenderToken?: number }; + const nextToken = (trackedEl.__shinywidgetsRenderToken ?? 0) + 1; + trackedEl.__shinywidgetsRenderToken = nextToken; + return nextToken; + } + _isCurrentRenderToken(el: HTMLElement, token: number): boolean { + return (el as HTMLElement & { __shinywidgetsRenderToken?: number }).__shinywidgetsRenderToken === token; } _onImplementation(lmWidget: HTMLElement, callback: () => void): void { if (this._hasImplementation(lmWidget)) { @@ -127,7 +168,7 @@ class IPyWidgetOutput extends Shiny.OutputBinding { // Some widget implementation (e.g., ipyleaflet, pydeck) won't actually // have rendered to the DOM at this point, so wait until they do - const mo = new MutationObserver((mutations) => { + const mo = new MutationObserver((_mutations) => { if (this._hasImplementation(lmWidget)) { mo.disconnect(); callback(); diff --git a/js/src/plotly.ts b/js/src/plotly.ts new file mode 100644 index 0000000..9f81060 --- /dev/null +++ b/js/src/plotly.ts @@ -0,0 +1,75 @@ +interface PlotlyEventEmitter { + on(eventName: string, callback: () => void): void; + once(eventName: string, callback: () => void): void; + removeListener(eventName: string, callback: () => void): void; +} + +const plotlyAfterPlotTimeoutMs = 5000; + +export function findPlotlyGraphDiv(root: HTMLElement): HTMLElement | null { + const plotlyEl = root.matches(".js-plotly-plot") + ? root + : root.querySelector(".js-plotly-plot"); + return plotlyEl instanceof HTMLElement ? plotlyEl : null; +} + +export async function waitForPlotlyReadyToReveal( + plotEl: HTMLElement, + dispatchResize: () => void, +): Promise { + const plotly = (window as any).Plotly; + await waitForPlotlyAfterPlot(plotEl); + + if (plotly?.Plots?.resize) { + try { + const afterResize = waitForPlotlyAfterPlot(plotEl); + await plotly.Plots.resize(plotEl); + await afterResize; + } catch (err) { + console.error("Error resizing Plotly widget before reveal:", err); + } + } + + dispatchResize(); +} + +function waitForPlotlyAfterPlot(plotEl: HTMLElement): Promise { + return new Promise((resolve) => { + const handler = () => { + cleanup(); + resolve(); + }; + + const cleanup = () => { + if (hasMethod(plotEl, "removeListener")) { + plotEl.removeListener("plotly_afterplot", handler); + } + window.clearTimeout(timeoutId); + }; + + const timeoutId = window.setTimeout(() => { + cleanup(); + resolve(); + }, plotlyAfterPlotTimeoutMs); + + if (hasMethod(plotEl, "once")) { + plotEl.once("plotly_afterplot", handler); + return; + } + + if (hasMethod(plotEl, "on")) { + plotEl.on("plotly_afterplot", handler); + return; + } + + cleanup(); + resolve(); + }); +} + +function hasMethod( + x: unknown, + key: K, +): x is T & Record unknown> { + return !!x && typeof (x as any)[key] === "function"; +} diff --git a/shinywidgets/_render_widget_base.py b/shinywidgets/_render_widget_base.py index cfdd075..26aed52 100644 --- a/shinywidgets/_render_widget_base.py +++ b/shinywidgets/_render_widget_base.py @@ -99,6 +99,7 @@ async def _render(self) -> Jsonifiable | None: return { "model_id": str(widget.model_id), "fill": fill, + "widget_pkg": widget_pkg(widget), } @property diff --git a/shinywidgets/static/output.js b/shinywidgets/static/output.js index 487a98e..02701e3 100644 --- a/shinywidgets/static/output.js +++ b/shinywidgets/static/output.js @@ -36,7 +36,17 @@ eval("__webpack_require__.r(__webpack_exports__);\n/* harmony export */ __webpac \***********************/ /***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => { -eval("__webpack_require__.r(__webpack_exports__);\n/* harmony import */ var _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! @jupyter-widgets/html-manager */ \"@jupyter-widgets/html-manager\");\n/* harmony import */ var _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(_jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__);\n/* harmony import */ var _comm__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./comm */ \"./src/comm.ts\");\n/* harmony import */ var _utils__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./utils */ \"./src/utils.ts\");\nvar _a;\n\n\n\n/******************************************************************************\n * Define a custom HTMLManager for use with Shiny\n ******************************************************************************/\nclass OutputManager extends _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__.HTMLManager {\n // In a soon-to-be-released version of @jupyter-widgets/html-manager,\n // display_view()'s first \"dummy\" argument will be removed... this shim simply\n // makes it so that our manager can work with either version\n // https://github.com/jupyter-widgets/ipywidgets/commit/159bbe4#diff-45c126b24c3c43d2cee5313364805c025e911c4721d45ff8a68356a215bfb6c8R42-R43\n async display_view(view, options) {\n const n_args = super.display_view.length;\n if (n_args === 3) {\n return super.display_view({}, view, options);\n }\n else {\n // @ts-ignore\n return super.display_view(view, options);\n }\n }\n}\n// Define our own custom module loader for Shiny\nconst shinyRequireLoader = async function (moduleName, moduleVersion) {\n // shiny provides a shim of require.js which allows