feat(pixel-loader): productionize swc-pixel-loader - #6605
Conversation
|
📚 Branch Preview Links🔍 Gen1 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
aramos-adobe
changed the title
aramos-adobe
added
the
skip_vrt
5t3ph
left a comment
5t3ph
left a comment
There was a problem hiding this comment.
Great work on this!
Two bugs, and some suggestions for further polish.
aramos-adobe
added
the
High priority PR review
5t3ph
left a comment
5t3ph
left a comment
There was a problem hiding this comment.
Latest animation looks nice!
See updated comment reply re: reduced motion.
Also, the icons are squared off again, but the demo you linked still shows the rounded appearance.
miwha-adobe
left a comment
miwha-adobe
left a comment
There was a problem hiding this comment.
Can we include dev warnings + tests for the icon / preset fallback? Right now an invalid value silently renders aiLogo / drops the preset with no warning. We have shared helpers in core/utils/dev-validation.ts (validateEnum / warnIf) that should make this pretty straightforward.
| import type { Cell } from './data.js'; | ||
|
|
||
| const FPS = 30; | ||
| const ms2f = (ms: number): number => (ms * FPS) / 1000; |
There was a problem hiding this comment.
Nit: This variable name was not super clear to me. Maybe something like msToFrames
|
@miwha-adobe How's this?
|
rubencarvalho
left a comment
rubencarvalho
left a comment
There was a problem hiding this comment.
Great work! I think it would make sense making this internal, though. We don't expect (nor want!) consumers to reach out to the pixel loader directly. We will compose it in our Prompt field and eventually Response status.
| // Cached shadow-root lookups. The container and cell elements are stable | ||
| // between renders, so query them once and reuse across `_playCells` / | ||
| // `_finishThenSwap` calls (including the render-less reduced-motion and pause | ||
| // paths). Invalidated in `updated` since a new icon/preset re-renders cells. |
There was a problem hiding this comment.
i could confirm there is a small bug here: on reconnect this only re-arms the ticker, but disconnectedCallback cancelled the cell animations via _cancelAnimations(). Since no property changes, Lit doesn't re-render and _playCells() never runs again. if we have a single-icon loader (no preset) that gets moved in the DOM (e.g. if we use something like appendChild, tab switch, maybe virtualization) it comes back frozen (i created a reproducible and confirmed it). if we add a this._playCells() here to mirror _cancelAnimations() on disconnect, it should fix it.
| @state() | ||
| private _displayedIcon: PixelLoaderIconName = 'aiLogo'; | ||
|
|
||
| /** Guards stale finish-then-swap completions against a newer change. */ |
There was a problem hiding this comment.
nit: this _syncTicker() runs on mount, then the first updated() sees preset in changedProperties and immediately clears + recreates the interval. So we tear down/rebuild the timer twice on every mount.
| ? this.preset | ||
| : undefined; | ||
| } | ||
|
|
There was a problem hiding this comment.
hmmm, is this JSDoc here correct? shouldn't it be on _finishThenSwap() (below?)
Match the React Spectrum loader corner rule: a cell corner rounds only when its horizontal, vertical, and diagonal neighbors are all empty. Previously a corner rounded whenever the two orthogonal neighbors were empty, so diagonally touching cells rounded corners that should stay square. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| } | ||
|
|
||
| if (changed.has('preset')) { | ||
| this._presetIndex = 0; |
There was a problem hiding this comment.
a bit of an edge/corner case but this reset runs in updated(), i.e. after render() has already drawn cells using the old _presetIndex. If you swap from one preset to another after it's auto-cycled (say preset A is at _presetIndex=3, then preset='B' is set), render() draws B[3]'s cells, then this resets to 0 and the synchronous _playCells() below queries the B[3] DOM cells but reads B[0]'s cell list. in practice, it's just that the wrong glyph animates for a frame, and if the counts differ the surplus cells flash opaque.
tl;dr is that moving this reset up into willUpdate (before render) should fix it
…spectrum-web-components into aramos-adobe/feat-pixel-loader-swc-2477
5 participants
Reviewer context: productionization summary
This PR stems from
jnjosh/pixel-loader(the prototype) and hardensswc-pixel-loaderfor production. Changes on top of that base:data.ts,animation.ts,geometry.ts, the stories meta, and the MDX (no more "prototype shell" language or internal design-spec section references).sizeis no longer a public attribute. Sizing is consumer-controlled via the--swc-pixel-loader-sizeCSS custom property (default 56px). Design has no size model yet; a typeds/m/lenum can be layered on non-breakingly later on top of the same property.pausedor under reduced motion. It now freezes on one icon (_isStaticgate + re-sync onpaused).matchMediachange listener re-evaluates when the OS setting toggles while the loader is mounted.--swc-pixel-loader-size,--swc-pixel-loader-color) via@csspropso they surface in the API table, plus a Sizing note in the MDX.toMatchAriaSnapshotfor the indeterminate progressbar.iconreflection,presetoverridingicon, the progressbar/aria-labelwiring, the paused static render (ticker regression guard), and the reduced-motion live toggle. 5/5 passing.Still open (not in this PR): dedicated behavior/preset stories, and design sign-off on the final icon set / preset names / eventual size scale.
Description
Productionizes the prototype
swc-pixel-loaderfrom thejnjosh/pixel-loaderbase branch. This PR stacks on that branch and covers the engineering hardening needed before the Prompt field generating-state branding can depend on the loader.Changes:
data.ts,animation.ts,geometry.ts, the stories meta, and the per-unit MDX so nothing reads as a "prototype shell" or cites internal design-spec section numbers a reader cannot access.sizenon-public. Dropped the freeformsize="56px"attribute. Sizing is now driven entirely by the--swc-pixel-loader-sizecustom property, which consumers override in their own CSS (for example, response-status will set it in a later project). Design does not yet have a size model, so a typeds/m/lenum would mean inventing values; keepingsizeoff the public attribute surface avoids shipping an attribute that a futuresizeenum would collide with. The enum can be layered on non-breakingly later, on top of the same custom property.setInterval) previously kept running whilepausedor underprefers-reduced-motion, so a "frozen" preset still swapped icons on a timer. Added a single_isStaticsource of truth, gated_syncTicker()on it, and re-synced the ticker whenpausedchanges. A paused or reduced-motion loader now holds one icon with no running animations.toMatchAriaSnapshotassertion for the indeterminateprogressbarand its accessible name.iconreflection,presetoverridingicon, the progressbar/aria-labelwiring, and the paused static render (including a regression guard that a paused preset runs no animations). All four test stories pass.Motivation and context
The pixel loader shipped on
jnjosh/pixel-loaderas an explicitly-marked prototype shell. It needs to be productionized before the Prompt field "generating" branding can consume it as the leading AI icon. This is part of the Conversational AI release and is tracked as a P0.References
RSP Code
Demo
Related issue(s)
jnjosh/pixel-loaderbranch.Screenshots (if appropriate)
Screen.Recording.2026-08-14.at.6.01.25.PM.mov
Author's checklist
jnjosh/pixel-loaderfeature branch, notmain; a changeset will be added when the stack targetsmain.)Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Icons and presets render and animate
iconcontrol through several values and setpresettocc,dc,exp,analyze, andmega.Paused and reduced-motion render statically
pausedin the Playground).presetset andpausedon, confirm the icon does not change over time (the ticker is suppressed).Size override via CSS custom property
swc-pixel-loader { --swc-pixel-loader-size: 24px; }.sizeattribute).Device review
Accessibility testing checklist
Required: Complete each applicable item and document your testing steps.
Keyboard (required — document steps below)
Screen reader (required — document steps below)
labelas the accessible name (default "Loading"; set a specific label such as "Generating response").aria-valuenow/percentage is announced (progress is indeterminate) and there are no duplicate or noisy announcements from the animation.