Show a loading spinner while an embedded package paints - #76
Conversation
The block renders an iframe that has to download, parse and lay out a whole package before anything appears, so a visitor sees an empty bordered frame for a noticeable moment and cannot tell whether it is loading or broken. Large packages make it worse. Cover that gap with a spinner overlaying the frame. It is revealed by the script itself, never server-side, so a visitor without JavaScript never gets an overlay that nothing could clear. It is announced with role="status" so it is not a silent animation, respects prefers-reduced-motion, and clears on the iframe's load event plus a short delay, because the theme builds its navigation right after load and clearing immediately can uncover a frame that is still blank. A 20 s timeout is the backstop for an embed whose load event never fires. The behavior ships as one enqueued asset rather than an inline <script> per block: five embeds on a page would otherwise ship five uncached copies of the same IIFE, and inline script is the first thing a hardened Content- Security-Policy drops. The plugin already enqueues its frontend CSS from the same hook. The spinner is gated on intersection. The iframe is loading="lazy", so a block below the fold has not begun loading yet -- starting the spinner at parse time would leave it spinning over an idle frame until the backstop. An IntersectionObserver with a 200px margin starts it when the browser is about to fetch, and where that API is missing the spinner just starts immediately. The catalogs under languages/ carry the one new string, "Loading content…", translated in all ten locales; CI regenerates them and fails on a stale tree.
The intersection gate was armed at rootMargin 200px on the premise that this is roughly when the browser starts fetching a loading="lazy" frame. It is not: Chrome's lazy-loading distance threshold is in the low thousands of pixels. For an embed sitting in the band between the two -- past 200px, inside the browser's fetch distance -- the frame was fetched and its load event fired while the observer had not triggered yet. The spinner then started on scroll, dropping a white overlay over content the visitor was already reading, and held it for the full 20 s backstop. Two changes, because the margin alone cannot be trusted: - A `settled` flag. Once the frame has loaded, failed, or outlived the backstop, start() is a no-op. The load handler sets it synchronously rather than after the 150 ms grace delay, so a scroll landing in that window cannot start the spinner either. This is the invariant; it does not depend on guessing another engine's scheduling. - The margin goes to 1500px, wide enough to cover the browser's fetch distance so the spinner is armed before the fetch rather than after it. The comment explaining the gate is rewritten: it argued from a premise that was simply wrong. Also narrow the "not inlined" assertion. Asserting the rendered block contains no `<script` promised more than the PR delivers -- the opt-in fullscreen button still prints an inline script, untouched here. It now asserts against the loader's own distinctive tokens.
Test in WordPress PlaygroundTest the plugin with the code from this branch:
|
The loader is enqueued in the footer and binds no earlier than DOMContentLoaded. An embed near the top of a long page -- a cached one especially -- can finish before bind() ever runs, and `load` is not re-emitted. settled therefore stayed false forever, and the moment the observer fired, start() dropped the overlay over content the visitor was already reading and held it for the full 20 s backstop. The previous round closed this race from the load side; this closes it from the bind side. Reproduced in headless Chromium against the real loader file, with the sequence forced (wait for the embed to complete, then inject the loader): the spinner appeared and never cleared. With the fix it never appears, over three consecutive runs, while a slow embed bound before load still shows and clears normally. bind() now asks the frame directly. The embed is same-origin -- the REST content proxy on this host -- and keeps allow-same-origin in its sandbox, so its document is readable, with a try/catch for the cross-origin case where it is not. The about:blank test is the part that matters: a lazy frame whose fetch has NOT started also reports readyState 'complete', on its placeholder document, and treating that as loaded would suppress the spinner in exactly the case it exists for. Verified both ways in the harness. Also drop the spinner animation under prefers-reduced-motion rather than slowing it to 3s. A slower spin is still a spin; the visible label and role="status" carry the message without it.
The scripts under assets/js/ had no unit tests at all: the only JS the CI pipeline exercised was through Playwright, which needs a full WordPress environment and cannot say why a handler misbehaved. Vitest with happy-dom covers the gap. The admin scripts are plain jQuery IIFEs loaded by wp_enqueue_script, so the harness gives them a window, a document and a global jQuery, then drives them through the DOM the way WordPress does -- no build step and no changes to the scripts themselves. CI runs `npm run test:js` next to PHPCS, before wp-env starts: neither needs a WordPress runtime, so both fail fast and cheap.
Six cases, all about when the spinner may and may not appear, because the expensive failure is not a missing spinner but one that covers content. The first fails against the loader as published: with a frame that already reported a completed real document, intersection still armed the overlay. The other five are the guards that keep the fix from overcorrecting -- a lazy frame whose fetch has not started reports readyState 'complete' on about:blank and must still get a spinner, as must a mid-flight frame and one whose document cannot be read at all. IntersectionObserver is stubbed so intersection is a decision the test makes rather than a layout accident, and contentDocument is stubbed per case because no test environment reproduces a real lazy frame's placeholder.
The loader was enqueued from wp_enqueue_scripts, so every frontend page of the site fetched it -- including the overwhelming majority with no eXeLearning block on them, where it binds nothing and exits. It is now registered on that hook and enqueued by render_block_preview(), at the point that emits the wrapper the loader looks for. A footer script enqueued while the content renders still prints, and in a REST render the handle was never registered, so the call is a harmless no-op. The three tests around it assert the split: registered but not enqueued after the hook alone, enqueued once a preview renders, and left alone by a block that renders no preview. They dequeue the handle first because WP_UnitTestCase shares one $wp_scripts across the file and the tests above render previews -- without that they assert the previous test's leftovers, which is how two of them failed in the class while passing in isolation. Also covers the two remaining loader branches in the JS suite: a frame that reports no document, and the path taken where IntersectionObserver does not exist. The catalogs under languages/ move by the line count this adds: only the `#:` source references shift, no string changes.
|
Pushed four commits addressing the review. The race was real, and reproduced. The loader is a footer script that binds no earlier than Fixed by asking the frame instead of only listening. After the The Regression-tested. The branch now carries the Vitest harness (identical to the one in #75, so the two merge cleanly in either order) and nine cases for the loader. The one that matters covers the sequence above — load fires, the loader binds afterwards, intersection happens, and Reduced motion now uses The loader now ships only where it is used. It was enqueued from Verification: |
The Vitest suite tests the embed loader, which is plain DOM; no setup file ever provided the global jQuery the config comment promised. Remove the dependency and make the comment describe the harness that actually exists.
…inner # Conflicts: # languages/exelearning-ca.mo # languages/exelearning-ca_valencia.mo # languages/exelearning-de_DE.mo # languages/exelearning-eo.mo # languages/exelearning-es_ES.mo # languages/exelearning-eu.mo # languages/exelearning-gl_ES.mo # languages/exelearning-it_IT.mo # languages/exelearning-pt_PT.mo # languages/exelearning-ro_RO.mo
1 participant
Problem
The block renders an iframe that has to download, parse and lay out a whole
package before anything appears. Until then the visitor sees an empty bordered
frame and cannot tell whether it is loading or broken. Large packages make the
gap long enough to look like a failure.
What this changes
A spinner overlays the frame while it loads.
is-loadingclass that shows itis only ever added from JavaScript, so a visitor with JS disabled never gets an
overlay that nothing could clear. The rendered HTML contains no
is-loading.role="status"witharia-live="polite"anda real "Loading content…" label, so it is not a silent spinner to assistive tech.
prefers-reduced-motionby removing the animation entirely: thering stays as a static shape and the
role="status"text carries the messageon its own.
loadevent plus a short delay. The eXeLearningtheme builds its navigation right after
load, so clearing immediately canuncover a frame that is still blank.
loadnever fires (blocked or failed),so the spinner can never sit over the frame forever.
Three implementation choices worth calling out:
One registered asset, enqueued only where it has work. Five embeds on a page
would otherwise ship five uncached copies of the same IIFE, and inline script is
the first thing a hardened Content-Security-Policy drops. The loader binds every
.exelearning-embed-loaderon the page in one pass. It is registered from thehook that enqueues the frontend stylesheet, but enqueued only when
render_block_preview()actually emits a wrapper for it to find — a page with noeXeLearning embed ships no loader request at all.
Gated on intersection, and guarded by a flag. The iframe is
loading="lazy",so the browser owns the fetch schedule. Arming the spinner at parse time would
leave it over a frame nothing is fetching yet; arming it only once the frame is
nearly visible would leave it over a frame that already finished — the browser
starts lazy fetches well before the frame is on screen, at a viewport-distance
threshold in the low thousands of pixels. So the
IntersectionObservermargin is1500px, wide enough to cover that distance, and a
settledflag makesstart()a no-op once the frame has loaded, failed, or outlived the backstop. The flag is
the invariant: it does not depend on guessing another engine's scheduling
correctly. Where
IntersectionObserveris missing the spinner just startsimmediately.
The frame is asked whether it already loaded, not only listened to. The
loader runs from the footer and binds no earlier than
DOMContentLoaded, so anembed near the top of a long page — a cached one especially — can fire
loadbefore
bind()ever runs, andloadis not re-emitted. The listeners areattached first, then the frame's document is inspected: a
readyStateofcompleteon a real (non-about:blank) document marks the frame settledimmediately. The
about:blanktest matters because a lazy frame whose fetch hasnot started yet also reports
complete, on its placeholder document — treatingthat as loaded would suppress the spinner in exactly the case it exists for. A
document the script cannot read (cross-origin) falls back to the normal event
flow.
Not applied to the shortcode path.
public/class-shortcodes.phprenders itsown
.exelearning-iframewithout the loader wrapper, so[exelearning]embedskeep their current behavior. Its poster mode defers the iframe
srcand hidesthe frame until the visitor clicks, so it needs a different trigger than
intersection rather than the same wrapper; that is deliberately left for a
follow-up.
Accessibility limitation.
role="status"announces the spinner when itappears, but nothing announces completion — the overlay is simply hidden. A
screen-reader user hears "Loading content…" and then silence rather than a
"content ready" message. Worth a follow-up; not introduced by this PR, since
before it there was no announcement at all.
assets/js/exelearning-embed-loader.jsis plain DOM — no jQuery, no WordPressglobals, no build step — so the same file is reusable by the Moodle, Omeka-S,
Nextcloud and Procomún integrations. Its only contract with the host is the
markup: a
.exelearning-embed-loaderwrapper around an.exelearning-iframe,plus the stylesheet. One caveat for anyone porting it:
bindAll()runs once, onDOMContentLoaded, so a host that injects embeds later (a single-page app) hasto call it again — the
exeLoaderBoundguard makes that idempotent.Tests
Five cases in
ElpUploadBlockTest: the preview is wrapped in the loader with thespinner and its ARIA attributes; the
is-loadingclass is absent from theserver-rendered HTML (the no-JavaScript guarantee); the loader is registered but
not enqueued by the stylesheet hook; rendering a preview enqueues it; and a page
that renders no preview leaves it unenqueued.
The "not inlined" concern is covered by asserting against the loader's own
distinctive tokens rather than "the markup contains no
<script>" — the opt-infullscreen button still prints an inline script of its own, which this PR does
not touch.
This PR also adds the plugin's first JavaScript test harness (Vitest + happy-dom,
npm run test:js) with nine cases againstexelearning-embed-loader.jsitself:a frame whose document already finished is never covered; a lazy
about:blankplaceholder and a mid-flight frame still are; an unreadable document falls back
to the event flow; the no-
IntersectionObserverpath arms immediately but stillrespects an already-loaded frame; and the normal order — cover on intersection,
clear on
load, and no spinner after aloadthat arrived first.Verification
vendor/bin/phpcs --standard=.phpcs.xml.dist -q .— exit 0npm run test:js— 9 tests passphpunitruns in CI (lint_and_test) on the branch headmake check-translations— "Translations are up to date and deterministic."wrong: the spinner is now armed before
loadrather than after it, and clearsshortly afterwards, with slow-loading controls confirming a legitimate spinner
is still shown.