[WEB-2783]fix: dropdown visibility in onboarding#6329
Conversation
WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant Component as InviteMembers
participant Popper as react-popper
participant Reference as ReferenceElement
participant Dropdown as DropdownElement
Component->>Popper: usePopper(referenceElement, popperElement)
Popper-->>Component: Return styles and attributes
Component->>Reference: setReferenceElement
Component->>Dropdown: setPopperElement
Dropdown->>Dropdown: Apply positioning styles
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/components/onboarding/invite-members.tsx (1)
216-221: Consider adding a fallback or conditional styling for the dropdown.
The absolute positioning andz-10class likely suffice for layering, but ensure no parent element withoverflow: hiddenor a lowerz-indexstacks over the dropdown, especially in nested contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/onboarding/invite-members.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-web
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
web/core/components/onboarding/invite-members.tsx (6)
18-18: Great addition to support popper-based dropdown positioning.
ImportingusePopperfrom thereact-popperlibrary is a good approach for accurate dropdown positioning.
20-20: CouplingListboxwithreact-popperis a strong combination.
Using Headless UI’s<Listbox>in conjunction withusePoppercreates a more controlled and accessible dropdown. Good job integrating these capabilities together.
104-105: Handle reference element lifecycle carefully.
Storing references to the button (referenceElement) and popper container (popperElement) is perfectly valid. However, ensure that these references are reset or cleaned up when unmounting the component to avoid potential memory leaks.
133-144: Confirm Popper modifiers and placement for different devices.
ThepreventOverflowmodifier withpadding: 12is a good start. Large screens, smaller viewports, or edge-cases with highly dynamic content may still need offset adjustments. Consider verifying the dropdown behavior in all UI breakpoints to ensure it stays visible.
194-194: Efficient reference assignment to Listbox button.
AssigningrefviasetReferenceElementstreamlines integration with the popper’s positioning logic. Confirm that events reliant on the reference element (like clicks, hovers) still work as expected.
223-245: Well-structured list of roles in the dropdown.
Mapping overROLE_DETAILSis clean, and usingselectedto highlight the chosen role is straightforward. Ensure that roles are synced with any future data source improvements (for instance, if role definitions become user-configurable).
3 participants
Description
Added popper to
ListBoxto improve visibility and positioning of dropdown.Type of Change
References
WEB-2783
Summary by CodeRabbit
react-popperlibrary for improved dropdown positioning