refactor(dropdown-menu): migrate DropdownMenu from Flow to TypeScript - #4731
Open
bonchevskyi wants to merge 1 commit into
Open
refactor(dropdown-menu): migrate DropdownMenu from Flow to TypeScript#4731bonchevskyi wants to merge 1 commit into
bonchevskyi wants to merge 1 commit into
Conversation
Contributor
WalkthroughAdds Flow and TypeScript implementations of ChangesDropdown menu feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TriggerButton
participant DropdownMenu
participant Document
participant Tether
participant Menu
TriggerButton->>DropdownMenu: click or keyboard input
DropdownMenu->>Document: register dismissal listeners
DropdownMenu->>Tether: render positioned menu
Tether->>Menu: mount menu content
Document->>DropdownMenu: outside interaction
DropdownMenu->>Menu: close menu
DropdownMenu->>TriggerButton: restore focus
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🤖 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/dropdown-menu/__tests__/DropdownMenu.test.tsx`:
- Around line 638-640: Fix the malformed test.each title in the “should not call
handleMenuClose() when event target is within the $description” case by removing
the stray opening template-literal/parenthesis characters and ensuring the title
has balanced delimiters. Keep the existing parameterized test and its behavior
unchanged.
🪄 Autofix (Beta)
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: e42d001c-9098-4050-9df6-3b0799bf3f85
⛔ Files ignored due to path filters (1)
src/components/dropdown-menu/__tests__/__snapshots__/MenuToggle.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/components/dropdown-menu/DropdownMenu.js.flowsrc/components/dropdown-menu/DropdownMenu.stories.tsxsrc/components/dropdown-menu/DropdownMenu.tsxsrc/components/dropdown-menu/MenuToggle.js.flowsrc/components/dropdown-menu/MenuToggle.tsxsrc/components/dropdown-menu/__tests__/DropdownMenu.test.tsxsrc/components/dropdown-menu/__tests__/MenuToggle.test.tsxsrc/components/dropdown-menu/index.js.flowsrc/components/dropdown-menu/index.tssrc/components/index.ts
bonchevskyi
force-pushed
the
refactor/flow-to-ts-dropdown-menu
branch
from
July 30, 2026 10:44
a6e15b8 to
ba13361
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/components/dropdown-menu/DropdownMenu.tsx (3)
160-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark the unused positional param.
isKeyboardEventis required positionally byMenu'sonClosecontract but unused here; prefixing with_(or adding a short comment) prevents future no-unused-vars churn.♻️ Suggested change
- handleMenuClose = (isKeyboardEvent: boolean, event: React.SyntheticEvent | MouseEvent) => { + // eslint-disable-next-line `@typescript-eslint/no-unused-vars` -- positional param required by Menu's onClose signature + handleMenuClose = (_isKeyboardEvent: boolean, event: React.SyntheticEvent | MouseEvent) => {🤖 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/dropdown-menu/DropdownMenu.tsx` around lines 160 - 163, Rename the unused positional parameter in handleMenuClose to use the project’s underscore convention, while preserving its position for Menu’s onClose contract and leaving the existing closeMenu and focusButton behavior unchanged.
56-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate
stateexplicitly.
state = { initialFocusIndex: null, isOpen: false }infersinitialFocusIndex: null, so reads ofthis.state.initialFocusIndexare typednullrather thannumber | null. Annotating keeps the declared state contract authoritative.♻️ Suggested change
- state = { + state: DropdownMenuState = { initialFocusIndex: null, isOpen: false, };🤖 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/dropdown-menu/DropdownMenu.tsx` around lines 56 - 59, Annotate the state property in the DropdownMenu component with its intended state type so initialFocusIndex is declared as number | null rather than inferred as only null, while preserving isOpen as a boolean and keeping the existing initial values.
198-217: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe blanket cast hides a real runtime hazard.
React.Children.toArraycan yield strings, numbers, and fragments, but the assertion toReact.ReactElement<Record<string, unknown>>[]tells the compiler they're all elements. With<DropdownMenu>text<Menu /></DropdownMenu>, the length check passes and Line 215 dereferences.propson a string →TypeErrorinstead of the intended contract error. Validating withReact.isValidElementkeeps the type narrowing honest and makes the failure mode match the existing error message.♻️ Proposed narrowing
- const elements = React.Children.toArray(children) as React.ReactElement<Record<string, unknown>>[]; + const elements = React.Children.toArray(children).filter( + (child): child is React.ReactElement<Record<string, unknown>> => React.isValidElement(child), + ); if (elements.length !== 2) { throw new Error('DropdownMenu must have exactly two children: A button component and a <Menu>'); }🤖 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/dropdown-menu/DropdownMenu.tsx` around lines 198 - 217, Replace the blanket cast in the DropdownMenu child-processing logic with validation using React.isValidElement for both children before accessing menuButton.props or menu.props. Reject any non-element child with the existing “exactly two children” contract error, while preserving the current handling for two valid elements.src/components/dropdown-menu/DropdownMenu.js.flow (1)
182-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the Flow shim declaration-only.
DropdownMenu.js.flowduplicates the entire implementation fromDropdownMenu.tsx; conversion shims should expose only type declarations, e.g.declare export default class DropdownMenu..., so Flow consumers don’t type-check against a copy that can drift.🤖 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/dropdown-menu/DropdownMenu.js.flow` around lines 182 - 291, Replace the duplicated render implementation in DropdownMenu.js.flow with a declaration-only Flow shim for the DropdownMenu class, exposing its public type contract through a declare export default class declaration. Remove the copied render logic and reference the existing DropdownMenu.tsx implementation as the source of behavior.
🤖 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/dropdown-menu/DropdownMenu.tsx`:
- Around line 13-28: Realign the prop JSDoc comments in
src/components/dropdown-menu/DropdownMenu.tsx lines 13-28 and
src/components/dropdown-menu/DropdownMenu.js.flow lines 14-25: place the
scroll-parent, visible-window, and right-align descriptions on
constrainToScrollParent, constrainToWindow, and isRightAligned respectively, and
remove or attach the stray menu-open description to onMenuOpen. Keep both files’
documentation identical.
---
Nitpick comments:
In `@src/components/dropdown-menu/DropdownMenu.js.flow`:
- Around line 182-291: Replace the duplicated render implementation in
DropdownMenu.js.flow with a declaration-only Flow shim for the DropdownMenu
class, exposing its public type contract through a declare export default class
declaration. Remove the copied render logic and reference the existing
DropdownMenu.tsx implementation as the source of behavior.
In `@src/components/dropdown-menu/DropdownMenu.tsx`:
- Around line 160-163: Rename the unused positional parameter in handleMenuClose
to use the project’s underscore convention, while preserving its position for
Menu’s onClose contract and leaving the existing closeMenu and focusButton
behavior unchanged.
- Around line 56-59: Annotate the state property in the DropdownMenu component
with its intended state type so initialFocusIndex is declared as number | null
rather than inferred as only null, while preserving isOpen as a boolean and
keeping the existing initial values.
- Around line 198-217: Replace the blanket cast in the DropdownMenu
child-processing logic with validation using React.isValidElement for both
children before accessing menuButton.props or menu.props. Reject any non-element
child with the existing “exactly two children” contract error, while preserving
the current handling for two valid elements.
🪄 Autofix (Beta)
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: 59fc3a5f-a42e-41d5-a498-c0fcd4e07826
⛔ Files ignored due to path filters (1)
src/components/dropdown-menu/__tests__/__snapshots__/MenuToggle.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/components/dropdown-menu/DropdownMenu.js.flowsrc/components/dropdown-menu/DropdownMenu.stories.tsxsrc/components/dropdown-menu/DropdownMenu.tsxsrc/components/dropdown-menu/MenuToggle.js.flowsrc/components/dropdown-menu/MenuToggle.tsxsrc/components/dropdown-menu/__tests__/DropdownMenu.test.tsxsrc/components/dropdown-menu/__tests__/MenuToggle.test.tsxsrc/components/dropdown-menu/index.js.flowsrc/components/dropdown-menu/index.tssrc/components/index.ts
bonchevskyi
force-pushed
the
refactor/flow-to-ts-dropdown-menu
branch
from
July 31, 2026 13:43
ba13361 to
7a7caf9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/dropdown-menu/MenuToggle.js.flow`:
- Around line 8-12: Rename the private Props type in MenuToggle.js.flow to
exported MenuToggleProps, use MenuToggleProps for the MenuToggle component
annotation, and re-export MenuToggleProps from the dropdown-menu Flow barrel
index.js.flow to match the TypeScript public API.
🪄 Autofix (Beta)
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: eec89f4b-5b2e-444f-ac3b-2d9f3171331c
⛔ Files ignored due to path filters (1)
src/components/dropdown-menu/__tests__/__snapshots__/MenuToggle.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/components/dropdown-menu/DropdownMenu.js.flowsrc/components/dropdown-menu/DropdownMenu.stories.tsxsrc/components/dropdown-menu/DropdownMenu.tsxsrc/components/dropdown-menu/MenuToggle.js.flowsrc/components/dropdown-menu/MenuToggle.tsxsrc/components/dropdown-menu/__tests__/DropdownMenu.test.tsxsrc/components/dropdown-menu/__tests__/MenuToggle.test.tsxsrc/components/dropdown-menu/index.js.flowsrc/components/dropdown-menu/index.tssrc/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/dropdown-menu/tests/MenuToggle.test.tsx
- src/components/dropdown-menu/index.js.flow
- src/components/dropdown-menu/MenuToggle.tsx
- src/components/dropdown-menu/index.ts
- src/components/index.ts
- src/components/dropdown-menu/DropdownMenu.stories.tsx
- src/components/dropdown-menu/DropdownMenu.tsx
- src/components/dropdown-menu/tests/DropdownMenu.test.tsx
- src/components/dropdown-menu/DropdownMenu.js.flow
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: 1
🤖 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/dropdown-menu/MenuToggle.js.flow`:
- Around line 8-12: Rename the private Props type in MenuToggle.js.flow to
exported MenuToggleProps, use MenuToggleProps for the MenuToggle component
annotation, and re-export MenuToggleProps from the dropdown-menu Flow barrel
index.js.flow to match the TypeScript public API.
🪄 Autofix (Beta)
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: eec89f4b-5b2e-444f-ac3b-2d9f3171331c
⛔ Files ignored due to path filters (1)
src/components/dropdown-menu/__tests__/__snapshots__/MenuToggle.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/components/dropdown-menu/DropdownMenu.js.flowsrc/components/dropdown-menu/DropdownMenu.stories.tsxsrc/components/dropdown-menu/DropdownMenu.tsxsrc/components/dropdown-menu/MenuToggle.js.flowsrc/components/dropdown-menu/MenuToggle.tsxsrc/components/dropdown-menu/__tests__/DropdownMenu.test.tsxsrc/components/dropdown-menu/__tests__/MenuToggle.test.tsxsrc/components/dropdown-menu/index.js.flowsrc/components/dropdown-menu/index.tssrc/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/dropdown-menu/tests/MenuToggle.test.tsx
- src/components/dropdown-menu/index.js.flow
- src/components/dropdown-menu/MenuToggle.tsx
- src/components/dropdown-menu/index.ts
- src/components/index.ts
- src/components/dropdown-menu/DropdownMenu.stories.tsx
- src/components/dropdown-menu/DropdownMenu.tsx
- src/components/dropdown-menu/tests/DropdownMenu.test.tsx
- src/components/dropdown-menu/DropdownMenu.js.flow
🛑 Comments failed to post (1)
src/components/dropdown-menu/MenuToggle.js.flow (1)
8-12: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 3 'MenuToggleProps|MenuToggle' src --glob '*.js.flow' --glob '*.ts' --glob '*.tsx'Repository: box/box-ui-elements
Length of output: 9366
Export the Flow prop type to keep the public API aligned.
src/components/dropdown-menu/index.tsexposesMenuToggleProps, butsrc/components/dropdown-menu/MenuToggle.js.flowkeeps the type private and does not re-export it fromsrc/components/dropdown-menu/index.js.flow. Rename this type toMenuToggleProps, export it, use it forMenuToggle, and re-export it from the Flow barrel so Flow consumers match the TypeScript API.🤖 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/dropdown-menu/MenuToggle.js.flow` around lines 8 - 12, Rename the private Props type in MenuToggle.js.flow to exported MenuToggleProps, use MenuToggleProps for the MenuToggle component annotation, and re-export MenuToggleProps from the dropdown-menu Flow barrel index.js.flow to match the TypeScript public API.
bonchevskyi
force-pushed
the
refactor/flow-to-ts-dropdown-menu
branch
from
July 31, 2026 14:09
7a7caf9 to
8502500
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dropdown-menu/DropdownMenu.tsx (1)
13-28: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winJSDoc comments remain off by one prop.
This issue is still present from a previous review. The doc comment above
className(line 13) describesconstrainToScrollParent. The comment aboveconstrainToScrollParent(line 15) describesconstrainToWindow. The comment aboveconstrainToWindow(line 17) describesisRightAligned. The comment aboveisRightAligned(line 23, "Function called when menu is opened") is a stray leftover that belongs toonMenuOpen.📝 Proposed fix to realign the JSDoc comments
export interface DropdownMenuProps { bodyElement?: HTMLElement; children: React.ReactNode; - /** Forces menu to render within the scroll parent */ className?: string; + /** Forces menu to render within the scroll parent */ constrainToScrollParent: boolean; - /** Forces menu to render within the visible window */ + /** Forces menu to render within the visible window */ constrainToWindow: boolean; - /** Right aligns menu to button */ constrainToWindowWithPin?: boolean; /** Enables responsive behaviors for this component */ isResponsive?: boolean; - /** Function called when menu is opened */ + /** Right aligns menu to button */ isRightAligned: boolean; /** Handler for dropdown menu close events */ onMenuClose?: (event: React.SyntheticEvent | MouseEvent) => void; /** Handler for dropdown menu open events */ onMenuOpen?: () => void;🤖 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/dropdown-menu/DropdownMenu.tsx` around lines 13 - 28, Realign the JSDoc comments in the DropdownMenu props declaration so each description documents its corresponding prop: document className as the CSS class, constrainToScrollParent as scroll-parent rendering, constrainToWindow as visible-window alignment, and isRightAligned as right alignment. Remove the stray function-description comment above isRightAligned and ensure the onMenuOpen callback retains the menu-open handler documentation.
🧹 Nitpick comments (1)
src/components/dropdown-menu/DropdownMenu.tsx (1)
12-12: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winType
childrenas a two-element tuple to enforce the contract at compile time.
children: React.ReactNodeallows strings, fragments, and arrays, while the two-child requirement is only checked at runtime. The cast afterReact.Children.toArray(children)also bypasses type checking. Typechildrenas[React.ReactElement, React.ReactElement]instead and destructure it directly.src/components/dropdown-menu/DropdownMenu.tsx:12
🤖 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/dropdown-menu/DropdownMenu.tsx` at line 12, Update the DropdownMenu component’s children prop from React.ReactNode to [React.ReactElement, React.ReactElement], then destructure the tuple directly instead of converting with React.Children.toArray or casting. Preserve the existing two-child rendering behavior while enforcing the contract at compile time.
🤖 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.
Duplicate comments:
In `@src/components/dropdown-menu/DropdownMenu.tsx`:
- Around line 13-28: Realign the JSDoc comments in the DropdownMenu props
declaration so each description documents its corresponding prop: document
className as the CSS class, constrainToScrollParent as scroll-parent rendering,
constrainToWindow as visible-window alignment, and isRightAligned as right
alignment. Remove the stray function-description comment above isRightAligned
and ensure the onMenuOpen callback retains the menu-open handler documentation.
---
Nitpick comments:
In `@src/components/dropdown-menu/DropdownMenu.tsx`:
- Line 12: Update the DropdownMenu component’s children prop from
React.ReactNode to [React.ReactElement, React.ReactElement], then destructure
the tuple directly instead of converting with React.Children.toArray or casting.
Preserve the existing two-child rendering behavior while enforcing the contract
at compile time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3e42533-5583-4afd-891f-6f98f9aea8d9
⛔ Files ignored due to path filters (1)
src/components/dropdown-menu/__tests__/__snapshots__/MenuToggle.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/components/dropdown-menu/DropdownMenu.js.flowsrc/components/dropdown-menu/DropdownMenu.stories.tsxsrc/components/dropdown-menu/DropdownMenu.tsxsrc/components/dropdown-menu/MenuToggle.js.flowsrc/components/dropdown-menu/MenuToggle.tsxsrc/components/dropdown-menu/__tests__/DropdownMenu.test.tsxsrc/components/dropdown-menu/__tests__/MenuToggle.test.tsxsrc/components/dropdown-menu/index.js.flowsrc/components/dropdown-menu/index.tssrc/components/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/dropdown-menu/tests/MenuToggle.test.tsx
- src/components/dropdown-menu/MenuToggle.tsx
- src/components/index.ts
- src/components/dropdown-menu/index.js.flow
- src/components/dropdown-menu/MenuToggle.js.flow
- src/components/dropdown-menu/DropdownMenu.js.flow
- src/components/dropdown-menu/DropdownMenu.stories.tsx
- src/components/dropdown-menu/tests/DropdownMenu.test.tsx
- src/components/dropdown-menu/index.ts
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
1 participant
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
DropdownMenucomponent to TypeScriptThis PR converts
src/components/dropdown-menufrom JavaScript with Flow to TypeScript.Changes
DropdownMenu.jsandMenuToggle.jsto.tsxwith exported props interfacesindex.jstoindex.ts, re-exporting the components and types.js.flowfiles for backward compatibilityDropdownMenuexports tosrc/components/index.tsTesting
src/components/dropdown-menu; all 45 pass and the snapshot matches previous outputyarn lint:ts, ESLint, and Flow checks passSummary by CodeRabbit