Skip to content

ref(forms): Convert BooleanField from class to functional component#106493

Merged
ryan953 merged 3 commits intomasterfrom
refactor/convert-boolean-field-to-functional
Jan 20, 2026
Merged

ref(forms): Convert BooleanField from class to functional component#106493
ryan953 merged 3 commits intomasterfrom
refactor/convert-boolean-field-to-functional

Conversation

@ryan953
Copy link
Member

@ryan953 ryan953 commented Jan 17, 2026

Summary

Refactors BooleanField from a class component to a functional component to align with modern React patterns and Sentry's frontend guidelines (no new class components).

This component was identified as a stateless class component (no internal state management), making it an ideal candidate for conversion to a functional component.

Changes

  • Converted class component to functional component
  • Removed Component import and class syntax
  • Made event parameter optional in handleChange to match OnEvent type signature
  • Simplified event type handling (React.ChangeEvent | React.FormEvent)
  • Removed redundant coerceValue helper method (inlined logic)
  • Maintained all existing functionality and behavior

Test plan

  • ✅ Linting passes
  • ✅ Accessibility tests pass (static/app/components/forms/fields/accessibility.spec.tsx)
  • ✅ No functional changes - component behavior is identical

Related to https://github.com/getsentry/frontend-tsc/issues/42

Refactors BooleanField from a class component to a functional component to align with modern React patterns and Sentry's frontend guidelines (no new class components).

Changes:
- Converted class component to functional component
- Removed Component import and class syntax
- Made event parameter optional in handleChange to match OnEvent type
- Simplified event type handling
- Removed redundant coerceValue helper (inlined logic)
- Maintained all existing functionality and behavior
@ryan953 ryan953 requested a review from a team as a code owner January 17, 2026 04:09
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 17, 2026
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@ryan953 ryan953 marked this pull request as draft January 17, 2026 04:48
@ryan953 ryan953 marked this pull request as ready for review January 20, 2026 17:29
Copy link
Member

@shashjar shashjar left a comment

Choose a reason for hiding this comment

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

looks good, left a couple minor notes abt testing + types

expect(checkbox).not.toBeChecked();

await userEvent.click(checkbox);
expect(onChange).toHaveBeenCalledWith(true, expect.anything());
Copy link
Member

Choose a reason for hiding this comment

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

should we expect the checkbox to be checked here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it always functions as a controlled input, so we'd have to wire up a Model to listen to the onChange, which would drive the new rendered value.
This test, as is, is just mocking it at that point. Claude did it.

<BooleanField name="fieldName" value onChange={onChange} confirm={confirm} />
);

// Clicking to disable (false state) - no confirm message for false
Copy link
Member

Choose a reason for hiding this comment

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

should there be a test where the confirm message actually gets displayed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's more of a test for FormField imo, and again controlled field.

@ryan953 ryan953 merged commit feedeeb into master Jan 20, 2026
65 checks passed
@ryan953 ryan953 deleted the refactor/convert-boolean-field-to-functional branch January 20, 2026 19:45
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants