Skip to content

Separator for dropdown , context and menubar menus #1588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 20, 2025
Merged

Separator for dropdown , context and menubar menus #1588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0f56066
add: separator
GoldGroove06 Sep 15, 2025
955bc2a
add: forward ref to comps
GoldGroove06 Sep 16, 2025
74f4538
add: styles
GoldGroove06 Sep 16, 2025
9c02fa1
add: changeset
GoldGroove06 Sep 16, 2025
7a5e9e0
fix: coverage workflow
GoldGroove06 Sep 16, 2025
768f14a
revert: coverage workflow
GoldGroove06 Sep 16, 2025
109253e
fix: props
GoldGroove06 Sep 16, 2025
1262db5
fix: customRootClass
GoldGroove06 Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-times-switch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@radui/ui": patch
---

Added Separator for dropdown , context and menubar menus
3 changes: 3 additions & 0 deletions src/components/ui/ContextMenu/ContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import ContextMenuPortal from './fragments/ContextMenuPortal';
import ContextMenuItem from './fragments/ContextMenuItem';
import ContextMenuSub from './fragments/ContextMenuSub';
import ContextMenuSubTrigger from './fragments/ContextMenuSubTrigger';
import ContextMenuSeparator from './fragments/ContextMenuSeparator';

export type ContextMenuElement = ElementRef<'div'>;
export type ContextMenuProps = ComponentPropsWithoutRef<'div'>;
Expand All @@ -18,6 +19,7 @@ type ContextMenuComponent = React.ForwardRefExoticComponent<ContextMenuProps & R
Item: typeof ContextMenuItem;
Sub: typeof ContextMenuSub;
SubTrigger: typeof ContextMenuSubTrigger;
Separator: typeof ContextMenuSeparator;
};

const ContextMenu = forwardRef<ContextMenuElement, ContextMenuProps>((_props, _ref) => {
Expand All @@ -34,5 +36,6 @@ ContextMenu.Portal = ContextMenuPortal;
ContextMenu.Item = ContextMenuItem;
ContextMenu.Sub = ContextMenuSub;
ContextMenu.SubTrigger = ContextMenuSubTrigger;
ContextMenu.Separator = ContextMenuSeparator;

export default ContextMenu;
23 changes: 23 additions & 0 deletions src/components/ui/ContextMenu/fragments/ContextMenuSeparator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import React, { forwardRef, ElementRef, ComponentPropsWithoutRef } from 'react';
import Separator from '../../Separator/Separator';
import ContextMenuContext from '../contexts/ContextMenuContext';

export type ContextMenuSeparatorElement = ElementRef<typeof Separator>;
export type ContextMenuSeparatorProps = {
className?: string
}& ComponentPropsWithoutRef<typeof Separator>;
const ContextMenuSeparator = forwardRef<ContextMenuSeparatorElement, ContextMenuSeparatorProps>(({ className, ...props }: ContextMenuSeparatorProps, ref) => {
const context = React.useContext(ContextMenuContext);
if (!context) {
console.warn('ContextMenuSeparator should be used within ContextMenuRoot');
return null;
}
const { rootClass } = context;
return (
<Separator ref={ref} customRootClass={rootClass} className={className} {...props}/>
);
});

ContextMenuSeparator.displayName = 'ContextMenuSeparator';

export default ContextMenuSeparator;
1 change: 1 addition & 0 deletions src/components/ui/ContextMenu/stories/ContextMenu.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const Basic: Story = {
<ContextMenu.Item label="Profile">Profile</ContextMenu.Item>
<ContextMenu.Item label="Settings">Settings</ContextMenu.Item>
<ContextMenu.Item label="Notifications">Notifications</ContextMenu.Item>
<ContextMenu.Separator/>
<ContextMenu.Sub >
<ContextMenu.SubTrigger >More Options <ChevronRight /></ContextMenu.SubTrigger>
<ContextMenu.Content >
Expand Down
3 changes: 3 additions & 0 deletions src/components/ui/DropdownMenu/DropdownMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import DropdownMenuPortal from './fragments/DropdownMenuPortal';
import DropdownMenuItem from './fragments/DropdownMenuItem';
import DropdownMenuSub from './fragments/DropdownMenuSub';
import DropdownMenuSubTrigger from './fragments/DropdownMenuSubTrigger';
import DropdownMenuSeparator from './fragments/DropdownMenuSeparator';

export type DropdownMenuElement = ElementRef<'div'>;
export type DropdownMenuProps = ComponentPropsWithoutRef<'div'>;
Expand All @@ -18,6 +19,7 @@ type DropdownMenuComponent = React.ForwardRefExoticComponent<DropdownMenuProps &
Item: typeof DropdownMenuItem;
Sub: typeof DropdownMenuSub;
SubTrigger: typeof DropdownMenuSubTrigger;
Separator : typeof DropdownMenuSeparator;
};

const DropdownMenu = forwardRef<DropdownMenuElement, DropdownMenuProps>((_props, _ref) => {
Expand All @@ -34,5 +36,6 @@ DropdownMenu.Portal = DropdownMenuPortal;
DropdownMenu.Item = DropdownMenuItem;
DropdownMenu.Sub = DropdownMenuSub;
DropdownMenu.SubTrigger = DropdownMenuSubTrigger;
DropdownMenu.Separator = DropdownMenuSeparator;

export default DropdownMenu;
24 changes: 24 additions & 0 deletions src/components/ui/DropdownMenu/fragments/DropdownMenuSeparator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React, { forwardRef, ElementRef, ComponentPropsWithoutRef } from 'react';
import Separator from '../../Separator/Separator';
import DropdownMenuContext from '../contexts/DropdownMenuContext';

export type DropdownMenuSeparatorElement = ElementRef<typeof Separator>;
export type DropdownMenuSeparatorProps = {
className?: string
} & ComponentPropsWithoutRef<typeof Separator>;

const DropdownMenuSeparator = forwardRef<DropdownMenuSeparatorElement, DropdownMenuSeparatorProps>(({ className, ...props }: DropdownMenuSeparatorProps, ref) => {
const context = React.useContext(DropdownMenuContext);
if (!context) {
console.warn('DropdownMenuSeparator should be used within DropdownMenuRoot');
return null;
}
const { rootClass } = context;
return (
<Separator ref={ref} customRootClass={rootClass} className={className} {...props}/>
);
});

DropdownMenuSeparator.displayName = 'DropdownMenuSeparator';

export default DropdownMenuSeparator;
1 change: 1 addition & 0 deletions src/components/ui/DropdownMenu/stories/DropdownMenu.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const Basic: Story = {
<DropdownMenu.Item label="Profile">Profile</DropdownMenu.Item>
<DropdownMenu.Item label="Settings">Settings</DropdownMenu.Item>
<DropdownMenu.Item label="Notifications">Notifications</DropdownMenu.Item>
<DropdownMenu.Separator/>
<DropdownMenu.Sub >
<DropdownMenu.SubTrigger >More Options <ChevronRight /></DropdownMenu.SubTrigger>
<DropdownMenu.Content >
Expand Down
2 changes: 2 additions & 0 deletions src/components/ui/Menubar/Menubar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import MenubarItem from './fragments/MenubarItem';
import MenubarSub from './fragments/MenubarSub';
import MenubarSubTrigger from './fragments/MenubarSubTrigger';
import MenubarMenu from './fragments/MenubarMenu';
import MenubarSeparator from './fragments/MenubarSeparator';

const Menubar = () => {
console.warn('Direct usage of Menubar is not supported. Please use Menubar.Root, Menubar.Item instead.');
Expand All @@ -20,5 +21,6 @@ Menubar.Portal = MenubarPortal;
Menubar.Item = MenubarItem;
Menubar.Sub = MenubarSub;
Menubar.SubTrigger = MenubarSubTrigger;
Menubar.Separator = MenubarSeparator;

export default Menubar;
26 changes: 26 additions & 0 deletions src/components/ui/Menubar/fragments/MenubarSeparator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React, { forwardRef, ElementRef, ComponentPropsWithoutRef } from 'react';
import Separator from '../../Separator/Separator';
import MenubarContext from '../contexts/MenubarContext';
import clsx from 'clsx';

export type MenubarSeparatorElement = ElementRef<typeof Separator>;
export type MenubarSeparatorProps = {
className?: string
} & ComponentPropsWithoutRef<typeof Separator>;
Comment on lines +7 to +9
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Confirm Separator’s styling prop; avoid prop duplication.

  • If Separator expects className (common), pass className and drop customRootClass.
  • If Separator indeed uses customRootClass, keep as-is, but ensure Separator does not also expose className to avoid type duplication/confusion.

Proposed change (if Separator expects className):

-export type MenubarSeparatorProps = {
-    className?: string
-} & ComponentPropsWithoutRef<typeof Separator>;
+export type MenubarSeparatorProps = ComponentPropsWithoutRef<typeof Separator>;
...
-        <Separator ref={ref} customRootClass={clsx(`${rootClass}`, className)} {...props}/>
+        <Separator ref={ref} className={clsx(rootClass, className)} {...props}/>

Run to verify Separator’s API:

Also applies to: 19-20

🏁 Script executed:

#!/bin/bash
# Locate Separator implementation(s)
fd -t f -a --glob "src/**/Separator*.{ts,tsx}"

# Inspect props for styling prop names
rg -n -C2 -P '\b(customRootClass|className)\b' src | sed -n '1,200p'

# Confirm default export form to match import style
rg -n -C2 -P 'export\s+default\s+(function\s+Separator|Separator)\b' src

Length of output: 17230

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- Separator.tsx ----"
nl -ba src/components/ui/Separator/Separator.tsx | sed -n '1,240p' || true

echo
echo "---- MenubarSeparator.tsx ----"
nl -ba src/components/ui/Menubar/fragments/MenubarSeparator.tsx | sed -n '1,240p' || true

Length of output: 137

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- Separator directory ----"
ls -la src/components/ui/Separator || true
echo

echo "---- Separator.tsx (first 240 lines) ----"
sed -n '1,240p' src/components/ui/Separator/Separator.tsx || true
echo

echo "---- MenubarSeparator.tsx (first 240 lines) ----"
sed -n '1,240p' src/components/ui/Menubar/fragments/MenubarSeparator.tsx || true
echo

echo "---- Search for styling props in Separator files ----"
rg -n --hidden --no-heading -S '\b(customRootClass|className)\b' src/components/ui/Separator || true

Length of output: 11543

Pass Menubar root via customRootClass and forward consumer className separately

  • Separator exposes both customRootClass and className (see src/components/ui/Separator/Separator.tsx) — MenubarSeparator currently merges consumer className into customRootClass: customRootClass={clsx(${rootClass}, className)}. That pushes consumer classes into the root-prefix/orientation logic instead of appending them as additional classes.
  • Change (src/components/ui/Menubar/fragments/MenubarSeparator.tsx): remove the explicit className?: string from MenubarSeparatorProps (it’s already in ComponentPropsWithoutRef) and render as:
    <Separator ref={ref} customRootClass={rootClass} className={className} {...props} />
  • Rationale: keep root prefix routing via customRootClass (used by customClassSwitcher/orientation) and ensure consumer className is appended predictably.
🤖 Prompt for AI Agents 
In src/components/ui/Menubar/fragments/MenubarSeparator.tsx around lines 7 to 9,
MenubarSeparator currently defines className?: string in its props and merges
the consumer className into customRootClass
(customRootClass={clsx(`${rootClass}`, className)}), which incorrectly injects
consumer classes into the root-prefix/orientation logic; remove the explicit
className?: string from MenubarSeparatorProps (it already comes from
ComponentPropsWithoutRef<typeof Separator>) and change the render to pass
customRootClass={rootClass} and className={className} (forward ref unchanged),
i.e., stop merging className into customRootClass and forward consumer className
separately along with {...props}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

explain it more with some example

Copy link
Contributor

Choose a reason for hiding this comment

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

@GoldGroove06, let me explain the issue with a detailed example showing why separating customRootClass and className is important:

Current Problematic Implementation

// Current MenubarSeparator.tsx
export type MenubarSeparatorProps = {
    className?: string  // ❌ Explicitly defining className (redundant)
} & ComponentPropsWithoutRef<typeof Separator>;

// In the render:
<Separator 
    ref={ref} 
    customRootClass={clsx(`${rootClass}`, className)}  // ❌ Merging everything into customRootClass
    {...props}
/>

What Happens Currently

Let's say:

  • rootClass from context = "rad-ui-menubar-separator"
  • User passes className="my-custom-spacing"

Current result:

<Separator customRootClass="rad-ui-menubar-separator my-custom-spacing" />

The Problem

The Separator component uses customRootClass for internal theming logic (like orientation detection, theme switching). When you merge user classes into it:

  1. Theme conflicts: User classes might interfere with internal theme logic
  2. Orientation issues: If Separator checks customRootClass for orientation keywords, user classes could break this
  3. Specificity problems: User styles mixed with system styles in the same prop

Recommended Implementation

// Fixed MenubarSeparator.tsx
export type MenubarSeparatorProps = 
    ComponentPropsWithoutRef<typeof Separator>; // ✅ No explicit className (already included)

// In the render:
<Separator 
    ref={ref} 
    customRootClass={rootClass}        // ✅ System/theme classes only
    className={className}              // ✅ User classes separate
    {...props}
/>

What Should Happen

Same inputs:

  • rootClass = "rad-ui-menubar-separator"
  • User passes className="my-custom-spacing"

Fixed result:

<Separator 
    customRootClass="rad-ui-menubar-separator"  // System theming
    className="my-custom-spacing"               // User customization
/>

Why This Separation Matters

// Example: User wants custom spacing
<Menubar.Separator className="mx-4 my-2" />

// Current (problematic): Everything mixed together
// customRootClass="rad-ui-menubar-separator mx-4 my-2"

// Fixed: Clean separation
// customRootClass="rad-ui-menubar-separator" 
// className="mx-4 my-2"

The Separator component can then:

  • Use customRootClass for internal theme/orientation logic
  • Apply className as additional user styles
  • Maintain proper CSS specificity and avoid conflicts

This same pattern should be applied to DropdownMenuSeparator and ContextMenuSeparator as well for consistency.


const MenubarSeparator = forwardRef<MenubarSeparatorElement, MenubarSeparatorProps>(({ className, ...props }: MenubarSeparatorProps, ref) => {
const context = React.useContext(MenubarContext);
if (!context) {
console.warn('MenubarSeparator should be used within MenubarRoot');
return null;
}
const { rootClass } = context;

return (
<Separator ref={ref} customRootClass={rootClass} className={className} {...props}/>
);
});

MenubarSeparator.displayName = 'MenubarSeparator';

export default MenubarSeparator;
1 change: 1 addition & 0 deletions src/components/ui/Menubar/stories/Menubar.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const SampleMenu = () => (
<Menubar.Item label="Profile">Profile</Menubar.Item>
<Menubar.Item label="Settings">Settings</Menubar.Item>
<Menubar.Item label="Notifications">Notifications</Menubar.Item>
<Menubar.Separator/>
<Menubar.Sub >
<Menubar.SubTrigger >More Options <ChevronRight /></Menubar.SubTrigger>
<Menubar.Content >
Expand Down
17 changes: 17 additions & 0 deletions styles/themes/components/context-menu.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,22 @@ width:max-content;
}
}

.rad-ui-context-menu-separator {
background-color: var(--rad-ui-color-gray-600);
align-self: stretch;
}

.rad-ui-context-menu-separator[data-rad-ui-accent-color] {
background-color: var(--rad-ui-color-accent-900);
}

.rad-ui-context-menu-separator-vertical{
margin: 0 8px;
width: 1px;
}

.rad-ui-context-menu-separator-horizontal{
margin: 8px 0;
height: 1px;
}

19 changes: 19 additions & 0 deletions styles/themes/components/dropdown-menu.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,24 @@ width:max-content;
}
}

.rad-ui-dropdown-menu-separator {
background-color: var(--rad-ui-color-gray-600);
align-self: stretch;
}

.rad-ui-dropdown-menu-separator[data-rad-ui-accent-color] {
background-color: var(--rad-ui-color-accent-900);
}

.rad-ui-dropdown-menu-separator-vertical{
margin: 0 8px;
width: 1px;
}

.rad-ui-dropdown-menu-separator-horizontal{
margin: 8px 0;
height: 1px;
}



17 changes: 17 additions & 0 deletions styles/themes/components/menubar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,22 @@ width:max-content;
}
}

.rad-ui-menubar-separator {
background-color: var(--rad-ui-color-gray-600);
align-self: stretch;
}

.rad-ui-menubar-separator[data-rad-ui-accent-color] {
background-color: var(--rad-ui-color-accent-900);
}

.rad-ui-menubar-separator-vertical{
margin: 0 8px;
width: 1px;
}

.rad-ui-menubar-separator-horizontal{
margin: 8px 0;
height: 1px;
}

Loading