Use Stimulus' own dispatch in form refreshes - #25363
Conversation
|
Warning This pull request does not link an OpenProject work package. Please add a link to the work package in the description, or reference it in the |
There was a problem hiding this comment.
🟡 Changes recommended
Add regression coverage for event targeting on nested form targets.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates the form-refresh controller to use Stimulus’s native dispatch API, removing the deprecated behavior while preserving debouncing.
Changes:
- Replaces
ApplicationControllerwith StimulusController. - Retains
useDebounce. - Corrects event targeting for nested forms.
Review note: Add a regression test covering a wrapper controller with a nested form target.
File summaries
| File | Summary |
|---|---|
frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts |
Uses native Stimulus dispatch and preserves debouncing. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
stimulus-use's `ApplicationController` swaps in its deprecated
`useDispatch()`, which warned on every refresh and read `{ target }` as
event detail instead of the element to dispatch on.
4770dfe to
ad62cf3
Compare
Ticket
No work package. Frontend test-output hygiene, following up on #24622.
What are you trying to accomplish?
Removes the
refresh-on-form-changes `useDispatch()` is deprecatedwarning. It fired on every form refresh, so ten times in the controller's specs and in the browser console in development.It also fixes a targeting bug.
RefreshOnFormChangesControllerextended stimulus-use'sApplicationController, which replaces Stimulus'dispatchwith the deprecateduseDispatch()version. That version takes(eventName, detail), sothis.dispatch('beforeSnapshot', { target: this.formTarget })puttargetinto the event detail and dispatched on the controller element instead of the form. The project creation wizard's submission form mounts the controller on a wrapper around its form, so the CKEditor listener on the form never saw the event.What approach did you choose and why?
The controller now extends Stimulus'
Controllerdirectly and keepsuseDebounce. It used nothing else fromApplicationController(isPreview,csrfToken,metaValue). Stimulus' built-indispatch(name, { target })already has the signature the call site was written for, and it produces the samerefresh-on-form-changes:beforeSnapshotevent name, bubbling by default, so the CKEditor listener is unaffected.AI involvement
Collaborative – AI generated a substantial part of the code; I reviewed and understand every line.
Merge checklist