Skip to content

feat(pwa): prompt before applying a new version (issue #42) - #95

Merged
tibroc merged 2 commits into
mainfrom
feat/pwa-update-notice
Aug 28, 2026
Merged

feat(pwa): prompt before applying a new version (issue #42)#95
tibroc merged 2 commits into
mainfrom
feat/pwa-update-notice

Conversation

@tibroc

@tibroc tibroc commented Aug 28, 2026

Copy link
Copy Markdown
Member

Closes the second half of #42 (the install hint shipped earlier). Implements
docs/specs/pwa-update-notification.md — that spec file lives on the branch of
PR #93 and isn't on main yet, so it isn't part of this diff.

What changed

  • registerType: 'autoUpdate''prompt'. A new deploy's worker installs and
    waits; nothing reloads on its own.
  • UpdateNotice (mounted once in DashboardShell) owns registration via
    useRegisterSW and shows a calm, bottom-left role="status" notice —
    "Neue Version verfügbar." + Neu laden + a dismiss ×. main.tsx no longer
    registers the worker itself (comment there points at the new owner).
  • Long-lived clients get told: registration.update() every 60 min (a
    constant, not config) and on every visibilitychange → visible.
  • Dismissal is page-load-scoped — nothing persisted, and a later update
    re-shows the notice.
  • Tokens only, no new hex; the entry animation is a CSS class so the global
    prefers-reduced-motion reset neutralises it. 44px targets at phone widths,
    compact sizes above the md: breakpoint.
  • The notice reserves an 88px bottom-right corner at every width so it can never
    cover the assistant launcher; asserted in e2e.
  • Docs: docs/02 §11.1 rewritten (the auth-safe caching block is untouched), one
    README line under Deployment → Notes.

The e2e seam

window.dispatchEvent(new CustomEvent('wolke:sw-need-refresh')) shows the same
notice. Playwright can't produce a second worker version against one embedded
binary, so this is how the viewport suite sees the real component in the real
layout. It's honest production code: with no worker waiting, Reload falls back
to location.reload().

Tests

  • Vitest (UpdateNotice.test.tsx, pwa-update.test.ts): hidden by default;
    shown on needRefresh; Reload calls updateServiceWorker(true); dismiss hides
    without touching storage; re-shows on the next update; both locales; axe clean;
    the seam and its plain-reload fallback; polling interval, visibility check,
    teardown, and a rejected check. Mutation-checked (forcing the component to
    render nothing fails 7 of them).
  • Playwright (e2e/update-notice.spec.ts, 3 tests × all six viewports):
    appears on trigger, expectViewportHealthy in the open state, doesn't
    overlap the top-bar nav or the launcher corner, dismiss restores a clean
    layout.

Gates: tsc --noEmit (app + e2e), vitest run 238 passed, make e2e 133
passed / 6 skipped, go test -race ./... clean, embedded build green.
internal/web/dist is untracked and stayed that way.

The first push failed CI's npm run lint on react-hooks/set-state-in-effect:
the notice cleared its dismissal from an effect watching needRefresh. (I first
misread that as a pre-existing full-icon.tsx error — git stash had left the
new untracked files in place, so the "unchanged" comparison was worthless.) The
follow-up commit derives it instead: count the updates this page load has heard
about, remember the count a dismissal was made at. Same behaviour, no
render-then-correct pass. Lint is now 0 errors / 8 pre-existing warnings, and
all gates were re-run green after the fix.

Manual verification of the real SW update path (spec §3)

Driven against the embedded binary on :8472 with the real mock IdP session —
build, load, confirm the worker controls the page, then change a user-visible
string, make build, restart the server, and watch the untouched tab.

  • Poll path (interval temporarily shortened to 10s): the notice appeared in
    the tab with no interaction; clicking Neu laden swapped
    index-DYB9gIZ6.jsindex-DC2lmAju.js and the new build's string was on
    the page. ✅
  • Resume path (interval back at the real 60 min, so only a resume can fire):
    after the deploy the tab sat idle with no notice, then a visibilitychange
    produced a waiting worker and the notice; Reload landed
    index-jJWePKAY.jsindex-CorI8Yds.js with the new string. ✅

Reported honestly: Playwright never puts a page into the hidden state —
bringToFront fires no visibilitychange, headless or headed, and
Page.setWebLifecycleState doesn't either — so in that second run the browser's
firing of the event was dispatched by the harness. Everything downstream (the
production listener, the real registration, the update fetch over HTTP, the
waiting worker, the notice, the reload) was real. A genuine phone-unlock resume
is the one link not exercised end-to-end here. Chrome DevTools MCP failed to
attach in this environment ("Target closed"), which is why this ran through
Playwright rather than an interactive browser session.

Desktop install UI is still deliberately left to the browser (unchanged from the
install-hint PR).

Left unmerged for supervisor review.

tibroc added 2 commits August 28, 2026 15:54
The service worker ran in autoUpdate mode: a new deploy's worker activated
silently and only took effect on the next full navigation. A long-lived tab or
the installed PWA kept running the old bundle indefinitely and never found out,
so fixes we ship didn't reach open clients.

Switch to prompt mode. A new worker installs and waits; <UpdateNotice> (mounted
once in the dashboard shell) shows a polite role="status" notice with a Reload
button, and only that click activates the worker and reloads. Nothing reloads on
its own — admin forms exist, and an unrequested reload eats input. Dismissal
lasts for the page load only; a later update shows the notice again.

Open clients learn about deploys because the registration is re-checked hourly
and whenever the document becomes visible again (the "phone unlocks, app
resumes" case). The component also shows on a wolke:sw-need-refresh CustomEvent
— the documented seam the e2e suite triggers, since Playwright cannot build a
second worker version against one embedded binary.

Covered by Vitest (states, both locales, axe, the polling helper) and by
e2e/update-notice.spec.ts at all six matrix viewports.
… an effect

react-hooks/set-state-in-effect (CI lint) flagged the effect that cleared the
dismissal when needRefresh rose. Counting the updates this page load has heard
about, and recording the count a dismissal was made at, gets the same behaviour
— re-show on the next update, persist nothing — with no render-then-correct
pass.

@tibroc tibroc left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Supervisor review: approve. Two recorded nits, non-blocking: (1) onRegisteredSW discards startUpdateChecks' teardown, so a DashboardShell remount would leak the hourly interval — latent since the shell mounts once per page load; (2) after dismissing a real update, a later deploy in the same tab doesn't re-show the notice (needRefresh is already true; only the seam increments the count) — benign, but the dismissal comment claims otherwise. Fine to fix opportunistically later.

@tibroc
tibroc merged commit ce2b3fa into main Aug 28, 2026
5 checks passed
@tibroc
tibroc deleted the feat/pwa-update-notice branch August 28, 2026 14:13
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.

1 participant