Skip to content

Commit b27f2ca

Browse files
fix(graph): fix toggle click and multiple dropdown issues (#16874)
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](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/command-menu/pages/page-layout/components/chart-settings/ChartSettingItem.tsx) Added `closeAnyOpenDropdown()` before opening a new dropdown. This follows the existing pattern used in: - [useCommandMenu.ts](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts) - [RecordBoardDragSelect.tsx](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoardDragSelect.tsx) - [useRecordGroupReorderConfirmationModal.ts](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorderConfirmationModal.ts) --- ## Fix 2: Toggle switches not clickable **File:** [MenuItemToggle.tsx](https://github.com/twentyhq/twenty/blob/main/packages/twenty-ui/src/navigation/menu/menu-item/components/MenuItemToggle.tsx) (twenty-ui) ### Investigation I traced the toggle click flow and compared working vs non-working usages: - ✅ [SettingsRolesList.tsx](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/settings/roles/components/SettingsRolesList.tsx) - toggles work (MenuItemToggle directly in Dropdown) - ✅ [ObjectOptionsDropdownRecordGroupsContent.tsx](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownRecordGroupsContent.tsx) - toggles work (MenuItemToggle in SelectableListItem) - ❌ [ChartSettingItem.tsx](https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/command-menu/pages/page-layout/components/chart-settings/ChartSettingItem.tsx) - toggles don't work (CommandMenuItemToggle in SelectableListItem) The component structure and props were identical, so I examined the HTML output. ### Root Cause Found **nested `<label>` elements**: ```html <!-- 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`: ```diff - const StyledToggleContainer = styled.label` + const StyledToggleContainer = styled.div` ``` The [Toggle](https://github.com/twentyhq/twenty/blob/main/packages/twenty-ui/src/input/components/Toggle.tsx) 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](https://github.com/twentyhq/twenty/blob/main/packages/twenty-ui/src/navigation/menu/menu-item/components/MenuItemToggle.tsx) API - All existing usages of [MenuItemToggle](https://github.com/twentyhq/twenty/blob/main/packages/twenty-ui/src/navigation/menu/menu-item/components/MenuItemToggle.tsx) should continue working (and potentially work better) --------- Co-authored-by: Charles Bochet <charles@twenty.com>
1 parent 7ce7627 commit b27f2ca

File tree

2 files changed

+5
-5
lines changed

2 files changed

+5
-5
lines changed

packages/twenty-front/src/modules/command-menu/pages/page-layout/components/chart-settings/ChartSettingItem.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { type ChartSettingsItem } from '@/command-menu/pages/page-layout/types/C
1616
import { isMinMaxRangeValid } from '@/command-menu/pages/page-layout/utils/isMinMaxRangeValid';
1717
import { CommandMenuPages } from '@/command-menu/types/CommandMenuPages';
1818
import { DropdownContent } from '@/ui/layout/dropdown/components/DropdownContent';
19+
import { useCloseAnyOpenDropdown } from '@/ui/layout/dropdown/hooks/useCloseAnyOpenDropdown';
1920
import { useOpenDropdown } from '@/ui/layout/dropdown/hooks/useOpenDropdown';
2021
import { SelectableListItem } from '@/ui/layout/selectable-list/components/SelectableListItem';
2122
import { useSelectableList } from '@/ui/layout/selectable-list/hooks/useSelectableList';
@@ -35,6 +36,7 @@ export const ChartSettingItem = ({
3536
configuration,
3637
}: ChartSettingItemProps) => {
3738
const { pageLayoutId } = usePageLayoutIdFromContextStoreTargetedRecord();
39+
const { closeAnyOpenDropdown } = useCloseAnyOpenDropdown();
3840
const { openDropdown } = useOpenDropdown();
3941
const { setSelectedItemId } = useSelectableList(
4042
COMMAND_MENU_LIST_SELECTABLE_LIST_ID,
@@ -71,6 +73,7 @@ export const ChartSettingItem = ({
7173
};
7274

7375
const handleDropdownOpen = () => {
76+
closeAnyOpenDropdown();
7477
openDropdown({
7578
dropdownComponentInstanceIdFromProps: item.id,
7679
});

packages/twenty-ui/src/navigation/menu/menu-item/components/MenuItemToggle.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
import styled from '@emotion/styled';
22
import { type IconComponent } from '@ui/display';
33
import { Toggle, type ToggleSize } from '@ui/input';
4-
import { useId } from 'react';
54
import { MenuItemLeftContent } from '../internals/components/MenuItemLeftContent';
65
import {
76
StyledMenuItemBase,
87
StyledMenuItemRightContent,
98
} from '../internals/components/StyledMenuItemBase';
109

11-
const StyledToggleContainer = styled.label`
10+
const StyledToggleContainer = styled.div`
1211
align-items: center;
1312
cursor: pointer;
1413
display: flex;
@@ -39,14 +38,13 @@ export const MenuItemToggle = ({
3938
toggleSize,
4039
disabled = false,
4140
}: MenuItemToggleProps) => {
42-
const instanceId = useId();
4341
return (
4442
<StyledMenuItemBase
4543
className={className}
4644
focused={focused}
4745
disabled={disabled}
4846
>
49-
<StyledToggleContainer htmlFor={instanceId}>
47+
<StyledToggleContainer>
5048
<MenuItemLeftContent
5149
LeftIcon={LeftIcon}
5250
text={text}
@@ -55,7 +53,6 @@ export const MenuItemToggle = ({
5553
/>
5654
<StyledMenuItemRightContent>
5755
<Toggle
58-
id={instanceId}
5956
value={toggled}
6057
onChange={disabled ? undefined : onToggleChange}
6158
toggleSize={toggleSize}

0 commit comments

Comments
 (0)