feat(list): add Spin overlay for loading and use Empty.PRESENTED_IMAGE_SIMPLE#9511
feat(list): add Spin overlay for loading and use Empty.PRESENTED_IMAGE_SIMPLE#9511chenshuai2144 merged 1 commit intoant-design:masterfrom
Conversation
Summary of ChangesHello @leshalv, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience of lists by introducing a loading spinner that activates when data is being fetched, and by standardizing the empty state display with a simpler, predefined image. These changes provide clearer visual feedback to users, improving the perceived performance and consistency of list components. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughProListBase.tsx 的列表加载与空状态 UI 得以优化。新增 Spin 组件用于展示加载状态,并将空状态显示改为简洁的图像样式,进一步提升用户体验。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a <Spin> overlay for loading states and updates the empty state component to use Empty.PRESENTED_IMAGE_SIMPLE. The changes are a good improvement for user experience. However, I've identified a couple of related issues in ProListBase.tsx where the new <Spin> component doesn't behave as a true overlay due to existing logic, and there's a latent bug that can cause children to be rendered twice. My review comment provides more details on these points.
| <Spin spinning={isLoading}> | ||
| {showPaginationTop && paginationNode} | ||
| {header && <div className={`${prefixCls}-header`}>{header}</div>} | ||
| {childrenContent} | ||
| {children} | ||
| {footer && <div className={`${prefixCls}-footer`}>{footer}</div>} | ||
| {loadMore} | ||
| {showPaginationBottom && paginationNode} | ||
| </Spin> |
There was a problem hiding this comment.
This change to use <Spin> for loading is a good one. However, it interacts with some existing logic in ways that might be unintended. There are two issues to consider:
-
Loading overlay behavior: The spinner won't function as a true overlay on stale data. This is because when
isLoadingis true, the list's content (childrenContent) is replaced by a placeholderdiv(see lines 453-454). This causes the list to appear empty during loading, rather than showing the spinner over the old content. For a better user experience, you might want to remove that logic so the spinner overlays the stale data. -
Duplicate rendering of children: There's a pre-existing bug where
childrencan be rendered twice. This happens whendataSourceis empty butchildrenare provided. The logic at line 456 setschildrenContent = children, and then both are rendered within the<Spin>component, causing duplication. This should be addressed by refactoring thechildrenContentlogic.
Both issues are related to how childrenContent is determined outside of this changed block. I'd recommend addressing them for this feature to work correctly and to fix the underlying bug.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/list/ProListBase.tsx (1)
448-448: 确认是否需要保留空状态描述文本。原代码可能包含
description="暂无数据"的自定义描述,现在改为使用Empty.PRESENTED_IMAGE_SIMPLE后未设置description,将显示 antd 默认的 "No Data" 文本。如果需要保持中文描述或依赖ConfigProvider的国际化配置,请确认此行为符合预期。💡 如需保留描述文本的可选方案
- <Empty image={Empty.PRESENTED_IMAGE_SIMPLE} /> + <Empty image={Empty.PRESENTED_IMAGE_SIMPLE} description="暂无数据" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/list/ProListBase.tsx` at line 448, The Empty component usage in ProListBase.tsx was changed to use Empty.PRESENTED_IMAGE_SIMPLE but removed the description, which will show Antd's default "No Data" text; decide whether to restore the original Chinese description or rely on ConfigProvider i18n and, if restoring, add a description prop back to the Empty element (e.g., description="暂无数据") or wire it to your localization helper/ConfigProvider so the Empty in ProListBase renders the intended localized text instead of the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/list/ProListBase.tsx`:
- Around line 511-519: The code currently renders children twice because when
splitDataSource is empty you assign childrenContent = children and then render
both {childrenContent} and {children}; remove the redundant standalone
{children} render in the JSX inside the Spin so only {childrenContent} is
rendered (ensure childrenContent logic still falls back to children when
appropriate), referencing childrenContent, children, splitDataSource and
isLoading to locate and verify the fix.
---
Nitpick comments:
In `@src/list/ProListBase.tsx`:
- Line 448: The Empty component usage in ProListBase.tsx was changed to use
Empty.PRESENTED_IMAGE_SIMPLE but removed the description, which will show Antd's
default "No Data" text; decide whether to restore the original Chinese
description or rely on ConfigProvider i18n and, if restoring, add a description
prop back to the Empty element (e.g., description="暂无数据") or wire it to your
localization helper/ConfigProvider so the Empty in ProListBase renders the
intended localized text instead of the default.
| <Spin spinning={isLoading}> | ||
| {showPaginationTop && paginationNode} | ||
| {header && <div className={`${prefixCls}-header`}>{header}</div>} | ||
| {childrenContent} | ||
| {children} | ||
| {footer && <div className={`${prefixCls}-footer`}>{footer}</div>} | ||
| {loadMore} | ||
| {showPaginationBottom && paginationNode} | ||
| </Spin> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/list/ProListBase.tsx | sed -n '450,520p'Repository: ant-design/pro-components
Length of output: 2909
🏁 Script executed:
cat -n src/list/ProListBase.tsx | sed -n '430,460p'Repository: ant-design/pro-components
Length of output: 1286
🏁 Script executed:
cat -n src/list/ProListBase.tsx | sed -n '505,525p' && echo "---" && rg -n "children" src/list/ProListBase.tsx | grep -E "(line 515|{children})" | head -20Repository: ant-design/pro-components
Length of output: 991
🏁 Script executed:
cat -n src/list/ProListBase.tsx | sed -n '155,175p'Repository: ant-design/pro-components
Length of output: 782
🏁 Script executed:
# Check for any comments around line 515 or in the broader render section
cat -n src/list/ProListBase.tsx | sed -n '500,525p' | grep -i "comment\|todo\|fixme\|hack\|bug" || echo "No comments found in render section"
# Also check the git history of this line to see if there's context
git log --oneline -p src/list/ProListBase.tsx 2>/dev/null | grep -A10 -B10 "children}" | head -50 || echo "Git history not available"Repository: ant-design/pro-components
Length of output: 1534
在特定条件下 children 被重复渲染。
当 splitDataSource 为空、存在 children 且非加载状态时,第 456 行将 childrenContent 赋值为 children。此时第 514 行 {childrenContent} 和第 515 行 {children} 会导致 children 被渲染两次。此外,第 515 行的独立渲染在其他条件分支中也显得冗余。
🐛 建议移除重复的 children 渲染
<Spin spinning={isLoading}>
{showPaginationTop && paginationNode}
{header && <div className={`${prefixCls}-header`}>{header}</div>}
{childrenContent}
- {children}
{footer && <div className={`${prefixCls}-footer`}>{footer}</div>}
{loadMore}
{showPaginationBottom && paginationNode}
</Spin>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Spin spinning={isLoading}> | |
| {showPaginationTop && paginationNode} | |
| {header && <div className={`${prefixCls}-header`}>{header}</div>} | |
| {childrenContent} | |
| {children} | |
| {footer && <div className={`${prefixCls}-footer`}>{footer}</div>} | |
| {loadMore} | |
| {showPaginationBottom && paginationNode} | |
| </Spin> | |
| <Spin spinning={isLoading}> | |
| {showPaginationTop && paginationNode} | |
| {header && <div className={`${prefixCls}-header`}>{header}</div>} | |
| {childrenContent} | |
| {footer && <div className={`${prefixCls}-footer`}>{footer}</div>} | |
| {loadMore} | |
| {showPaginationBottom && paginationNode} | |
| </Spin> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/list/ProListBase.tsx` around lines 511 - 519, The code currently renders
children twice because when splitDataSource is empty you assign childrenContent
= children and then render both {childrenContent} and {children}; remove the
redundant standalone {children} render in the JSX inside the Spin so only
{childrenContent} is rendered (ensure childrenContent logic still falls back to
children when appropriate), referencing childrenContent, children,
splitDataSource and isLoading to locate and verify the fix.
feat(list): add Spin overlay for loading and use Empty.PRESENTED_IMAGE_SIMPLE
Summary by CodeRabbit
更新说明