Skip to content

[WEB-4441]fix: members account type dropdown position#7759

Merged
sriramveeraghanta merged 1 commit intopreviewfrom
fix-members_list_dropdown
Sep 11, 2025
Merged

[WEB-4441]fix: members account type dropdown position#7759
sriramveeraghanta merged 1 commit intopreviewfrom
fix-members_list_dropdown

Conversation

@vamsikrishnamathala
Copy link
Member

@vamsikrishnamathala vamsikrishnamathala commented Sep 10, 2025

Description

Migrated CustomSelect dropdown from HeadlessUI Listbox to Combobox with portal rendering to fix dropdown positioning relative to viewport, and improve accessibility.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Screenshots and Media (if applicable)

Test Scenarios

References

Summary by CodeRabbit

  • Refactor

    • Rebuilt CustomSelect’s underlying dropdown implementation without changing its public API.
  • Bug Fixes

    • More reliable positioning and dismissal when clicking outside.
    • Prevents content clipping by constraining panel height with internal scrolling.
  • Style

    • Improved layering to appear above surrounding content.
    • Adjusted default widths for better alignment.
  • Documentation

    • No changes required for consumers; existing usage continues to work as before.

@vamsikrishnamathala vamsikrishnamathala self-assigned this Sep 10, 2025
@vamsikrishnamathala vamsikrishnamathala added the 🐛bug Something isn't working label Sep 10, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Walkthrough

Migrated CustomSelect from Headless UI Listbox to Combobox, rendering options via a React Portal to document.body. Adjusted Popper refs and container structure for positioning, updated styling and scrolling constraints, and removed an onClick close handler in favor of portal-based outside-click handling. Public API exports remain unchanged.

Changes

Cohort / File(s) Summary of Changes
Combobox migration and portalized options
packages/ui/src/dropdowns/custom-select.tsx
Replaced Listbox with Combobox (wrapper, Button, Options, Option). Rendered Combobox.Options via createPortal to document.body. Reworked Popper ref handling with an outer container and inner max-height wrapper. Updated classes (min widths, z-index, scroll constraints). Removed inline close handler; relies on portal/outside-click behavior. Exports unchanged.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor U as User
    participant CS as CustomSelect
    participant CB as Combobox
    participant PB as Portal (document.body)
    participant P as Popper/Positioning

    U->>CS: Click trigger
    CS->>CB: Open Combobox
    CB->>PB: Render Options via createPortal
    PB->>P: Attach popper refs (setPopperElement)
    P-->>PB: Compute position (placement, styles)

    U->>PB: Navigate/select option
    PB->>CB: onChange(value)
    CB->>CS: Update selected state
    CS->>CB: Close Combobox
    CB-->>PB: Unmount Options (portal)
    note over PB,CB: Outside clicks handled via portal container
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

🌐frontend, requires approval

Suggested reviewers

  • gakshita
  • anmolsinghbhatia

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description supplies a clear summary and correctly flags it as a bug fix, but it omits any test scenarios under “Test Scenarios” and does not link the related issue or add screenshots if relevant, leaving those sections empty. Please populate the “Test Scenarios” section with the specific tests you ran to verify the dropdown behavior (e.g., unit or integration tests, manual steps) and add a link to the related WEB-4441 issue in the “References” section or note if no screenshots are required.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely identifies the key change—fixing the members account type dropdown’s positioning—without unnecessary details, and it directly reflects the primary user-facing issue addressed by the PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

A hop, a skip, I open the box—
The list became a combo, clever as fox.
Through portals I peek, options on high,
Popper aligns them under the sky.
Clickity-click, selection just right—
Carrot-approved UX, crisp and light! 🥕✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-members_list_dropdown

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@makeplane
Copy link

makeplane bot commented Sep 10, 2025

Pull Request Linked with Plane Work Items

Comment Automatically Generated by Plane

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/ui/src/dropdowns/custom-select.tsx (1)

108-116: Potential z-index conflict with other UI elements

The z-index value of z-30 (z-index: 30 in Tailwind) might not be sufficient to ensure the dropdown always appears above other UI elements like modals or overlays, which typically use higher z-index values.

Consider using a higher z-index value or managing it through a z-index system:

               className={cn(
-                "my-1 overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 text-xs shadow-custom-shadow-rg focus:outline-none min-w-48 whitespace-nowrap z-30",
+                "my-1 overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 text-xs shadow-custom-shadow-rg focus:outline-none min-w-48 whitespace-nowrap z-50",
                 optionsClassName
               )}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30b1751 and 72752c2.

📒 Files selected for processing (1)
  • packages/ui/src/dropdowns/custom-select.tsx (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui/src/dropdowns/custom-select.tsx (2)
packages/ui/src/utils/classname.tsx (1)
  • cn (4-4)
packages/ui/src/dropdowns/helper.tsx (1)
  • ICustomSelectItemProps (93-97)
⏰ 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: Build and lint web apps
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/ui/src/dropdowns/custom-select.tsx (3)

1-1: Migration to Combobox looks good!

The migration from Listbox to Combobox is appropriate for this use case, as Combobox provides better support for portal rendering and keyboard navigation while maintaining similar dropdown functionality.


105-130: Good implementation of portal rendering for dropdown positioning!

The use of createPortal to render the dropdown options in document.body effectively solves the positioning issues relative to the viewport. The data-prevent-outside-click attribute combined with the portal ensures proper click handling.


58-67: Good accessibility implementation with Combobox

The Combobox implementation maintains keyboard navigation support through onKeyDown handler and properly manages focus states. The disabled prop is correctly propagated to the Combobox component.

@sriramveeraghanta sriramveeraghanta merged commit 4fe2ef7 into preview Sep 11, 2025
7 of 8 checks passed
@sriramveeraghanta sriramveeraghanta deleted the fix-members_list_dropdown branch September 11, 2025 08:50
@coderabbitai coderabbitai bot mentioned this pull request Sep 11, 2025
6 tasks
aaryan610 added a commit that referenced this pull request Sep 30, 2025
* chore: added access for workspace admin to edit project settings

* chore: workspace admin to update members details

* chore: workspace admin to label, state, workflow settings

* Revert "chore: added access for workspace admin to edit project settings"

This reverts commit 803b56514887339d884eaef170de8a9e4ecfda8c.

* chore: updated worspace admin access for projects

* Revert "chore: workspace admin to update members details"

This reverts commit ac465d618d7a89ef696db3484e515957b6b5e264.

* Revert "chore: workspace admin to label, state, workflow settings"

This reverts commit f01a89604e71792096cbae8e029cac160ea209fb.

* chore: workspace admin access in permission classes and decorator

* chore: check for teamspace members

* chore: refactor permission logic

* [WIKI-632] chore: accept additional props for document collaborative editor (#7718)

* chore: add collaborative document editor extended props

* fix: additional rich text extension props

* fix: formatting

* chore: add types to the trailing node extension

---------

Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com>

* [WEB-4854] chore: project admin accesss to workspace admins (#7749)

* chore: project admin accesss to workspace admins

* chore: frontend changes

* chore: remove console.log

* chore: refactor permission decorator

* chore: role enum

* chore: rearrange role_choices

* Potential fix for code scanning alert no. 636: URL redirection from remote source (#7760)

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* [WEB-4441]fix: members account type dropdown position #7759

* [WEB-4857] fix: applied filters root update #7750

* [WEB-4858]chore: updated content for error page (#7766)

* chore: updated content for error page

* chore: updated btn url

* fix: merge conflicts

* fix: merge conflicts

* fix: use enum for roles

---------

Co-authored-by: vamsikrishnamathala <matalav55@gmail.com>
Co-authored-by: Lakhan Baheti <94619783+1akhanBaheti@users.noreply.github.com>
Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com>
Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Vamsi Krishna <46787868+vamsikrishnamathala@users.noreply.github.com>
yarikoptic pushed a commit to yarikoptic/plane that referenced this pull request Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛bug Something isn't working ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants