Skip to content

refactor(useFormTrigger): move void controller to constructor - #15108

Merged
jcfranco merged 1 commit into
devfrom
jcfranco/move-useFormTrigger-to-constructor
Sep 1, 2026
Merged

jcfranco merged 1 commit into
devfrom
jcfranco/move-useFormTrigger-to-constructor

Conversation

@jcfranco

@jcfranco jcfranco commented Aug 31, 2026 •

Copy link
Copy Markdown
Member

monday.com sync: #12948982957

Related Issue: N/A

Summary

This PR updates useFormTrigger, which was missed in #15075, to be invoked in the constructor instead, avoiding unnecessary boilerplate and potential confusion.

@jcfranco
jcfranco requested a lite review from Copilot August 31, 2026 22:31
@github-actions github-actions Bot added the refactor Issues tied to code that needs to be significantly reworked. label Aug 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns useFormTrigger with the pattern established in #15075 for void-returning controllers by invoking it in component constructors instead of assigning its (void) result to a class field, reducing misleading boilerplate and improving clarity across components and tests.

Changes:

  • Moved useFormTrigger()(this) invocation from a class field initializer into the constructor in affected components.
  • Updated useFormTrigger usage with options (conditional disabling) to be invoked in the constructor as well.
  • Adjusted the browser spec test components to match the same constructor-invocation pattern.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/components/src/controllers/useFormTrigger.browser.spec.tsx Updates test components to invoke useFormTrigger in constructors instead of assigning to a field.
packages/components/src/components/button/button.tsx Moves useFormTrigger invocation into Button’s constructor (alongside existing controller setup).
packages/components/src/components/action/action.tsx Adds a constructor to invoke useFormTrigger rather than storing a misleading formTrigger field.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jcfranco
jcfranco force-pushed the jcfranco/move-useFormTrigger-to-constructor branch from 9086997 to 55f5960 Compare September 1, 2026 00:27
@jcfranco
jcfranco changed the base branch from jcfranco/14766-speed-up-global-attribute-watching to dev September 1, 2026 00:27
@jcfranco
jcfranco requested a review from a team September 1, 2026 00:28

@driskull driskull left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jcfranco jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label Sep 1, 2026
@jcfranco
jcfranco merged commit 7a3af38 into dev Sep 1, 2026
20 checks passed
@jcfranco
jcfranco deleted the jcfranco/move-useFormTrigger-to-constructor branch September 1, 2026 16:55
benelan added a commit that referenced this pull request Sep 1, 2026
* origin/dev: (47 commits)
  refactor(useFormTrigger): move void controller to constructor (#15108)
  refactor(slider): use refs instead of DOM queries (#15096)
  test(slider): migrate tests to browser mode (#15095)
  refactor: enable `noImplicitAny` (#15027)
  chore: release next
  feat(block): add background color token (#15099)
  fix(tile): restore top/bottom slot spacing (#15101)
  build(deps): update dependency eslint to v10.9.1 (#15036)
  build(deps): update dependency eslint-plugin-storybook to v10.5.10 (#14807)
  build(deps): update dependency @eslint-react/eslint-plugin to v5.18.6 (#14722)
  build(deps): update dependency eslint-plugin-perfectionist to v5.10.1 (#14801)
  build(deps): update dependency @eslint-react/kit to v5.18.6 (#14723)
  refactor: apply `transition-default` mixin (#8851)
  chore: release next
  feat(block): deprecate `collapsible` in favor of `expandable` (#15091)
  chore: release next
  style(radio-button-group): apply linter fixes (#15085)
  fix(radio-button-group): sync group `disabled` only when set (#15084)
  refactor: derive a shared commonTests helper for scale propagation (#15073)
  chore: release next
  ...
@eriklharper

Copy link
Copy Markdown
Contributor

Excellent call!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Issues tied to code that needs to be significantly reworked. skip visual snapshots Pull requests that do not need visual regression testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants