Skip to content

feat: fail loudly when a trigger is created without an action#24716

Merged
Artur- merged 4 commits into
mainfrom
fail-on-half-baked-trigger
Jun 22, 2026
Merged

feat: fail loudly when a trigger is created without an action#24716
Artur- merged 4 commits into
mainfrom
fail-on-half-baked-trigger

Conversation

@Artur-

@Artur- Artur- commented Jun 22, 2026

Copy link
Copy Markdown
Member

A Trigger that is never armed with an action does nothing on the client, which is almost always a forgotten terminal binding call (e.g. Clipboard.onClick(button) with no following writeText(...)). Detect this in the Trigger base class — so it covers every trigger/action user, not just clipboard — by deferring a check to beforeClientResponse and throwing an IllegalStateException if no action was committed.

Track arming as an intent flag set the moment a binding commits to an action, and add Trigger.triggersWhenAttached(target, supplier) so a binding that legitimately defers wiring until a target component attaches (Fullscreen.enter(detachedPanel)) is not mistaken for a forgotten one.

A Trigger that is never armed with an action does nothing on the client,
which is almost always a forgotten terminal binding call (e.g.
Clipboard.onClick(button) with no following writeText(...)). Detect this
in the Trigger base class — so it covers every trigger/action user, not
just clipboard — by deferring a check to beforeClientResponse and
throwing an IllegalStateException if no action was committed.

Track arming as an intent flag set the moment a binding commits to an
action, and add Trigger.triggersWhenAttached(target, supplier) so a
binding that legitimately defers wiring until a target component attaches
(Fullscreen.enter(detachedPanel)) is not mistaken for a forgotten one.
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown

Test Results

 1 452 files  ±0   1 452 suites  ±0   1h 26m 8s ⏱️ + 1m 43s
10 264 tests +3  10 196 ✅ +3  68 💤 ±0  0 ❌ ±0 
10 736 runs  +3  10 667 ✅ +3  69 💤 ±0  0 ❌ ±0 

Results for commit 267941f. ± Comparison against base commit b3ec2b5.

♻️ This comment has been updated with latest results.

Artur- added 3 commits June 22, 2026 12:18
Replace Trigger.triggersWhenAttached(target, supplier) with an overload
of triggers(Component, SerializableSupplier<Action>), so deferred arming
reads as the same operation rather than a separate concept.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@mcollovati mcollovati requested a review from tltv June 22, 2026 11:34
@Artur- Artur- added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit 7f5f50f Jun 22, 2026
32 of 33 checks passed
@Artur- Artur- deleted the fail-on-half-baked-trigger branch June 22, 2026 13:50
vaadin-bot added a commit that referenced this pull request Jun 22, 2026
… (CP: 25.2) (#24734)

This PR cherry-picks changes from the original PR #24716 to branch 25.2.
---
#### Original PR description
> A Trigger that is never armed with an action does nothing on the
client, which is almost always a forgotten terminal binding call (e.g.
Clipboard.onClick(button) with no following writeText(...)). Detect this
in the Trigger base class — so it covers every trigger/action user, not
just clipboard — by deferring a check to beforeClientResponse and
throwing an IllegalStateException if no action was committed.
> 
> Track arming as an intent flag set the moment a binding commits to an
action, and add Trigger.triggersWhenAttached(target, supplier) so a
binding that legitimately defers wiring until a target component
attaches (Fullscreen.enter(detachedPanel)) is not mistaken for a
forgotten one.
>

Co-authored-by: Artur Signell <artur@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants