feat: Add pageVisibilitySignal() to Page for tracking browser tab visibility#23614
Merged
mcollovati merged 22 commits intomainfrom May 6, 2026
Merged
feat: Add pageVisibilitySignal() to Page for tracking browser tab visibility#23614mcollovati merged 22 commits intomainfrom
mcollovati merged 22 commits intomainfrom
Conversation
Artur-
force-pushed
the
PageVisibility
branch
from
bbc3f80 to
4c0cd0d
Compare
|
github-project-automation
Bot
moved this to 🔎Iteration reviews
in Vaadin Flow | Hilla | Kits ongoing work
mshabarov
moved this from 🔎Iteration reviews
to 🟢Ready to Go
in Vaadin Flow | Hilla | Kits ongoing work
mshabarov
moved this from 🟢Ready to Go
to 🪵Product backlog
in Vaadin Flow | Hilla | Kits ongoing work
CLAUDE.md was mixing operational guidance (how to build/test, repo structure, commit hygiene) with design rules (how to shape an API, wrap a browser API, expose signals, model result types). The latter grew out of the geolocation PR review cycle and is now long enough to live on its own. CLAUDE.md keeps build commands, coding conventions and component notes. DESIGN_GUIDELINES.md collects the API-shape and browser-wrapping rules plus patterns pulled from existing platform APIs (per-UI facades, asReadonly signals, sealed hierarchies, bootstrap v-xxx params). Each file links to the other at the top.
…INES.md Cross-checked DESIGN_GUIDELINES.md against the Geolocation implementation and review history. Added the decisions that were implicit before: - Package-private constructor on facade-handed-out handles (GeolocationTracker). - Keep framework-internal mutators on UIInternals, not on user-facing classes — documents why setGeolocationAvailability moved off ExtendedClientDetails. - Options records pattern: canonical constructor with validation + Builder with Duration / int-ms overloads + Serializable. - addEventDetail().allowInert() for DOM listeners that must keep streaming through inert states. - Client-side executeJs errors log at DEBUG, not WARN — they usually mean the feature is unavailable, not a server bug. - Server ↔ client signalling section covering event-to-Signal bridging, client-initiated bridge-back via vaadin-xxx-change on document.body, and stable server-generated UUID keys for async browser handles like watchPosition. - Feature-capability detection without triggering prompts via isSecureContext / featurePolicy / navigator.permissions.query.
…ibility Add a read-only signal-based API on Page that tracks whether the browser tab is visible and focused, visible but not focused, or hidden. Uses the browser Page Visibility API combined with focus/blur events, with a Firefox workaround for deferred visibilitychange. Client-side logic lives in page-visibility.js loaded via @jsmodule on UI.
Follow the Geolocation-style bootstrap path for page visibility so the signal reflects the real state from the first render instead of defaulting to VISIBLE: - Add PageVisibility.UNKNOWN sentinel for the pre-bootstrap window; the signal is seeded with UNKNOWN, then replaced by the value reported via the new v-pv init param. - Move the mutable ValueSignal to UIInternals; Page hands out a cached asReadonly() wrapper so subscriber identity stays stable across calls. - Port page-visibility.js to flow-client/src/main/frontend/PageVisibility.ts, imported from Flow.ts so it loads before collectBrowserDetails runs. The init(element) installer is now idempotent via a per-element WeakMap to stop duplicate listeners from ever attaching. - Log executeJs client-side errors at DEBUG and guard PageVisibility.valueOf for forward-compat with newer clients. - Expand the Javadoc with .get() vs .peek() guidance and the Firefox/focus reliability caveats. Update DESIGN_GUIDELINES.md to codify what this refactor applied: when to place TS in flow-client vs META-INF/frontend, the idempotent init(element) requirement, and the sentinel-on-bootstrap rule. Add PageVisibility to the precedent list alongside Geolocation and windowSizeSignal.
mcollovati
requested changes
Apr 24, 2026
Collaborator
mcollovati
left a comment
mcollovati
left a comment
There was a problem hiding this comment.
I guess we can add a couple of E2E tests by using Selenium APIs to switch between windows/tabs.
Rename `ExtendedClientDetails.fromJson` to `updateFromJson` to reflect that it mutates UI state, move the `setExtendedClientDetails` call into the method itself, and require a non-null UI via `Objects.requireNonNull`. Drop the now-redundant `UIInternals.setPageVisibility` wrapper; callers use `getPageVisibilitySignal().set(...)` directly.
Allows e.g. adding event listeners in the Page constructor
Match the Geolocation API's pattern: dispatch `vaadin-page-visibility-change` on `document.body` and install the listeners at module load. Drops the per-element `init(element)` API and the Java→JS round-trip that called it; the server-side `addEventListener` on `ui.getElement()` still receives the event via DOM bubbling. Per review feedback on PR #23614.
Mirror the windowSizeSignal pattern: own the ValueSignal directly on Page, expose a package-private setPageVisibility(...) for bootstrap to call via ui.getPage(), and drop the field + accessor from UIInternals. The signal is eagerly initialized (cheap) and the read-only wrapper + DOM listener stay lazy.
The read-only wrapper is now a final field, eagerly initialized from the underlying signal. The DOM listener install stays lazy (Page is constructed before UI internals are ready) but no longer keeps the DomListenerRegistration on the side — a boolean guard is enough since nothing ever uses the registration.
setPageVisibility now accepts the raw String the client sends, parses it, and logs at debug level when the value is unknown so a forward- compatible client value is still observable in logs. The bootstrap path and the DOM event listener share the same entry point. The geolocation seed inlining gets reverted now that there is no parallel helper to mirror.
Defer Page creation to the UI constructor body (mirroring the existing Geolocation pattern), so Page can install the visibility-change DOM listener directly in its own constructor. Drops the visibilityListenerInstalled guard and the ensureVisibilityListener helper; pageVisibilitySignal() is now a plain getter.
Mirrors the GeolocationIT shape: a small @Push view binds the signal to a status div, and the IT drives transitions via JavaScript instead of real tab switching (headless Chrome does not background tabs predictably). Covers the initial VISIBLE state from bootstrap, the blur/focus → VISIBLE_NOT_FOCUSED → VISIBLE round trip, and the visibilitychange path with document.hidden spoofed via Object.defineProperty.
The flow-client TS suite stubs window.Vaadin in beforeEach, so any access to window.Vaadin.Flow.pageVisibility from Flow.ts fails with "Cannot read properties of undefined (reading 'current')". The global had only one consumer (Flow.ts itself), so replace it with a direct named import — currentPageVisibility() — and drop the global setup from PageVisibility.ts. The side-effect listener install still runs because the module is imported.
Headless Chrome reports document.hasFocus() === false because the tab has no OS focus, so bootstrap seeds the signal with VISIBLE_NOT_FOCUSED rather than VISIBLE. Accept both for the initial assertion, and dispatch a synthetic focus event from the other tests to drive the signal to a known VISIBLE baseline before testing transitions.
The IT drives state changes by dispatching synthetic DOM events from the test, which are themselves client→server requests; their responses naturally carry any resulting state.setText() updates. Push is not needed and adding @Push to a single route in a webapp that doesn't otherwise enable push appears to disturb how Atmosphere is set up for test-root-context, which is the most plausible cause of the cascading IT failures across that module.
mcollovati
requested changes
May 4, 2026
- Page.pageVisibilitySignal Javadoc no longer claims the listener is installed lazily on first access; it is installed when Page is constructed alongside the UI. - PageVisibilityView uses an AtomicInteger counter for readability.
mcollovati
reviewed
May 6, 2026
Comment on lines
+32
to
+33
| * No value has been reported by the browser yet. Only observed between | ||
| * server attach and the completion of the first client handshake; after |
Collaborator
There was a problem hiding this comment.
Only observed between server attachment and the completion of the first client handshake
Is this still true? Or does server attach mean that the value is UNKNOWN, for example, in a UIInitListener? If so, I would avoid the attach word that can be confusing and make the user think that the value is not available in Component attach listener.
Drop the misleading "between server attach and the first client handshake" wording on both PageVisibility.UNKNOWN and Page#pageVisibilitySignal. The bootstrap seeds the signal before any user code (UI init, UIInitListener, component attach) runs, so UNKNOWN is essentially never observed in practice; the previous wording could suggest UNKNOWN might still be visible inside Component#onAttach.
|
mcollovati
approved these changes
May 6, 2026
github-project-automation
Bot
moved this from 🪵Product backlog
to Done
in Vaadin Flow | Hilla | Kits ongoing work
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a read-only signal-based API on Page that tracks whether the browser tab is visible and focused, visible but not focused, or hidden. Uses the browser Page Visibility API combined with focus/blur events, with a Firefox workaround for deferred visibilitychange. Client-side logic lives in page-visibility.js loaded via @jsmodule on UI.