-
-
Notifications
You must be signed in to change notification settings - Fork 250
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.
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
feat: add popupRender prop to support custom popup content #747
base: master
Are you sure you want to change the base?
Conversation
Walkthrough新增了关于自定义弹出层渲染的文档章节、示例代码及样式文件,同时在核心组件(Menu、SubMenu、MenuContext 和接口定义)中增加了对 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 用户
participant M as Menu组件
participant SM as 子菜单组件
participant PR as popupRender函数
participant D as 自定义渲染组件
U->>M: 鼠标悬停或点击菜单项
M->>SM: 通知显示子菜单
SM->>PR: 检查并调用 popupRender(若定义)
alt 使用自定义 popupRender
PR->>D: 返回自定义渲染内容
D-->>SM: 替换默认渲染
else 未使用自定义
SM-->>SM: 使用默认渲染
end
SM-->>M: 展示最终弹出层效果
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/SubMenu/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
tests/popupRender.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/interface.ts (1)
146-149
: 建议改进 PopupRender 类型定义
info
参数中的item
类型使用any
过于宽松,建议使用更具体的类型定义。建议如下修改:
export type PopupRender = ( node: React.ReactElement, - info: { item: any; keys: string[] }, + info: { item: SubMenuType | MenuItemType; keys: string[] }, ) => React.ReactNode;docs/examples/customPopupRender.tsx (2)
70-82
: 建议优化 popupRender 函数的性能当前实现在每次渲染时都会重新创建函数,建议使用
useCallback
来优化性能。建议如下修改:
+import { useCallback } from 'react'; const NavigationDemo = () => { - const popupRender = (node: ReactElement) => ( + const popupRender = useCallback((node: ReactElement) => ( <div className="navigation-popup"> <div className="navigation-grid"> {React.Children.map(node.props.children.props.children, child => ( <div className="navigation-item"> {React.cloneElement(child, { className: `${child.props.className || ''} navigation-menu-item`, })} </div> ))} </div> </div> - ); + ), []);
7-69
: 建议添加 menuItems 的类型定义为了提高代码的可维护性和类型安全性,建议为
menuItems
添加明确的类型定义。建议如下修改:
+import type { ItemType } from 'rc-menu'; const NavigationDemo = () => { - const menuItems = [ + const menuItems: ItemType[] = [ { key: 'home', label: 'Home', }, // ... ];tests/popupRender.test.tsx (1)
6-175
: 建议增加更多边缘测试用例当前的测试覆盖了基本功能,但建议添加以下场景的测试:
- 处理
popupRender
返回null
的情况- 测试动态切换
popupRender
函数- 测试错误处理场景
我可以帮助生成这些额外的测试用例,是否需要?
src/SubMenu/index.tsx (1)
281-303
: 建议优化 renderPopupContent 函数的实现当前实现逻辑清晰,但建议考虑以下优化:
- 添加对
mergedPopupRender
返回值的类型检查- 考虑添加错误边界处理
建议修改实现如下:
const mergedPopupRender = propsPopupRender || contextPopupRender; const renderPopupContent = () => { const triggerMode = triggerModeRef.current; const originNode = ( <MenuContextProvider mode={triggerMode === 'horizontal' ? 'vertical' : triggerMode} > <SubMenuList id={popupId} ref={popupRef}> {children} </SubMenuList> </MenuContextProvider> ); + try { if (mergedPopupRender) { - return mergedPopupRender(originNode, { + const customNode = mergedPopupRender(originNode, { item: props, keys: connectedPath, }); + return React.isValidElement(customNode) ? customNode : originNode; } return originNode; + } catch (error) { + console.error('Failed to render custom popup content:', error); + return originNode; + } };docs/examples/customPopupRender.less (2)
1-44
: 建议优化 navigation-popup 样式实现当前导航弹出层样式结构清晰,但建议考虑以下优化:
- 考虑添加媒体查询以支持响应式布局
- 建议使用 CSS 变量管理颜色和间距值
- 可以添加暗色主题支持
建议添加如下样式:
+:root { + --navigation-popup-padding: 24px; + --navigation-popup-bg: #fff; + --navigation-popup-shadow: 0 6px 16px rgba(0, 0, 0, 0.08); + --navigation-text-secondary: rgba(0, 0, 0, 0.45); +} +@media (max-width: 768px) { + .navigation-popup { + min-width: 100%; + + .navigation-grid { + grid-template-columns: 1fr; + } + } +} +@media (prefers-color-scheme: dark) { + :root { + --navigation-popup-bg: #1f1f1f; + --navigation-popup-shadow: 0 6px 16px rgba(0, 0, 0, 0.15); + --navigation-text-secondary: rgba(255, 255, 255, 0.45); + } +}
46-76
: 建议优化 panel-popup 样式实现面板弹出层样式实现简洁,但建议:
- 使用 CSS 变量统一管理样式值
- 添加过渡动画提升交互体验
- 考虑无障碍支持
建议添加如下样式:
+.panel-popup { + /* ... existing styles ... */ + + .panel-content { + .rc-menu-item { + /* ... existing styles ... */ + + transition: background-color 0.2s ease; + + &:focus-visible { + outline: 2px solid #1890ff; + outline-offset: -1px; + } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/demo/customPopupRender.md
(1 hunks)docs/examples/customPopupRender.less
(1 hunks)docs/examples/customPopupRender.tsx
(1 hunks)src/Menu.tsx
(4 hunks)src/SubMenu/index.tsx
(6 hunks)src/context/MenuContext.tsx
(2 hunks)src/interface.ts
(1 hunks)tests/popupRender.test.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/demo/customPopupRender.md
🔇 Additional comments (3)
src/context/MenuContext.tsx (1)
11-11
: 代码变更看起来不错!新增的
popupRender
属性和相关类型导入的位置恰当,保持了代码的整洁性和可维护性。Also applies to: 50-50
src/SubMenu/index.tsx (1)
7-7
: 新增 PopupRender 类型导入和接口定义
popupRender
属性的添加增强了 SubMenu 组件的自定义能力,使其能够支持自定义弹出层内容渲染。Also applies to: 39-39
src/Menu.tsx (1)
34-34
: 完善 Menu 组件的 PopupRender 支持
popupRender
属性的添加使 Menu 组件能够在全局层面支持自定义弹出层渲染,与 SubMenu 组件形成完整的自定义渲染体系。Also applies to: 163-163
node: ReactElement, | ||
info: { item: any; keys: string[] }, | ||
) => { | ||
const isSecondLevel = info.keys.length == 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
修复相等性比较
使用 ==
进行相等性比较不是最佳实践,建议使用 ===
。
建议如下修改:
- const isSecondLevel = info.keys.length == 2;
+ const isSecondLevel = info.keys.length === 2;
Also applies to: 114-114
src/SubMenu/index.tsx
Outdated
const mergedPopupRender = propsPopupRender || contextPopupRender; | ||
|
||
// renderPopupContent | ||
const renderPopupContent = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
看起来不用封装成一个函数,如果为了代码结构可以考虑直接改成 useMemo
tests/popupRender.test.tsx
Outdated
import type { ReactElement } from 'react'; | ||
|
||
describe('Menu PopupRender Tests', () => { | ||
// 基础测试数据 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
都用英文~
tests/popupRender.test.tsx
Outdated
</SubMenu> | ||
</Menu>, | ||
); | ||
fireEvent.mouseEnter(screen.getByText('Test')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不用这么复杂,直接 Menu 受控展开 key 就行了,我们只需要测 render,交互不用测
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #747 +/- ##
=======================================
Coverage 99.59% 99.59%
=======================================
Files 27 27
Lines 738 748 +10
Branches 202 206 +4
=======================================
+ Hits 735 745 +10
Misses 3 3 ☔ View full report in Codecov by Sentry. |
This implements the RFC for customizable SubMenu popup content.
Summary by CodeRabbit