Skip to content

fix: wrap item label when extra exists #863

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
QDyanbing wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −5
Open

fix: wrap item label when extra exists #863

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
12 changes: 9 additions & 3 deletions src/utils/nodeUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,17 @@ function convertItemsToNodes(
return <MergedDivider key={mergedKey} {...restProps} />;
}

const hasExtra = !!extra || extra === 0;

return (
<MergedMenuItem key={mergedKey} {...restProps} extra={extra}>
{label}
{(!!extra || extra === 0) && (
<span className={`${prefixCls}-item-extra`}>{extra}</span>
{hasExtra ? (
<>
<span className={`${prefixCls}-item-label`}>{label}</span>
<span className={`${prefixCls}-item-extra`}>{extra}</span>
</>
) : (
label
)}
Comment on lines +51 to 62
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

不要用 truthy 判断 extra 是否存在。

extra 的类型是 ReactNode，'' 之类的合法值会被当成“没有 extra”，从而跳过 item-label / item-extra 的包装，布局会和“已传入 extra”时不一致。建议改成按是否传入来判断，而不是按布尔值判断。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/utils/nodeUtil.tsx` around lines 51 - 62, The current truthy check (const
hasExtra = !!extra || extra === 0) treats valid ReactNode values like '' or 0
inconsistently; change the presence check to detect whether the prop was
provided instead of its truthiness (e.g., use extra !== undefined or check props
hasOwnProperty('extra')) so that hasExtra correctly reflects passed-in values
and the MergedMenuItem rendering (prefixCls-item-label / prefixCls-item-extra
wrapping around label) is applied whenever extra was supplied.

</MergedMenuItem>
);
Comment on lines +51 to 64
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

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

medium

The logic for wrapping the label and rendering the extra content is currently implemented in nodeUtil.tsx, which only affects items created via the items prop. This leads to inconsistent behavior when MenuItem is used directly via JSX (e.g., <MenuItem extra="⌘B">Menu Item</MenuItem>), as MenuItem.tsx currently ignores the extra prop and doesn't apply the wrapper.

Additionally, prefixCls is optional in nodeUtil.tsx, which could lead to undefined-item-label class names if it's missing.

Recommendation: Move this rendering logic into InternalMenuItem within src/MenuItem.tsx. This ensures a consistent DOM structure for both items prop and JSX usage, and allows safe access to prefixCls from MenuContext.

To implement this:

  1. Update src/MenuItem.tsx to handle the extra prop and wrap the children/label.
  2. Stop omitting extra from restProps in src/MenuItem.tsx (line 209).
  3. Simplify nodeUtil.tsx to just pass the label and extra prop as shown in the suggestion.
        return (
          <MergedMenuItem key={mergedKey} {...restProps} extra={extra}>
            {label}
          </MergedMenuItem>
        );

Expand Down
12 changes: 10 additions & 2 deletions tests/__snapshots__/MenuItem.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ exports[`MenuItem overwrite default role should set extra to group option 1`] =
role="menuitem"
tabindex="-1"
>
Menu Item 1
<span
class="rc-menu-item-label"
>
Menu Item 1
</span>
<span
class="rc-menu-item-extra"
>
Expand All @@ -40,7 +44,11 @@ exports[`MenuItem overwrite default role should set extra to option 1`] = `
role="menuitem"
tabindex="-1"
>
Top Menu Item
<span
class="rc-menu-item-label"
>
Top Menu Item
</span>
<span
class="rc-menu-item-extra"
>
Expand Down
Loading