Skip to content

fix(graph): fix toggle click and multiple dropdown issues#16874

Merged
charlesBochet merged 3 commits intotwentyhq:mainfrom
aditya-cherukuru:fix/graph-small-bugs
Jan 8, 2026
Merged

fix(graph): fix toggle click and multiple dropdown issues#16874
charlesBochet merged 3 commits intotwentyhq:mainfrom
aditya-cherukuru:fix/graph-small-bugs

Conversation

@aditya-cherukuru
Copy link
Copy Markdown
Contributor

Fixes #16843

Summary

This PR fixes two bugs in graph settings:

  1. Multiple dropdowns opening simultaneously
  2. Toggle switches not clickable

Fix 1: Multiple dropdowns

File: ChartSettingItem.tsx

Added closeAnyOpenDropdown() before opening a new dropdown. This follows the existing pattern used in:


Fix 2: Toggle switches not clickable

File: MenuItemToggle.tsx (twenty-ui)

Investigation

I traced the toggle click flow and compared working vs non-working usages:

The component structure and props were identical, so I examined the HTML output.

Root Cause

Found nested <label> elements:

<!-- Before (problematic) -->
<label htmlFor="id123">           <!-- MenuItemToggle's outer label -->
  <div>...content...</div>
  <label>                         <!-- Toggle's internal label -->
    <input id="id123" type="checkbox">
  </label>
</label>

While htmlFor theoretically links to the checkbox, nested labels are invalid HTML and can cause click propagation issues in certain browser contexts (particularly when combined with React event handling and SelectableList keyboard navigation).

Solution

Changed StyledToggleContainer from label to div:

- const StyledToggleContainer = styled.label`
+ const StyledToggleContainer = styled.div`

The Toggle component already handles clicks via its internal label wrapper, so the outer label was redundant.

Also removed unused useId, htmlFor, and id props that were only needed for the label association.

Testing

  • ✅ TypeScript checks pass on twenty-ui
  • ✅ TypeScript checks pass on twenty-front
  • No breaking changes to MenuItemToggle API
  • All existing usages of MenuItemToggle should continue working (and potentially work better)

- Close any open dropdown before opening a new one in graph settings

- Fix MenuItemToggle nested label issue that prevented toggle clicks
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 31, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:21040

This environment will automatically shut down when the PR is closed or after 5 hours.

@lucasbordeau lucasbordeau self-requested a review January 5, 2026 10:25
@charlesBochet
Copy link
Copy Markdown
Member

Thank you!

@charlesBochet charlesBochet added this pull request to the merge queue Jan 8, 2026
Merged via the queue into twentyhq:main with commit b27f2ca Jan 8, 2026
61 checks passed
@twenty-eng-sync
Copy link
Copy Markdown

Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

1 similar comment
@twenty-eng-sync
Copy link
Copy Markdown

Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

NotYen pushed a commit to NotYen/twenty-ym that referenced this pull request Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Graph small bugs

2 participants