Skip to content

fix(dropdown-menu): make submenu indicator icon and divider spacing themable #4667

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

Open
athz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(dropdown-menu): make submenu indicator icon and divider spacing themable #4667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions .changeset/dropdown-menu-theme-fixes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astryxdesign/core': patch
---

[fix] DropdownMenu: make the submenu indicator icon and menu divider spacing actually themable. The `astryx-dropdown-menu-indicator-icon` target now sits on the chevron glyph itself (not the wrapper span), so a theme can restyle its size; and the menu divider's vertical margin is exposed via `--_dropdown-menu-divider-margin` so it can be retuned without out-specifying the global divider slot.
@athz
2 changes: 2 additions & 0 deletions packages/core/src/DropdownMenu/DropdownMenu.doc.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ export const docs = {
vars: [
{name: '--_dropdown-menu-radius', description: 'Border radius of the menu popup', default: 'var(--radius-element)', private: true},
{name: '--_dropdown-menu-padding', description: 'Inner padding of the menu popup', default: 'var(--spacing-1)', private: true},
{name: '--_dropdown-menu-divider-margin', description: 'Vertical margin above and below a menu divider', default: 'var(--spacing-1)', private: true},
],
derived: [
{property: 'borderRadius', vars: ['--_dropdown-menu-radius']},
{property: 'padding', vars: ['--_dropdown-menu-padding']},
{property: 'marginBlock', vars: ['--_dropdown-menu-divider-margin']},

@cixzhang cixzhang Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm I think this communicates that margin block is converted to the internal bar for block margin around the divider which is kind of odd at the menu level

@cixzhang cixzhang Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we make divider more directly themeable?

],
},
description: 'Main dropdown menu component with a trigger button and popup item list.',
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/DropdownMenu/DropdownMenuSubMenu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,10 +671,13 @@ describe('DropdownMenuSubMenu theming slots', () => {
name: /Move to/,
hidden: true,
});
// The indicator-icon slot wraps the chevron affordance inside the trigger
// row.
expect(
trigger.querySelector('.astryx-dropdown-menu-indicator-icon'),
).toBeInTheDocument();
// The indicator-icon slot sits on the chevron glyph itself (the element
// that carries the icon size), so a theme can restyle its size/color
// directly — not on the wrapper span, which could not reach the size.
const indicator = trigger.querySelector(
'.astryx-dropdown-menu-indicator-icon',
);
expect(indicator).toBeInTheDocument();
expect(indicator).toHaveClass('astryx-icon');
});
});
19 changes: 8 additions & 11 deletions packages/core/src/DropdownMenu/DropdownMenuSubMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,20 +474,17 @@ export function DropdownMenuSubMenu(
);

const endAffordance = hasSpinner ? (
<span
{...mergeProps(
themeProps('dropdown-menu-indicator-icon'),
Comment thread
cixzhang marked this conversation as resolved.
stylex.props(triggerStyles.caret),
)}>
<span {...stylex.props(triggerStyles.caret)}>
<Spinner size="sm" />
</span>
) : (
<span
{...mergeProps(
themeProps('dropdown-menu-indicator-icon'),
stylex.props(triggerStyles.caret),
)}>
<Icon icon="chevronRight" size="sm" color="secondary" />
<span {...stylex.props(triggerStyles.caret)}>
<Icon
icon="chevronRight"
size="sm"
color="secondary"
{...themeProps('dropdown-menu-indicator-icon')}
Comment thread
cixzhang marked this conversation as resolved.
/>
</span>
);

Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/DropdownMenu/renderDropdownItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ const styles = stylex.create({
userSelect: 'none',
},
divider: {
marginBlock: spacingVars['--spacing-1'],
// Exposed as a themable var (default: the token) so a theme can retune the
// menu divider's vertical rhythm via `--_dropdown-menu-divider-margin`
// without out-specifying the global Divider slot. Matches the component's
// existing `--_dropdown-menu-*` private-var pattern.
marginBlock: `var(--_dropdown-menu-divider-margin, ${spacingVars['--spacing-1']})`,
},
});

Expand Down
Loading