feat: PromiseAction + CopyTextToClipboardAction + RequestFullscreenAction#24394
Merged
Conversation
…tion
Adds a `PromiseAction` base class in the trigger framework and two
concrete subclasses — `CopyTextToClipboardAction` and
`RequestFullscreenAction` — on top of the executeJs-based trigger
framework introduced in step 1.
`PromiseAction` is the generic primitive: many gesture-bound browser
APIs are asynchronous (clipboard, fullscreen, file picker, share,
payments, …) and follow the same shape — call the API, then handle the
resolved value or the rejection. Subclasses override
`appendPromiseExpression(JsBuilder, StringBuilder)` to emit the
promise-yielding JS; the base wires the outcome reporting.
Two construction modes, mirroring the Geolocation API:
- No-arg `super()` — fire-and-forget; the rendered JS is just the
promise expression and the server never sees the outcome.
- `super(onSuccess, onError)` — `onSuccess` runs after the promise
resolves; `onError` receives the browser's error message after the
promise rejects. Both run on the UI thread; both are required (pass
`() -> {}` or `err -> {}` to opt out of one, matching the
Geolocation pattern).
The with-outcome mode lazily registers one
`ReturnChannelRegistration` per trigger host node and appends
`.then(()=>$N(true,null)).catch(e=>$N(false, msg))` to the promise
expression, so the client invokes the channel after the promise
resolves or rejects. `JsBuilder.capture(Object)` is the new
package-private hook that lets actions allocate a `$N` placeholder for
non-Element captures (the return channel here, which materialises
client-side as a callable JS function).
Concrete subclasses:
- `CopyTextToClipboardAction` — supplies
`navigator.clipboard.writeText(textExpr)` as the promise expression.
Takes an `Action.Input<String>` for the text. Named verb-first so
parallels like `CopyImageToClipboardAction` and
`PasteFromClipboardAction` read naturally.
- `RequestFullscreenAction` — supplies `<target>.requestFullscreen()`
as the promise expression. Takes a target `Component` whose root
element will be fullscreened.
Both ITs (`TriggerCopyTextToClipboardView`/`IT`,
`TriggerRequestFullscreenView`/`IT`) shim the underlying browser API
to resolving/rejecting promises and assert the server-side status div
updates accordingly — avoiding dependence on browser clipboard /
fullscreen permissions in CI.
Artur-
force-pushed
the
feature/trigger-step2
branch
2 times, most recently
from
1dc9334 to
db61e7a
Compare
`PromiseAction.appendStatement` used to assemble the `.then`/`.catch`
glue by chained `StringBuilder.append` calls, with the channel
receiving positional `(boolean, message)` args decoded by index in
`dispatch`. Two smells in one place: structural JS hidden in string
concatenation, and an implicit positional wire contract.
Replace both:
- A static `JsFunction OBSERVE_PROMISE` carries the wrapper body once,
with named runtime arguments `(promise, channel)`. Subclasses are
unchanged; `appendStatement` now allocates two captures (observer +
channel), then composes one call: `$observe($promiseExpr, $channel)`.
No more `.append(".then(()=>")…` chains; the handler body is one
line, the JS literal lives in one place.
- Introduce a private `record Outcome(boolean ok, @nullable String
error)` as the wire shape. The OBSERVE_PROMISE body pushes
`{ok: true}` / `{ok: false, error: …}`; `dispatch` reads it via
`JacksonUtils.readValue` rather than positional `args.get(0)` /
`args.get(1)` decoding. Renaming or adding a field can no longer
silently desynchronize the two sides.
Test assertions follow: the handler body is now `$0(<promise>, $1);`
across `PromiseActionTest`, `CopyTextToClipboardActionTest`, and
`RequestFullscreenActionTest`; the channel-invocation tests pass an
`Outcome`-shaped `ObjectNode` instead of positional booleans/strings.
`OBSERVE_PROMISE` is a `static final` shared instance — `JsFunction`
is `Serializable` and immutable, and each `addJsInitializer`
registration still gets its own capture entry, so there's no
cross-UI state and no leak. 19/19 trigger-internal tests pass.
`LiteralInput` was package-private and accepted `@Nullable T`. Both
were carry-overs from its only original user, `SetPropertyAction`'s
null-clearing convenience constructor, but neither survives as
LiteralInput becomes a building block for other actions:
- Public so callers can wire a fixed value through any action that
takes an `Action.Input<? extends T>` — e.g. copying a constant
string with `new CopyTextToClipboardAction(new LiteralInput<>("…"))`.
- Non-null because `null` as a literal payload almost never matches
a sensible browser API. `writeText(null)` writes the string
`"null"` to the clipboard; `target.requestFullscreen.call(null)`
is meaningless. Forbidding it at the `LiteralInput` constructor
catches the foot-gun at compile time (via NullAway) wherever the
type permits, and at runtime via `Objects.requireNonNull`
otherwise.
`SetPropertyAction`'s null-accepting convenience constructor stays —
clearing a property by assigning `null` is a real use case. It now
routes the null branch through a private `NULL_LITERAL` singleton
that emits the JS literal `null`, instead of leaking nullability
into `LiteralInput`.
Adds a `fireAndForget_literalInput_encodesValueAsJsonLiteral` test
in `CopyTextToClipboardActionTest` that copies `hello "world"\n` and
asserts the quotes and newline are JSON-escaped — proves callers
can't break out of the JS string by passing values with quotes or
newlines.
The view now has two buttons — `#copy` ("Copy input value") wired to a
PropertyInput on the input field, and `#copy-static` ("Copy static
text") wired to a LiteralInput carrying `hello "world"\n`. The literal
deliberately includes a quote and a newline so the IT verifies the
value the browser ends up with is exactly what Java passed in, i.e.
the JSON encoding done at build time round-trips without losing or
mangling characters.
Button labels updated to describe what each button copies.
mshabarov
reviewed
May 21, 2026
Contributor
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
API and implementation looks good to me overall - clear purposes for added classes, nice classes hierarchy. I left several comments with some of them not being 100% changes requests, so ready to merge even as is with a meaningful elaborations of why it's implemented this way.
Addresses the inline review comments on PR #24394: PromiseAction - Drop the Geolocation cross-link from the javadoc — it was a stale inline reference, not load-bearing API documentation. - Replace `onSuccess: Runnable` / `onError: Consumer<String>` with typed records: `onSuccess: Consumer<Success>` carrying the resolved value (when the subclass' promise produced one) and `onError: Consumer<Error>` carrying the rejection's name and message. The error name (typically a `DOMException` class like "NotAllowedError") is what callers usually want to switch on. - Update the wire shape accordingly: `{ok, value, error: {name, message}}`. The dispatch now decodes one `Outcome` record via Jackson and constructs `Success`/`Error` from it. - Tighten the fire-and-forget detection to check both callbacks (defensive against future constructor changes that might break the both-null-or-both-non-null invariant). - Add a javadoc note explaining `final` on `appendStatement` — subclasses customise via `appendPromiseExpression`, not by overriding the wiring; that's what keeps the `Outcome` wire contract stable. CopyTextToClipboardAction - Constructor takes `Consumer<String> onCopied` (typed) and adapts to PromiseAction's `Consumer<Success>` internally. The promise expression is wrapped in an IIFE `((v) => writeText(v).then(() => v))(<textExpr>)` so the resolved value is the copied string — the server sees what actually reached the clipboard even when the input was a client-side `PropertyInput`. RequestFullscreenAction - Clarify the "low-level" javadoc note: explicitly point at PR #24326 (`Component.requestFullscreen()`) as the higher-level facade that wraps the element for Vaadin theming/overlays; this action is the trigger-framework primitive it builds on. - Constructor stays `Runnable onSuccess` (no meaningful value to deliver) but `Consumer<Error>` for the error path. Tests - PromiseActionTest splits the channel-invocation cases into "value present", "value missing", and "error with name+message". - CopyTextToClipboardActionTest gains an `onCopied_receivesTheString` test and updates the rendered-JS assertions to the IIFE wrapper. - ITs use the new typed callbacks; the rejecting shims now throw `new DOMException('DeniedByTest', 'NotAllowedError')` so the IT can assert both `err.name()` and `err.message()` reach the server.
mshabarov
reviewed
May 22, 2026
mshabarov pointed out that AbortError isn't a spec-documented rejection for the clipboard / fullscreen APIs the current PromiseAction subclasses wrap — only NotAllowedError is. The speculative `AbortError, …` example overstated what callers should expect. Remove it from both `PromiseAction.Error`'s javadoc and the `RequestFullscreenAction` class-level note; leave the documented NotAllowedError as the concrete example.
`STATIC_TEXT` ends with `\n`. Selenium's `WebElement.getText()`
normalises and trims surrounding whitespace, so the status
assertion never matched in CI even though the
`navigator.clipboard.writeText` round-trip check did. Switch to
`document.getElementById('status').textContent` via
`executeScript`, which preserves the trailing newline.
Other IT cases are unaffected — they compare strings without
boundary whitespace.
mshabarov
approved these changes
May 22, 2026
|
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
2 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.
Adds a
PromiseActionbase class in the trigger framework and two concrete subclasses —CopyTextToClipboardActionandRequestFullscreenAction— on top of the executeJs-based trigger framework introduced in step 1.PromiseActionis the generic primitive: many gesture-bound browser APIs are asynchronous (clipboard, fullscreen, file picker, share, payments, …) and follow the same shape — call the API, then handle the resolved value or the rejection. Subclasses overrideappendPromiseExpression(JsBuilder, StringBuilder)to emit the promise-yielding JS; the base wires the outcome reporting.Two construction modes, mirroring the Geolocation API:
super()— fire-and-forget; the rendered JS is just the promise expression and the server never sees the outcome.super(onSuccess, onError)—onSuccessruns after the promise resolves;onErrorreceives the browser's error message after the promise rejects. Both run on the UI thread; both are required (pass() -> {}orerr -> {}to opt out of one, matching the Geolocation pattern).The with-outcome mode lazily registers one
ReturnChannelRegistrationper trigger host node and appends.then(()=>$N(true,null)).catch(e=>$N(false, msg))to the promise expression, so the client invokes the channel after the promise resolves or rejects.JsBuilder.capture(Object)is the new package-private hook that lets actions allocate a$Nplaceholder for non-Element captures (the return channel here, which materialises client-side as a callable JS function).Concrete subclasses:
CopyTextToClipboardAction— suppliesnavigator.clipboard.writeText(textExpr)as the promise expression. Takes anAction.Input<String>for the text. Named verb-first so parallels likeCopyImageToClipboardActionandPasteFromClipboardActionread naturally.RequestFullscreenAction— supplies<target>.requestFullscreen()as the promise expression. Takes a targetComponentwhose root element will be fullscreened.Both ITs (
TriggerCopyTextToClipboardView/IT,TriggerRequestFullscreenView/IT) shim the underlying browser API to resolving/rejecting promises and assert the server-side status div updates accordingly — avoiding dependence on browser clipboard / fullscreen permissions in CI.