refactor(toggle): migrate Toggle from Flow to TypeScript - #4764
Open
bonchevskyi wants to merge 1 commit into
Open
refactor(toggle): migrate Toggle from Flow to TypeScript#4764bonchevskyi wants to merge 1 commit into
bonchevskyi wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR adds TypeScript and Flow-compatible ChangesToggle migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/toggle/__tests__/Toggle.test.tsx (1)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse typed props in both migrated test helpers.
Both helpers use
any, so TypeScript does not validate the public component contracts.
src/components/toggle/__tests__/Toggle.test.tsx#L8-L9: usePartial<ToggleProps>.src/components/toggle/__tests__/ToggleField.test.tsx#L7-L8: use a fixture type based onToggleFieldPropsthat requiresfield.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/toggle/__tests__/Toggle.test.tsx` around lines 8 - 9, Replace the any-typed getWrapper props in src/components/toggle/__tests__/Toggle.test.tsx lines 8-9 with Partial<ToggleProps>. In src/components/toggle/__tests__/ToggleField.test.tsx lines 7-8, type the helper fixture from ToggleFieldProps while requiring the field property, so both migrated test helpers validate their component contracts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/toggle/Toggle.js.flow`:
- Around line 29-31: Update the onMouseEnter and onMouseLeave callback
declarations in the Toggle prop types to use SyntheticMouseEvent<HTMLDivElement>
instead of SyntheticInputEvent<HTMLDivElement>, matching the container div
handlers while preserving their existing callback signatures.
In `@src/components/toggle/ToggleField.tsx`:
- Around line 9-16: Remove the Formik meta bag before forwarding props from
ToggleField. In src/components/toggle/ToggleField.tsx lines 9-16, destructure
and discard meta alongside form; apply the same change in
src/components/toggle/ToggleField.js.flow lines 11-13 so both wrappers prevent
meta from reaching TogglePrimitive and the native input.
---
Nitpick comments:
In `@src/components/toggle/__tests__/Toggle.test.tsx`:
- Around line 8-9: Replace the any-typed getWrapper props in
src/components/toggle/__tests__/Toggle.test.tsx lines 8-9 with
Partial<ToggleProps>. In src/components/toggle/__tests__/ToggleField.test.tsx
lines 7-8, type the helper fixture from ToggleFieldProps while requiring the
field property, so both migrated test helpers validate their component
contracts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2061a3ba-9108-481a-a541-7ede3c409e45
⛔ Files ignored due to path filters (2)
src/components/toggle/__tests__/__snapshots__/Toggle.test.tsx.snapis excluded by!**/*.snapsrc/components/toggle/__tests__/__snapshots__/ToggleField.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/components/toggle/Toggle.js.flowsrc/components/toggle/Toggle.stories.tsxsrc/components/toggle/Toggle.tsxsrc/components/toggle/ToggleField.js.flowsrc/components/toggle/ToggleField.tsxsrc/components/toggle/__tests__/Toggle.test.tsxsrc/components/toggle/__tests__/ToggleField.test.tsxsrc/components/toggle/index.js.flowsrc/components/toggle/index.ts
💤 Files with no reviewable changes (1)
- src/components/toggle/Toggle.stories.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/toggle/__tests__/Toggle.test.tsx (1)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse typed props in both migrated test helpers.
Both helpers use
any, so TypeScript does not validate the public component contracts.
src/components/toggle/__tests__/Toggle.test.tsx#L8-L9: usePartial<ToggleProps>.src/components/toggle/__tests__/ToggleField.test.tsx#L7-L8: use a fixture type based onToggleFieldPropsthat requiresfield.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/toggle/__tests__/Toggle.test.tsx` around lines 8 - 9, Replace the any-typed getWrapper props in src/components/toggle/__tests__/Toggle.test.tsx lines 8-9 with Partial<ToggleProps>. In src/components/toggle/__tests__/ToggleField.test.tsx lines 7-8, type the helper fixture from ToggleFieldProps while requiring the field property, so both migrated test helpers validate their component contracts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/toggle/Toggle.js.flow`:
- Around line 29-31: Update the onMouseEnter and onMouseLeave callback
declarations in the Toggle prop types to use SyntheticMouseEvent<HTMLDivElement>
instead of SyntheticInputEvent<HTMLDivElement>, matching the container div
handlers while preserving their existing callback signatures.
In `@src/components/toggle/ToggleField.tsx`:
- Around line 9-16: Remove the Formik meta bag before forwarding props from
ToggleField. In src/components/toggle/ToggleField.tsx lines 9-16, destructure
and discard meta alongside form; apply the same change in
src/components/toggle/ToggleField.js.flow lines 11-13 so both wrappers prevent
meta from reaching TogglePrimitive and the native input.
---
Nitpick comments:
In `@src/components/toggle/__tests__/Toggle.test.tsx`:
- Around line 8-9: Replace the any-typed getWrapper props in
src/components/toggle/__tests__/Toggle.test.tsx lines 8-9 with
Partial<ToggleProps>. In src/components/toggle/__tests__/ToggleField.test.tsx
lines 7-8, type the helper fixture from ToggleFieldProps while requiring the
field property, so both migrated test helpers validate their component
contracts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2061a3ba-9108-481a-a541-7ede3c409e45
⛔ Files ignored due to path filters (2)
src/components/toggle/__tests__/__snapshots__/Toggle.test.tsx.snapis excluded by!**/*.snapsrc/components/toggle/__tests__/__snapshots__/ToggleField.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/components/toggle/Toggle.js.flowsrc/components/toggle/Toggle.stories.tsxsrc/components/toggle/Toggle.tsxsrc/components/toggle/ToggleField.js.flowsrc/components/toggle/ToggleField.tsxsrc/components/toggle/__tests__/Toggle.test.tsxsrc/components/toggle/__tests__/ToggleField.test.tsxsrc/components/toggle/index.js.flowsrc/components/toggle/index.ts
💤 Files with no reviewable changes (1)
- src/components/toggle/Toggle.stories.tsx
🛑 Comments failed to post (1)
src/components/toggle/Toggle.js.flow (1)
29-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect existing Flow mouse-handler type conventions before updating this API. rg -n -C 2 --glob '*.js.flow' \ 'SyntheticMouseEvent<HTMLDivElement>|onMouseEnter.*Synthetic|onMouseLeave.*Synthetic' \ srcRepository: box/box-ui-elements
Length of output: 856
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "## Toggle.js.flow" cat -n src/components/toggle/Toggle.js.flow | sed -n '1,110p' echo echo "## Toggle.tsx relevant event props/usages" if [ -f src/components/toggle/Toggle.tsx ]; then cat -n src/components/toggle/Toggle.tsx | sed -n '1,160p' else fd -a 'Toggle.tsx$' . fi echo echo "## Flow event declarations in repo" rg -n --glob '*.js.flow' --glob 'flow-typed/**' 'SyntheticMouseEvent|SyntheticInputEvent|type SyntheticMouseEvent|type SyntheticInputEvent' site src flow-typed 2>/dev/null | head -200Repository: box/box-ui-elements
Length of output: 9701
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path for p in Path('.').rglob('*.js.flow'): text = p.read_text(errors='ignore') if 'SyntheticInputEvent<HTMLDivElement>' in text or 'SyntheticMouseEvent<HTMLDivElement>' in text: print(p) for i,line in enumerate(text.splitlines(), 1): if 'SyntheticInputEvent<HTMLDivElement>' in line or 'SyntheticMouseEvent<HTMLDivElement>' in line: print(f' {i}: {line.strip()}') PYRepository: box/box-ui-elements
Length of output: 334
Use mouse event types for the container handlers.
onMouseEnterandonMouseLeaveare attached to the toggle container<div>at line 71, so Flow consumers receive mouse events, not input events. Change both declarations toSyntheticMouseEvent<HTMLDivElement>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/toggle/Toggle.js.flow` around lines 29 - 31, Update the onMouseEnter and onMouseLeave callback declarations in the Toggle prop types to use SyntheticMouseEvent<HTMLDivElement> instead of SyntheticInputEvent<HTMLDivElement>, matching the container div handlers while preserving their existing callback signatures.
This was referenced Aug 11, 2026
bonchevskyi
force-pushed
the
refactor/flow-to-ts-toggle
branch
from
August 12, 2026 11:20
c6e3fbb to
2c9ff5b
Compare
bonchevskyi
force-pushed
the
refactor/flow-to-ts-toggle
branch
from
August 12, 2026 11:33
2c9ff5b to
2748d5e
Compare
vitali-usik
reviewed
Aug 13, 2026
| describe('components/toggle/Toggle', () => { | ||
| const getWrapper = (props = {}) => | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const getWrapper = (props: any = {}) => |
Contributor
There was a problem hiding this comment.
q: Can we use ToggleProps instead of any here?
|
|
||
| describe('components/toggle/ToggleField', () => { | ||
| const getWrapper = (props = {}) => shallow(<ToggleField {...props} />); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
Contributor
There was a problem hiding this comment.
q: Can we use ToggleFieldProps instead of any here?
Comment on lines
+11
to
+13
| // eslint-disable-next-line @typescript-eslint/no-unused-vars -- strip Formik form and meta bags from forwarded props | ||
| form, | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars -- strip Formik form and meta bags from forwarded props |
Contributor
There was a problem hiding this comment.
q: Should we pass form and meta if we do not use them in component?
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.
Convert Toggle component to TypeScript
This PR converts
src/components/togglefrom JavaScript with Flow to TypeScript.Changes
Toggle.jstoToggle.tsxwith exportedTogglePropsinterfaceToggleField.jstoToggleField.tsxwith exportedToggleFieldPropsinterfaceindex.jstoindex.ts, re-exporting the components and their typesToggle.stories.jstoToggle.stories.tsx__tests__/Toggle.test.jsand__tests__/ToggleField.test.jsto.test.tsx.js.flowfiles for backward compatibilityTesting
src/components/toggle; all 10 pass with snapshots matching previous outputyarn lint:tsandflow checkpassSummary by CodeRabbit
New Features
Tests