feat: trigger/action framework#24361
Merged
Merged
Conversation
Artur-
force-pushed
the
feature/trigger-step1
branch
from
2f54e28 to
462630b
Compare
mshabarov
reviewed
May 19, 2026
Contributor
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
First round of review, looked at the top most interfaces/classes.
mshabarov
reviewed
May 19, 2026
Artur-
force-pushed
the
feature/trigger-step1
branch
2 times, most recently
from
dbd02aa to
abb0dae
Compare
github-project-automation
Bot
moved this to 🔎Iteration reviews
in Vaadin Flow | Hilla | Kits ongoing work
mshabarov
previously approved these changes
May 20, 2026
Contributor
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
Looks good to me from the API shape perspective. Haven't yet checked carefully the implementation and test coverage, but giving a stamp now to not block other PRs, will double check later.
Adds a server-side API for wiring client-side triggers (DOM events) to client-side actions, reading values from arguments, without a server round-trip when the action doesn't ask for one. Public package `com.vaadin.flow.component.trigger`: - Interfaces: `Trigger`, `Action`, `Argument<T>`. - Abstract bases: `AbstractTrigger`, `AbstractAction`, `AbstractArgument`. Each carries a namespaced type id (`flow:event`, `myapp:double-tap`). Subclasses override `buildClientConfig(ConfigContext)` to ship JSON config and `applyServerSideEffect(ArrayNode)` for any server-side reaction to client-reported outcomes (delivered through a per-host return channel). - Built-ins: `DomEventTrigger` (generic DOM event by name), `PropertyArgument<T>` (read a JS property at fire time), `SetPropertyAction<T>` (assign a JS property when the trigger fires). Internal package `com.vaadin.flow.component.trigger.internal`: - `TriggerSupport extends ServerSideFeature`: per-host store of triggers/actions/arguments + bindings. Emits a single JSON snapshot per host through `Element.executeJs`, coalescing mutations via `StateTree.beforeClientResponse`. Lazily registers a `ReturnChannelRegistration` so actions that report back to the server (clipboard outcome, etc.) get there without a separate protocol. - `ConfigContext`: lets subclasses encode argument/element references by stable id during `buildClientConfig`. Client (`flow-client/src/main/frontend/Triggers.ts`): - Self-registers `window.Vaadin.Flow.triggers` with public `registerTrigger / registerAction / registerArgument / bind / unbind` so add-on `@JsModule`s can plug in their own factories. - `bind` is idempotent — second bind disposes the previous installation. - Built-in factories for `flow:event`, `flow:property`, `flow:set-property`. Framework wiring: `NodeFeatures.TRIGGER_SUPPORT = 28`, slot in `NodeFeatureRegistry` and `BasicElementStateProvider`, `NodeFeatureTest` expectations updated.
Seal Action/Argument interfaces, consolidate buildSnapshot loops, drop unused test/helper methods, and remove obvious narration comments. No behavior change; all trigger tests pass.
The trigger framework is not yet stable; keep the whole surface internal so revisions don't break consumers.
Triggers can now expose their own output as Arguments that downstream
Actions consume on the same footing as element-derived Arguments. The
JS expression for a trigger-output Argument refers to a variable in the
trigger's handler scope (e.g. event["screenX"]); a scope guard refuses
to render an Argument from trigger A into trigger B's handler.
- HandlerExprArg: package-private Argument whose JS is a handler-scoped
expression, carrying its owning trigger.
- JsBuilder: now carries the current trigger so handler-scoped Arguments
can assert they're rendered into their own handler.
- DomEventTrigger.property(name): generic accessor returning
Argument<T> for event[name].
- ClickTrigger: typed sugar over DomEventTrigger("click") with
screenX/screenY/clientX/clientY/shiftKey/ctrlKey/altKey/metaKey.
- SetPropertyAction: new constructor accepting Argument<? extends T>;
literal-value path delegates through LiteralArg so rendering is
uniform.
…s only Trigger, Action, and Argument added nothing once they were sealed against their corresponding abstract base classes. Folding them into the bases removes one layer of indirection, drops the runtime/sealed plumbing, and lets AbstractTrigger.triggers(AbstractAction...) replace the previous runtime cast. - Delete Trigger.java, Action.java, Argument.java. - AbstractTrigger now declares triggers(AbstractAction...) / remove() directly and implements Serializable. - AbstractAction / AbstractArgument drop the `non-sealed` modifier and the implements clause; they only need to be Serializable. - DomEventTrigger.property and ClickTrigger accessors return AbstractArgument<T>. - SetPropertyAction's Argument constructor now takes AbstractArgument<? extends T>, removing the cast. - TriggerTest updated to inspect the new JsFunction-based init wrapper (the user expression is now passed as a JsFunction parameter to a fixed framework wrapper).
The action handler is now built as a JsFunction with the element
references as captures and `event` as a runtime argument. The install
JS is reduced to framework boilerplate that references the handler at
$0 — no user-supplied content leaks into the install string.
- AbstractTrigger.triggers constructs JsFunction.of(handlerBody,
captures).withArguments("event") and passes it as the single
addJsInitializer parameter.
- AbstractTrigger.installJs() now takes no arguments. Subclasses just
produce the install JS that references the handler at $0.
- DomEventTrigger.installJs returns the literal addEventListener +
removeEventListener pair with $0 as the handler reference.
- JsBuilder.params() renamed to captures() to match what the value is
now used for.
- Tests inspect the install JsFunction's body + the captured handler
JsFunction; the install body for DomEventTrigger is now stable enough
to assert verbatim.
With the interfaces gone, the abstract base classes own the names. Now DomEventTrigger extends Trigger, SetPropertyAction extends Action, and PropertyArgument extends Argument.
Addresses PR review: - Argument is overloaded with "method argument" — renamed Argument<T> to Input<T>, PropertyArgument to PropertyInput, HandlerExprArg to HandlerInput, LiteralArg to LiteralInput. Reads as "an input that an Action consumes from a Trigger." - Trigger javadoc now spells out that the synchronous-dispatch claim doesn't extend to the asynchronous browser APIs that motivate the framework — server-observable effects of an action (e.g. a callback for navigator.clipboard.writeText) may reach the server arbitrarily later than the gesture itself.
Making Input a nested type of Action encodes the consumer direction in the name: a Trigger that hands out an Action.Input isn't claiming to "produce an input", it's offering a value typed for what an Action consumes. The Map.Entry-style nesting also keeps the abstract base discoverable from Action without bloating the package surface. - Action.java now hosts a nested `public abstract static class Input<T>`. - Top-level Input.java deleted. - Concrete subclasses extend Action.Input<T> (PropertyInput, LiteralInput, HandlerInput). - DomEventTrigger.property, ClickTrigger accessors, SetPropertyAction constructor, and the test all reference Action.Input<T> explicitly. - Updated Action's javadoc to explain the input/trigger relationship.
Reviewer wants varargs OR chaining, not both. Pick varargs — each call wires one or more actions in one shot, and no return is needed. Multiple wirings come from multiple triggers() calls on the same trigger.
Artur-
force-pushed
the
feature/trigger-step1
branch
from
9fb93d7 to
876af78
Compare
main reordered the addJsInitializer wrapper from [element, userFn, initId] to [element, initId, userFn]. The test helper that inspects the trigger's install JsFunction now reads it from index 2 instead of index 1.
|
mshabarov
approved these changes
May 21, 2026
github-project-automation
Bot
moved this from 🔎Iteration reviews
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
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 server-side API for wiring client-side triggers (DOM events) to
client-side actions, reading values from arguments, without a server
round-trip when the action doesn't ask for one.
The whole surface is kept internal at com.vaadin.flow.component.trigger.internal while the framework matures;
nothing here is part of the public API yet.
Java side (com.vaadin.flow.component.trigger.internal):
extend the corresponding abstract base.
AbstractCallbackAction. Each carries a namespaced type id
(flow:event, myapp:double-tap). Subclasses override
buildClientConfig(ConfigContext) to ship JSON config; callback
actions additionally declare a payload type and an
applyServerSideEffect(T) hook the framework calls after Jackson
deserialises the JSON the client reports back over the per-host
return channel.
PropertyArgument (read a JS property at fire time),
SetPropertyAction (assign a JS property when the trigger fires).
triggers/actions/arguments + bindings. Emits a single JSON snapshot
per host through Element.executeJs, coalescing mutations via
StateTree.beforeClientResponse. Lazily registers a
ReturnChannelRegistration for callback actions.
stable id during buildClientConfig.
Client (flow-client/src/main/frontend/Triggers.ts):
registerTrigger / registerAction / registerArgument / bind / unbind
so add-on @jsmodules can plug in their own factories.
Framework wiring: NodeFeatures.TRIGGER_SUPPORT = 28, slot in
NodeFeatureRegistry and BasicElementStateProvider, NodeFeatureTest
expectations updated.