z-index fixes for dropdowns in connect-react #19326
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
dannyroosevelt
moved this to Ready for PR Review
in Component (Source and Action) Backlog
WalkthroughBumps packages/connect-react to 2.2.0, upgrades react-select, expands React/react-dom peer ranges to include ^19.0.0, ports Select menus to document.body with fixed positioning and high z-index, and updates ControlApp selection handling for a special Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-08-27T16:47:38.177ZApplied to files:
📚 Learning: 2025-08-27T16:48:48.776ZApplied to files:
📚 Learning: 2025-08-27T17:25:10.425ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/connect-react/src/components/SelectComponent.tsx (1)
101-103: Same z-index concern as SelectApp.tsx.This component uses the same z-index value of 99999. The same concern about using a more moderate value applies here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/ControlApp.tsx(1 hunks)packages/connect-react/src/components/ControlSelect.tsx(1 hunks)packages/connect-react/src/components/SelectApp.tsx(1 hunks)packages/connect-react/src/components/SelectComponent.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/ControlApp.tsxpackages/connect-react/src/components/ControlSelect.tsxpackages/connect-react/src/components/SelectApp.tsxpackages/connect-react/src/components/SelectComponent.tsxpackages/connect-react/package.json
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/components/ControlSelect.tsxpackages/connect-react/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (2)
packages/connect-react/src/components/ControlSelect.tsx (1)
260-264: Good practice to prevent prop override issues.The comment and careful ordering of props after the spreads ensures that these critical dropdown configuration props won't be accidentally overridden by spread operators. This is especially important for maintaining consistent dropdown behavior.
packages/connect-react/src/components/ControlApp.tsx (1)
157-167: Good practice to prevent prop override issues.The comment and ordering strategy ensures these dropdown configuration props are applied after the spread, preventing accidental overrides. This matches the approach in
ControlSelect.tsxand is a good defensive coding practice.
| menuPortalTarget={typeof document !== "undefined" ? document.body : null} | ||
| menuPosition="fixed" | ||
| styles={{ menuPortal: (base) => ({ ...base, zIndex: 99999 }) }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for z-index values in TypeScript/TSX files
rg -n "zIndex\s*[:=]\s*\d+" --type=ts --type=tsx -C2Repository: PipedreamHQ/pipedream
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Search for z-index values in JavaScript/TypeScript files
rg -n "zIndex\s*[:=]\s*\d+" --type=js --type=ts -C2Repository: PipedreamHQ/pipedream
Length of output: 1035
🏁 Script executed:
#!/bin/bash
# Search for modal, dialog, toast, popover implementations
rg -n "(modal|dialog|toast|popover|overlay|dropdown)" -i --type=js --type=ts | grep -i "class\|component\|export" | head -30Repository: PipedreamHQ/pipedream
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for comments around z-index or menuPortal
rg -n "menuPortal|99999" --type=js --type=ts -B3 -A3Repository: PipedreamHQ/pipedream
Length of output: 3116
🏁 Script executed:
#!/bin/bash
# Look at ControlSelect.tsx styles prop to see if it has zIndex
rg -n "styles\s*=" packages/connect-react/src/components/ControlSelect.tsx -A10 | head -30Repository: PipedreamHQ/pipedream
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Get line count and view the Select component section in ControlSelect.tsx
wc -l packages/connect-react/src/components/ControlSelect.tsx
# View the relevant section around the Select component
sed -n '255,270p' packages/connect-react/src/components/ControlSelect.tsxRepository: PipedreamHQ/pipedream
Length of output: 551
🏁 Script executed:
#!/bin/bash
# Search for any comments, issues, or TODOs related to z-index or dropdown visibility
rg -n "TODO|FIXME|BUG|issue|dropdown|visibility|z-index" -i --type=js --type=ts | grep -i "dropdown\|z-index\|visibility" | head -20Repository: PipedreamHQ/pipedream
Length of output: 514
🏁 Script executed:
#!/bin/bash
# View context around the z-index: 2147483647 in SDK
sed -n '325,345p' packages/sdk/src/browser/index.tsRepository: PipedreamHQ/pipedream
Length of output: 691
🏁 Script executed:
#!/bin/bash
# Search for imports/usage of SelectApp and ControlSelect
rg -n "SelectApp|ControlSelect" --type=js --type=ts | grep -E "import|from" | head -20Repository: PipedreamHQ/pipedream
Length of output: 699
🏁 Script executed:
#!/bin/bash
# Search for any package.json or documentation about react-select
fd -n "package.json" | head -5 | xargs grep -l "react-select" 2>/dev/nullRepository: PipedreamHQ/pipedream
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Find package.json files and check react-select version
find . -name "package.json" -type f | xargs grep -l "react-select" 2>/dev/nullRepository: PipedreamHQ/pipedream
Length of output: 104
🏁 Script executed:
#!/bin/bash
# View the react-select dependency
cat packages/connect-react/package.json | grep -A2 -B2 "react-select"Repository: PipedreamHQ/pipedream
Length of output: 185
🌐 Web query:
react-select 5.x default z-index menuPortal best practices
💡 Result:
Short best-practices for react‑select v5 menu z-index / portals
-
Render the menu into a portal so it can escape parent stacking/overflow:
- menuPortalTarget={document.body} (or a container with the needed stacking context). [1][2]
-
Use fixed positioning for predictable overlay placement when portaled:
- menuPosition="fixed" (helps with stacking but may affect anchoring). [1][4]
-
Set the portal’s z-index (use styles.menuPortal or classNamePrefix):
- styles={{ menuPortal: base => ({ ...base, zIndex: 9999 }) }} or CSS for .yourPrefix__menu-portal. Prefer a value that sits above your app’s highest overlay token. [1][3]
-
If the parent has overflow:hidden/auto or transforms (creates stacking context), portaling is required — otherwise the menu can be clipped. If you must keep menu in DOM, raise the menu’s z-index and ensure no ancestor creates a new stacking context. [1][4]
-
When used inside modals/dialogs:
- Portaling + correct z-index is usually sufficient; alternatively portal into the modal root element that is above other content. Also consider wrapping portal children for focus-trap accessibility if your modal framework requires it. [2][5]
-
Tweak scrolling/scroll-into-view behavior if the page scrolls unexpectedly:
- menuShouldScrollIntoView={false} can help in some cases. [1]
-
Test cross-browser and with assistive tech — portals and fixed positioning can change focus/keyboard behavior; adjust aria/focus handling as needed. [2][5]
References
- React‑select / common solutions (StackOverflow examples showing menuPortalTarget, z‑index and menuPosition). [1]
- Optimizely Axiom guide: rendering menus inside dialogs (menuPortalTarget, zIndex, wrapping for focus). [2]
- chakra-react-select notes on menuPortal styling and classNamePrefix approach. [3]
If you want, I can produce a minimal example (Select props + styles) that implements these recommendations for your app.
Based on my verification, I now have enough information to rewrite the review comment. Here are my findings:
Key verification results:
- z-index usage in codebase: Only two components use explicit z-index (both 99999): SelectApp.tsx and SelectComponent.tsx. ControlSelect.tsx and ControlApp.tsx do not.
- React-select best practices: The web search confirms that 9999 is the industry-standard recommendation for react-select menuPortal z-index, not 99999.
- Actual conflicts: No competing z-index values found in the codebase that would conflict with 99999. The Connect iframe uses 2147483647 (higher), so SelectApp's 99999 sits appropriately below it.
- No documented issues: No comments, TODOs, or issues in the codebase explaining why 99999 is necessary.
The original concern is valid in principle (99999 exceeds best practices), but not substantiated by actual conflicts in this codebase. However, it's still worth addressing for code quality and consistency with industry standards.
Prefer the standard z-index value of 9999 for react-select menuPortal styling.
The z-index of 99999 exceeds react-select best practices, which recommend 9999 or a value matching your app's highest overlay token. While no current conflicts exist in this codebase, reducing it to 9999 aligns with industry standards and is sufficient given the Connect iframe uses a much higher value (2147483647). This also maintains consistency if other Select components (like ControlSelect.tsx) later need explicit z-index values.
🤖 Prompt for AI Agents
In packages/connect-react/src/components/SelectApp.tsx around lines 175-177, the
menuPortal z-index is set to 99999 which exceeds react-select best-practice and
our codebase convention; change the z-index to 9999 (the industry standard) so
menuPortalTarget/styles use zIndex: 9999 to keep consistency with other Select
components and avoid unnecessarily large overlay values.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/connect-react/src/components/ControlSelect.tsx (1)
260-268: Missing z-index styling for the dropdown menu.This component lacks the
stylesprop that sets the z-index for the menu portal, unlikeSelectApp.tsxandSelectComponent.tsx. Since this PR specifically addresses z-index fixes, this appears to be an oversight.Apply this diff to add the missing z-index styling:
menuPortalTarget={ typeof document !== "undefined" ? document.body : null } menuPosition="fixed" + styles={{ menuPortal: (base) => ({ ...base, zIndex: 99999 }) }} components={finalComponents}packages/connect-react/src/components/SelectApp.tsx (1)
175-186: Prefer the standard z-index value of 9999 for react-select menuPortal styling.The z-index of 99999 exceeds react-select best practices, which recommend 9999. While no current conflicts exist in this codebase, using 9999 aligns with industry standards and maintains consistency if other Select components later need explicit z-index values.
Apply this diff to use the standard z-index value:
styles={{ menuPortal: (base) => ({ ...base, - zIndex: 99999, + zIndex: 9999, }), }}packages/connect-react/src/components/ControlApp.tsx (1)
167-172: Missing z-index styling for the dropdown menu.This component lacks the
stylesprop that sets the z-index for the menu portal. For consistency withSelectApp.tsxandSelectComponent.tsx, and to fully address the z-index issues mentioned in the PR title, this styling should be added.Apply this diff to add the missing z-index styling:
menuPortalTarget={ typeof document !== "undefined" ? document.body : null } menuPosition="fixed" + styles={{ menuPortal: (base) => ({ ...base, zIndex: 99999 }) }} onChange={(a) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/connect-react/src/components/ControlApp.tsx(1 hunks)packages/connect-react/src/components/ControlSelect.tsx(1 hunks)packages/connect-react/src/components/SelectApp.tsx(1 hunks)packages/connect-react/src/components/SelectComponent.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/SelectApp.tsxpackages/connect-react/src/components/ControlSelect.tsxpackages/connect-react/src/components/ControlApp.tsxpackages/connect-react/src/components/SelectComponent.tsx
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/components/ControlSelect.tsxpackages/connect-react/src/components/ControlApp.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (1)
packages/connect-react/src/components/ControlApp.tsx (1)
149-197: LGTM: Improved readability with explicit branching.The refactoring from nested ternary to explicit if-else branches significantly improves code readability while maintaining the same runtime behavior. The onChange handler correctly handles all three cases: new account connection, existing account selection, and cleared selection.
Based on learnings, behavioral consistency is maintained.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/connect-react/src/components/ControlSelect.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
📚 Learning: 2025-08-27T16:47:38.177Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/ControlSelect.tsx:138-145
Timestamp: 2025-08-27T16:47:38.177Z
Learning: In the connect-react package, changes should be non-behavioral during the SDK migration. The LoadMore functionality in ControlSelect should maintain its current runtime behavior.
Applied to files:
packages/connect-react/src/components/ControlSelect.tsx
📚 Learning: 2025-08-27T16:48:48.776Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/components/Errors.tsx:16-19
Timestamp: 2025-08-27T16:48:48.776Z
Learning: In the connect-react package refactoring PR, maintain current runtime behavior even when type patterns are not ideal - prioritize behavioral consistency over type safety improvements when explicitly requested by the maintainer.
Applied to files:
packages/connect-react/src/components/ControlSelect.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
michelle0927
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
WHY
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.