Skip to content

feat(edit): loading spinner for pages and sheets - #1147

Open
markdaugherty wants to merge 9 commits into
adobe:mainfrom
markdaugherty:loading
Open

feat(edit): loading spinner for pages and sheets#1147
markdaugherty wants to merge 9 commits into
adobe:mainfrom
markdaugherty:loading

Conversation

@markdaugherty

@markdaugherty markdaugherty commented Jul 17, 2026

Copy link
Copy Markdown
Member

What changed

Adds a centered loading spinner to the page editor and the sheet view while their content loads. For pages, the spinner shows until the document has synced over the collab WebSocket and rendered (gated on a new contentReady flag set from onWsSync), not merely when the empty editor mounts. For sheets, it shows while data is fetched and the grid is rendered. Reuses the existing .da-loading-spinner visual style.

Why

Pages and sheets previously rendered an empty area until content finished loading, with no indication anything was happening.

Reviewer notes

  • The page spinner deliberately persists through the mounted-but-empty-editor phase; it clears on the same synced signal already used to enable editing, so content and spinner-removal stay in sync.
  • .da-loading-spinner / @keyframes da-spin are redefined locally in da-content.css and sheet.css (sized up to 32px) rather than importing shared/styles/base.css, since neither view adopts that sheet.
  • Unit tests added for both behaviors (da-content spinner render + prose contentReady on sync).

Mark Daugherty and others added 2 commits July 17, 2026 13:57
Pages previously rendered an empty editor until content loaded. Add a
centered spinner (reusing the shared .da-loading-spinner pattern) that
shows in the page editor until the document has synced and rendered, and
in the sheet view until data is fetched and the grid is rendered.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a da-content test that mounts the component and asserts the loading
spinner shows until contentReady is set, and extend the prose initProse
sync test to assert contentReady is set once the WebSocket syncs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@markdaugherty
markdaugherty marked this pull request as ready for review July 17, 2026 21:48
@mhaack
mhaack requested a review from sharanyavinod September 2, 2026 07:32
@mhaack

mhaack commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@markdaugherty can you have a look at the failing e2e tests

if (pm) pm.contentEditable = 'true';
}
// Content has synced and rendered — clear the loading spinner.
if (daContent) daContent.contentReady = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be reset on nav as well? Currently the spinner appears only on the very first /edit -> all subsequent pages still have the previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants