-
Notifications
You must be signed in to change notification settings - Fork 27
feat: 优化 Loading 组件动画效果 #214
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
base: main
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthrough新增基于 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant LoadingEntry as Loading 入口组件
participant Lottie as Lottie 组件
participant EffectComp as Effect 组件 (CreativeRecommendationEffect)
participant Player as EffectPlayer
App->>LoadingEntry: render({ type, size, autoplay, loop })
LoadingEntry->>LoadingEntry: 根据 type 分发
alt type == "spark"
LoadingEntry->>Lottie: render CreativeSparkLottie(size,autoplay,loop)
Lottie-->>App: DOM 渲染动画
else type == "recommendation"
LoadingEntry->>EffectComp: render CreativeRecommendationEffect(size,autoplay,loop)
EffectComp->>Player: mount Player(sceneUrl, autoplay)
Player-->>App: 渲染场景或显示降级图像
else
LoadingEntry->>Lottie: render LoadingLottie(size,autoplay,loop)
Lottie-->>App: DOM 渲染默认 Loading
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Summary of ChangesHello @nooooooom, 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! 此拉取请求旨在全面优化 Loading 组件的动画效果,通过引入新的动画播放器和多种动画类型,极大地提升了组件的灵活性和视觉表现力。现在,开发者可以根据不同的应用场景,轻松选择和配置多样化的加载动画,从而为用户提供更具吸引力和反馈感的界面体验。 Highlights
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
|
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.
Code Review
此次 Pull Request 通过引入新的动画类型(spark、recommendation)和重构底层实现,成功地增强了 Loading 组件的功能。为支持 @galacean/effects 而新增的 EffectPlayer 组件以及新的 Lottie 动画组件都实现得很好。文档和演示也相应地得到了更新,这一点值得称赞。
我发现了一些可以改进的地方:
EffectPlayer中的loop功能尚未实现,这与接口定义不符,需要补充完成。- 部分新文件中包含了应移除的 BOM 字符。
- 一个用于调整样式的“魔法值”可以添加注释以提高代码清晰度。
- 文档中有一处重复内容需要修正。
总体而言,这是一次出色的贡献,极大地提升了组件的灵活性和视觉效果。
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 (8)
docs/components/loading.md (2)
11-12: 实现已改为多种动画管线,文档对底层技术栈的表述建议再核对当前介绍仍然写着“基于 Framer Motion 实现展示流畅的加载动画效果”,而代码层面已经通过
LoadingLottie、CreativeSparkLottie以及CreativeRecommendationEffect(基于 EffectPlayer)来渲染动画。建议确认一下 Loading 现有实现的真实依赖栈,如果已经从 Framer Motion 迁移到 Lottie / Effect 方案,文档这里可以改成更中性的“加载动画组件”或更新为新的技术栈描述,避免给使用者造成误解。Also applies to: 40-45
40-45: 特性与使用说明存在“灵活尺寸”描述重复,中文语感也可微调
- “特性”里有两条「📏 灵活尺寸」,其中第 42 行和第 44 行意义高度重叠,建议保留一条即可,把“通过
size/style.fontSize控制整体大小”的新版描述合并进去,删除旧的仅提fontSize的那条,避免读者困惑。- 语句「一个优雅的加载动画组件,基于 Framer Motion 实现展示流畅的加载动画效果。」从中文语感上更自然的写法可以考虑改为「一个优雅地展示加载动画的组件,基于 … 实现流畅地加载动画效果。」(LanguageTool 的提示也有类似建议),可按团队文案风格酌情调整。
这些都不影响功能,仅作为文案层的小优化建议。
Also applies to: 48-52
src/Components/effects/LoadingEffect/index.tsx (1)
5-59: LoadingEffect 的 props 类型与实际透传行为不完全一致,建议同步收紧或透传目前声明为:
export const LoadingEffect: React.FC<Omit<EffectPlayerProps, 'sceneUrl'>> = ({ autoplay = true, loop = true, className, style, size, }) => { return ( <EffectPlayer sceneUrl={loadingEffect} size={size} style={style} autoplay={autoplay} loop={loop} className={className} /> ); };有两个小问题:
类型允许但实现中忽略的属性
EffectPlayerProps除了sceneUrl外还包含downgradeImage和所有HTMLAttributes<HTMLDivElement>(如onClick、aria-*等)。- 但组件只解构了
autoplay/loop/className/style/size,其余 props 在运行时全部丢失。类型上看“支持”,实际却不生效,可能会让使用者误解。若有意固定降级图,也建议在类型层面体现
- 如果
LoadingEffect不打算开放downgradeImage配置,建议在类型中一并排除,并同时透传其他 HTML 属性。可以参考如下改法,使类型和行为对齐,并保留通用属性透传(供无障碍等场景使用):
-export const LoadingEffect: React.FC<Omit<EffectPlayerProps, 'sceneUrl'>> = ({ - autoplay = true, - loop = true, - className, - style, - size, -}) => { - return ( - <EffectPlayer - sceneUrl={loadingEffect} - size={size} - style={style} - autoplay={autoplay} - loop={loop} - className={className} - /> - ); -}; +export const LoadingEffect: React.FC< + Omit<EffectPlayerProps, 'sceneUrl' | 'downgradeImage'> +> = ({ autoplay = true, loop = true, className, style, size, ...rest }) => { + return ( + <EffectPlayer + sceneUrl={loadingEffect} + size={size} + style={style} + autoplay={autoplay} + loop={loop} + className={className} + {...rest} + /> + ); +};另外,注释中「使用 Lottie 动画展示加载动画的组件」的表述可以根据 EffectPlayer 实际依赖的引擎适当改成更通用的“特效动画”/“效果动画”,以免和 Lottie 组件混淆。
docs/demos/loading.tsx (1)
4-121: Demo 结构完善,建议同步更新「技术栈」描述以贴合新实现
- 新增的
DemoCard组件把各类 Loading 用法包装成卡片,结构清晰,配合type、autoplay、loop、size的组合,基本把新 API 能力都演示到了,这块设计不错。- 不过当前文件底部仍然写着使用 Framer Motion 的
motion.svg/motion.ellipse来实现动画,而新的实现已经切换到LoadingLottie/CreativeSparkLottie/CreativeRecommendationEffect这套体系上。建议和组件实际实现再对一遍,看是否需要将这里的“技术栈”文案更新为更符合现状的说明(比如强调 Lottie / EffectPlayer),避免阅读文档时产生错觉。src/Components/Loading/index.tsx (1)
6-8: 基于 type 的多动画分支实现合理,建议同步更新组件注释实现层面看:
LoadingProps在原有LoadingLottieProps上增加了type?: 'loading' | 'spark' | 'recommendation',并且在实现中通过if/else分支选择对应的动画组件,type未指定时回退到LoadingLottie,和文档中默认值'loading'保持一致。lottieProps默认size: '1em',并允许用户通过props.size或style.fontSize调整,和文档对「灵活尺寸」的说明相符。- spark 分支通过
transform: 'scale(1.2)'做了一点视觉强化,也保持了对原有style的合并。唯一不太匹配的是头部 JSDoc 仍然在描述“两个旋转的椭圆形”等旧版实现细节。建议把注释改成更抽象的、与当前多动画实现一致的描述,例如强调:
- 提供三种不同风格的加载动画(loading/spark/recommendation);
- 默认类型及典型使用场景。
这样文档、注释和实现三者会更加一致。
Also applies to: 33-50
src/Components/effects/CreativeRecommendationEffect/index.tsx (1)
5-57: CreativeRecommendationEffect 同样存在 props 类型与实现不一致的问题当前实现:
export const CreativeRecommendationEffect: React.FC< Omit<EffectPlayerProps, 'sceneUrl'> > = ({ autoplay = true, loop = true, className, style, size }) => { return ( <EffectPlayer sceneUrl={creativeRecommendation} downgradeImage="..." size={size} style={style} autoplay={autoplay} loop={loop} className={className} /> ); };问题与
LoadingEffect类似:
类型允许传入但实现忽略的属性
Omit<EffectPlayerProps, 'sceneUrl'>依然包含downgradeImage和所有HTMLAttributes<HTMLDivElement>。- 组件只解构并使用了 autoplay/loop/className/style/size,其余全部被丢弃,调用者若传入
downgradeImage或onClick等属性,行为上是无效的。更符合意图的类型与实现建议
- 如果这里是刻意固定
downgradeImage(看起来是统一的兜底图),建议在类型上也排除它,同时仍然透传其他 HTML 属性,避免“看起来可以传其实无效”的情况:-export const CreativeRecommendationEffect: React.FC< - Omit<EffectPlayerProps, 'sceneUrl'> -> = ({ autoplay = true, loop = true, className, style, size }) => { - return ( - <EffectPlayer - sceneUrl={creativeRecommendation} - downgradeImage="..." - size={size} - style={style} - autoplay={autoplay} - loop={loop} - className={className} - /> - ); -}; +export const CreativeRecommendationEffect: React.FC< + Omit<EffectPlayerProps, 'sceneUrl' | 'downgradeImage'> +> = ({ autoplay = true, loop = true, className, style, size, ...rest }) => { + return ( + <EffectPlayer + sceneUrl={creativeRecommendation} + downgradeImage="..." + size={size} + style={style} + autoplay={autoplay} + loop={loop} + className={className} + {...rest} + /> + ); +};
- 注释中的“使用 Lottie 动画”表述
- 与
LoadingEffect一样,建议根据 EffectPlayer 的实际实现适当调整 wording(例如“创意推荐特效动画组件”),避免和纯 Lottie 组件的概念混淆。整体逻辑是 OK 的,上述调整主要是为了让类型、实现和文档保持一致。
src/Components/effects/EffectPlayer.tsx (2)
38-56: TODO: 实现 loop 功能
loop属性已在接口中定义并有文档说明,但当前实现中被注释掉未使用。请确认是否需要在后续版本中实现此功能。是否需要我帮助生成实现 loop 功能的代码,或者创建一个 issue 来跟踪此任务?
119-136: 可选:改进降级图片的可访问性当前降级图片的
alt属性值为 "fallback",比较通用。可以考虑使用更具描述性的文案,或者通过 props 传入自定义的 alt 文本。此外,如果降级图片传达了重要信息,可以考虑添加
role="img"和aria-label来提升可访问性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/components/loading.md(2 hunks)docs/demos/loading.tsx(1 hunks)package.json(1 hunks)src/Components/Loading/index.tsx(2 hunks)src/Components/Robot/index.tsx(0 hunks)src/Components/effects/CreativeRecommendationEffect/creativeRecommendation.json(1 hunks)src/Components/effects/CreativeRecommendationEffect/index.tsx(1 hunks)src/Components/effects/EffectPlayer.tsx(1 hunks)src/Components/effects/LoadingEffect/index.tsx(1 hunks)src/Components/effects/LoadingEffect/loading.json(1 hunks)src/Components/effects/index.ts(1 hunks)src/Components/lotties/CreativeSparkLottie/index.tsx(1 hunks)src/Components/lotties/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/Components/Robot/index.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/Components/effects/CreativeRecommendationEffect/index.tsx (1)
src/Components/effects/EffectPlayer.tsx (1)
EffectPlayerProps(12-36)
src/Components/effects/LoadingEffect/index.tsx (1)
src/Components/effects/EffectPlayer.tsx (1)
EffectPlayerProps(12-36)
src/Components/Loading/index.tsx (3)
src/Components/lotties/LoadingLottie/index.tsx (2)
LoadingLottieProps(5-29)LoadingLottie(68-95)src/Components/lotties/CreativeSparkLottie/index.tsx (1)
CreativeSparkLottie(68-95)src/Components/effects/CreativeRecommendationEffect/index.tsx (1)
CreativeRecommendationEffect(42-58)
docs/demos/loading.tsx (1)
src/Components/Loading/index.tsx (1)
Loading(33-51)
🪛 LanguageTool
docs/components/loading.md
[uncategorized] ~11-~11: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:优雅"地"加
Context: ...e: 通用 order: 5 --- # Loading 加载 一个优雅的加载动画组件,基于 Framer Motion 实现展示流畅的加载动画效果。 ...
(wb4)
[uncategorized] ~11-~11: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:流畅"地"加
Context: ... 加载 一个优雅的加载动画组件,基于 Framer Motion 实现展示流畅的加载动画效果。 ## 代码演示 <code src="../demos/lo...
(wb4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (11)
package.json (1)
54-54: 新增 @galacean/effects 依赖建议确认版本与体积影响引入
@galacean/effects作为运行时依赖是合理的(EffectPlayer 系列组件需要),但建议再确认下:目前选的^2.7.3是否是团队期望的基线版本,以及在典型业务打包体积下是否可接受,避免对下游应用造成意外的 bundle 放大或运行环境兼容性问题。src/Components/lotties/index.ts (1)
1-5: 统一导出 Lottie 组件的入口设计合理通过 barrel 文件集中导出
CreativeSparkLottie、DazingLottie、LoadingLottie、ThinkingLottie和TextLoading,有利于对外提供稳定、清晰的 API,使用上也更简洁,没有看到明显的问题。src/Components/effects/index.ts (1)
1-3: Effect 组件集中导出方式清晰可用将
CreativeRecommendationEffect、EffectPlayer、LoadingEffect通过具名导出统一暴露,使上层组件可以从单一入口引入效果相关能力,结构清晰,没有发现问题。src/Components/lotties/CreativeSparkLottie/index.tsx (3)
5-29: LGTM!接口定义清晰,文档完善,Props 设计合理。
68-95: LGTM!组件实现正确,样式合并顺序正确(用户样式覆盖默认样式),
aria-hidden="true"适用于装饰性动画。
1-3: 导入语句验证通过
creativeSpark.json文件存在于正确的路径中。所有导入语句都有效且必要,无需修改。src/Components/effects/EffectPlayer.tsx (3)
12-36: LGTM!接口定义良好,正确排除了
ref属性。注意到loop属性已定义但实现中有 TODO 标记(第 42-43 行)。
71-91: LGTM!正确使用
useLayoutEffect进行 Player 初始化,空依赖数组确保只在挂载时创建一次。通过useLatest保持downgradeImage引用新鲜,避免了闭包陈旧问题。清理函数正确释放资源。
94-111: 审查依赖数组的正确性第 94-101 行的
useEffect在sceneUrl改变时重新加载场景并使用当前的autoplay值。如果autoplay改变而sceneUrl保持不变,场景不会重新加载。这可能是有意的设计,但请确认是否符合预期行为。第 103-111 行的独立
autoplayeffect 处理播放/暂停切换,设计合理。src/Components/effects/LoadingEffect/loading.json (1)
1-922: LGTM!加载效果配置结构合理,使用 8 个旋转段(0°-315°)和渐进延迟(0-0.87s)创建旋转动画效果。无外部依赖,完全基于形状渲染,有利于离线使用。
src/Components/effects/CreativeRecommendationEffect/creativeRecommendation.json (1)
6-13: 外部图片资源已验证可用两个 CDN 图片 URL 均已成功验证:
- 原始格式图片:HTTP 200 ✓
- WebP 格式图片:HTTP 200 ✓
资源当前可正常访问,JSON 配置无问题。
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
♻️ Duplicate comments (2)
src/Components/effects/EffectPlayer.tsx (2)
1-10: 依赖版本问题已在之前的评审中标记之前的评审已经指出
@galacean/effects版本^2.7.3不存在,最新稳定版本是2.6.5。请参考之前的评审意见更新依赖版本。
27-31: loop 属性未实现问题已在之前的评审中标记之前的评审已经指出
loop属性在接口中定义但未实现(实现部分被注释掉)。请参考之前的评审意见,要么实现该功能,要么暂时从接口中移除该属性。
🧹 Nitpick comments (3)
docs/components/loading.md (1)
40-43: 优化特性描述以减少与使用说明的重复。第42行关于"灵活尺寸"的特性说明与第47-49行的使用说明存在内容重复。这与之前的代码评审意见一致——建议在特性部分保留高层特性描述,具体使用细节交由使用说明部分详细说明,以保持文档的简洁性和层次清晰。
建议修改第40-43行,将"灵活尺寸"特性的具体配置细节移至使用说明,或在特性部分仅保留概括性描述:
- 📏 **灵活尺寸**: 支持通过 `size` 属性或 `style` 中的 `fontSize` 控制整体大小 + 📏 **灵活尺寸**: 支持多种方式调整动画尺寸src/Components/effects/EffectPlayer.tsx (2)
71-91: 建议添加 ESLint 忽略注释或重新审视依赖数组这个
useLayoutEffect使用空依赖数组是有意为之,目的是只在组件挂载时创建一次 Player 实例。后续的sceneUrl和autoplay变化由其他 effect 处理。但是,React 的
exhaustive-depsESLint 规则会标记缺少onError、sceneUrl和autoplay依赖。虽然当前实现逻辑正确,但建议添加 ESLint 忽略注释以明确这是有意的设计决策:// ==================== Create Player ==================== + // eslint-disable-next-line react-hooks/exhaustive-deps useLayoutEffect(() => {或者在注释中说明为什么使用空依赖数组。
94-101: 建议处理 exhaustive-deps 警告这个 effect 在加载场景时使用了
autoplay(第 98 行),但autoplay不在依赖数组中。当前设计是有意为之:只在sceneUrl变化时重新加载场景,autoplay的变化由单独的 effect(103-111 行)通过暂停/恢复来处理,避免不必要的场景重新加载。但
exhaustive-deps规则会标记这个问题。建议添加 ESLint 注释说明:useEffect(() => { if (playerRef.current) { setFallbackImage(undefined); playerRef.current.loadScene(sceneUrl, { autoplay, }); } + // autoplay 的变化由单独的 effect 处理,避免不必要的场景重新加载 + // eslint-disable-next-line react-hooks/exhaustive-deps }, [sceneUrl]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/components/loading.md(2 hunks)src/Components/effects/EffectPlayer.tsx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/components/loading.md
[uncategorized] ~11-~11: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:优雅"地"加
Context: ...e: 通用 order: 5 --- # Loading 加载 一个优雅的加载动画组件,基于 Framer Motion 实现展示流畅的加载动画效果。 ...
(wb4)
[uncategorized] ~11-~11: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:流畅"地"加
Context: ... 加载 一个优雅的加载动画组件,基于 Framer Motion 实现展示流畅的加载动画效果。 ## 代码演示 <code src="../demos/lo...
(wb4)
🔇 Additional comments (1)
docs/components/loading.md (1)
21-50: API文档和新功能说明整体结构良好。新增的
type参数、动画类型分类、特性和使用说明都清晰准确地反映了Loading组件的增强功能。文档结构层次分明,易于用户理解和使用。
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: 0
🧹 Nitpick comments (4)
src/Workspace/Task/style.ts (1)
24-24: 可选:考虑提取为命名常量根据编码规范,建议将硬编码的数值提取为命名常量以提高可读性。不过,由于这是单次使用的样式值,提取可能是可选的。
可以这样重构:
+const STATUS_FONT_SIZE = 16; + const genStyle: GenerateStyle<ChatTokenType> = (token) => { return { [`${token.componentCls}`]: { // ... '&-status': { display: 'flex', alignItems: 'center', justifyContent: 'center', flexShrink: 0, marginTop: '2px', - fontSize: 16, + fontSize: STATUS_FONT_SIZE, },src/Components/effects/EffectPlayer.tsx (1)
119-134: 建议使用 Tailwind 类进行样式设置根据编码规范,应优先使用 Tailwind 类而非内联样式。虽然动态
size属性需要内联样式,但其他静态样式可以改用 Tailwind 类。可以这样重构:
return ( - <div {...attrs} ref={containerRef} style={containerStyle}> + <div + {...attrs} + ref={containerRef} + className="relative flex justify-center items-center" + style={{ width: size, height: size, ...style }} + > {fallbackImage && ( <img src={fallbackImage} alt="fallback" - style={{ - position: 'absolute', - top: 0, - left: 0, - width: '100%', - height: '100%', - }} + className="absolute top-0 left-0 w-full h-full" /> )} </div> );根据编码规范。
src/Workspace/Task/index.tsx (2)
40-48: 可选:考虑使用 Tailwind 类根据编码规范,建议使用 Tailwind 类替代内联样式。虽然 CSS 变量用于主题化是合适的,但
transform: scale(1.1)可以使用 Tailwind 的scale-110类。可以这样重构:
case 'loading': return ( <LoadingEffect + className="scale-110" style={{ color: 'var(--color-gray-text-disabled)', - transform: 'scale(1.1)', }} /> );根据编码规范。
89-89: 可选:使用 Tailwind 类设置 cursor根据编码规范,建议使用 Tailwind 类替代内联样式。
可以这样重构:
<div key={item.key} - className={classNames( + className={classNames( `${prefixCls}-item`, `${prefixCls}-item-${item.status}`, hashId, + onItemClick && 'cursor-pointer', )} role={onItemClick ? 'button' : undefined} tabIndex={onItemClick ? 0 : undefined} onClick={onItemClick ? () => handleItemClick(item) : undefined} onKeyDown={...} - style={{ cursor: onItemClick ? 'pointer' : undefined }} >根据编码规范。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/demos/workspace-demo.tsx(1 hunks)src/Components/effects/EffectPlayer.tsx(1 hunks)src/Workspace/Task/index.tsx(4 hunks)src/Workspace/Task/style.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,go,rb,php,scala,kotlin,swift}
📄 CodeRabbit inference engine (.cursor/rules/clean-code.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,go,rb,php,scala,kotlin,swift}: Replace hard-coded values with named constants. Use descriptive constant names that explain the value's purpose. Keep constants at the top of the file or in a dedicated constants file.
Variables, functions, and classes should reveal their purpose. Names should explain why something exists and how it's used. Avoid abbreviations unless they're universally understood.
Don't comment on what the code does - make the code self-documenting. Use comments to explain why something is done a certain way. Document APIs, complex algorithms, and non-obvious side effects.
Each function should do exactly one thing. Functions should be small and focused. If a function needs a comment to explain what it does, it should be split.
Extract repeated code into reusable functions. Share common logic through proper abstraction. Maintain single sources of truth.
Hide implementation details. Expose clear interfaces. Move nested conditionals into well-named functions.
If you need more than 3 levels of indentation, you should fix your program. Functions must be short and small, doing only one thing and doing it well.
Files:
docs/demos/workspace-demo.tsxsrc/Components/effects/EffectPlayer.tsxsrc/Workspace/Task/style.tssrc/Workspace/Task/index.tsx
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx,ts,js}: Use early returns whenever possible to make code more readable in React/JavaScript/TypeScript components
Use descriptive variable and function/const names; event handlers should use 'handle' prefix (e.g., handleClick for onClick, handleKeyDown for onKeyDown)
Files:
docs/demos/workspace-demo.tsxsrc/Components/effects/EffectPlayer.tsxsrc/Workspace/Task/style.tssrc/Workspace/Task/index.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Always use Tailwind classes for styling HTML elements; avoid using CSS or tags in React components
Use 'class:' syntax instead of ternary operators in class tags whenever possible in React components
Implement accessibility features on elements including tabindex='0', aria-label, on:click, and on:keydown attributes where appropriate
Files:
docs/demos/workspace-demo.tsxsrc/Components/effects/EffectPlayer.tsxsrc/Workspace/Task/index.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use const arrow functions instead of function declarations (e.g., 'const toggle = () =>') and define types where possible in React/TypeScript
Files:
docs/demos/workspace-demo.tsxsrc/Components/effects/EffectPlayer.tsxsrc/Workspace/Task/style.tssrc/Workspace/Task/index.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-26T06:59:43.211Z
Learnt from: CR
Repo: ant-design/agentic-ui PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-26T06:59:43.211Z
Learning: Applies to **/*.{tsx,jsx} : Implement accessibility features on elements including tabindex='0', aria-label, on:click, and on:keydown attributes where appropriate
Applied to files:
src/Workspace/Task/index.tsx
🧬 Code graph analysis (1)
src/Workspace/Task/index.tsx (3)
src/Components/effects/LoadingEffect/index.tsx (1)
LoadingEffect(42-59)src/TaskList/index.tsx (1)
TaskList(199-254)src/Workspace/Task/style.ts (1)
useTaskStyle(58-67)
🔇 Additional comments (6)
src/Components/effects/EffectPlayer.tsx (2)
71-91: Player 创建和销毁逻辑正确使用空依赖数组的
useLayoutEffect确保 Player 只在挂载时创建一次,并在卸载时正确销毁。后续的状态变化通过其他useEffect处理,这是合理的设计模式。
94-111: I apologize, but I'm encountering a technical issue accessing the repository to perform the verification. However, I can analyze the review comment based on the code snippet provided.The review comment raises a legitimate concern about effect ordering. Let me provide a rewritten review based on the visible code pattern:
Potential redundant operations when both
sceneUrlandautoplaychange simultaneously.The two effects may cause double operations: when
sceneUrlchanges,loadScene()is called with the currentautoplayvalue (line 98); ifautoplayalso changed in the same render cycle, the second effect (lines 103-111) will separately callresume()orpause(). This could result in:
loadScene(sceneUrl, { autoplay: true })followed byresume()(redundant)- Or potential state inconsistency if timing matters
Consider consolidating this logic: either pass
autoplayonly toloadScene()and rely on that, or verify that both effects running together produces the intended behavior.
However, to properly verify this concern, I would need access to:
- The
playerRefAPI implementation (what doesloadScenewith{ autoplay }do?)- Usage patterns of this component (do both props change together frequently?)
- Whether the separate effects are intentional for decoupled concerns
src/Workspace/Task/index.tsx (3)
8-26: 类型定义清晰且结构良好新增的
TaskStatus、TaskItem和TaskListProps接口定义清晰,遵循 TypeScript 最佳实践。公开导出这些类型有助于外部使用。
57-64: 事件处理器实现规范
handleItemClick遵循编码规范:
- 使用
handle前缀命名- 使用箭头函数
- 简洁且职责单一
76-89: 优秀的无障碍访问实现无障碍功能实现非常完善:
- ✅ 正确设置
role="button"语义- ✅ 使用
tabIndex={0}支持键盘导航- ✅ 实现 Enter 和 Space 键交互
- ✅ Space 键时调用
preventDefault()防止页面滚动- ✅ 提供视觉反馈(cursor: pointer)
- ✅ 所有属性按需条件渲染
这遵循了无障碍最佳实践并符合编码规范中的可访问性要求。
基于学习经验和编码规范。
docs/demos/workspace-demo.tsx (1)
205-205: 演示数据更新合理将任务状态更改为
'loading'以展示新的LoadingEffect动画效果,与 PR 目标一致。
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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 (4)
src/Components/Loading/Loading.tsx (1)
4-28: 同步组件注释与实际 Lottie 实现并提取默认尺寸常量当前 JSDoc 仍然描述为“两个旋转的椭圆形”,但实现已经换成 Lottie 动画,具体形态可能变化,建议改成更贴近实现的中性描述(例如“标准加载动画,基于 Lottie 实现”),以免后续换动画资源时文档再度失真。同时,
'1em'这个默认尺寸在Loading/CreativeSparkLoading/CreativeRecommendationLoading中都被硬编码使用,建议提取为一个统一的常量(例如DEFAULT_LOADING_SIZE = '1em')并从三个组件复用,方便后续统一调整。src/Components/Loading/CreativeSparkLoading.tsx (1)
27-34: 简化 props 写法并与其它 Loading 组件共享默认尺寸常量这里的
({ ...props }: LoadingLottieProps)其实只是把参数对象浅拷贝了一次,写成(props: LoadingLottieProps)会更直观一些。另一个小建议是与Loading/CreativeRecommendationLoading统一使用同一个默认尺寸常量(例如DEFAULT_LOADING_SIZE),避免在多个文件里重复硬编码'1em',后续要改默认尺寸时只需要改一处即可。src/Components/Loading/CreativeRecommendationLoading.tsx (1)
28-36: 对外统一为 Loading API 很合理,建议抽取共享默认尺寸常量让
CreativeRecommendationLoading暴露LoadingLottieProps,内部再适配到CreativeRecommendationEffect的 props,这个设计很好地保证了三个 Loading 组件的 API 一致性。与前两个组件类似,这里也直接写死了size: '1em',建议后续把默认尺寸提取为共享常量统一引用,减少魔法值分散在多个文件里的情况。docs/demos/loading.tsx (1)
8-47: 抽取重复样式与尺寸为常量,避免散落的 magic numberDemo 里多处内联样式和尺寸值(如
width: 120、gap: 24、fontSize: 64、demoSizes = [32, 48, 64, 80]等)在不同块中重复出现,后续如果要统一调整展示密度/尺寸会比较费劲。建议:
- 将常用布局样式(居中 flex、标题文本样式等)封装成常量或小的样式函数;
- 将示例尺寸数组提到组件外,并命名为如
DEMO_SIZES之类的常量;- 如果项目整体约定使用 Tailwind className 而尽量少用内联 style,可以考虑后续逐步把这些 style 替换为 Tailwind 类名(本 PR 可按需取舍)。
Also applies to: 55-63, 85-103, 105-125
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
docs/components/loading.md(1 hunks)docs/demos/loading.tsx(1 hunks)src/Components/Loading/CreativeRecommendationLoading.tsx(1 hunks)src/Components/Loading/CreativeSparkLoading.tsx(1 hunks)src/Components/Loading/Loading.tsx(1 hunks)src/Components/Loading/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,go,rb,php,scala,kotlin,swift}
📄 CodeRabbit inference engine (.cursor/rules/clean-code.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,go,rb,php,scala,kotlin,swift}: Replace hard-coded values with named constants. Use descriptive constant names that explain the value's purpose. Keep constants at the top of the file or in a dedicated constants file.
Variables, functions, and classes should reveal their purpose. Names should explain why something exists and how it's used. Avoid abbreviations unless they're universally understood.
Don't comment on what the code does - make the code self-documenting. Use comments to explain why something is done a certain way. Document APIs, complex algorithms, and non-obvious side effects.
Each function should do exactly one thing. Functions should be small and focused. If a function needs a comment to explain what it does, it should be split.
Extract repeated code into reusable functions. Share common logic through proper abstraction. Maintain single sources of truth.
Hide implementation details. Expose clear interfaces. Move nested conditionals into well-named functions.
If you need more than 3 levels of indentation, you should fix your program. Functions must be short and small, doing only one thing and doing it well.
Files:
src/Components/Loading/Loading.tsxsrc/Components/Loading/CreativeRecommendationLoading.tsxsrc/Components/Loading/CreativeSparkLoading.tsxdocs/demos/loading.tsxsrc/Components/Loading/index.tsx
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx,ts,js}: Use early returns whenever possible to make code more readable in React/JavaScript/TypeScript components
Use descriptive variable and function/const names; event handlers should use 'handle' prefix (e.g., handleClick for onClick, handleKeyDown for onKeyDown)
Files:
src/Components/Loading/Loading.tsxsrc/Components/Loading/CreativeRecommendationLoading.tsxsrc/Components/Loading/CreativeSparkLoading.tsxdocs/demos/loading.tsxsrc/Components/Loading/index.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Always use Tailwind classes for styling HTML elements; avoid using CSS or tags in React components
Use 'class:' syntax instead of ternary operators in class tags whenever possible in React components
Implement accessibility features on elements including tabindex='0', aria-label, on:click, and on:keydown attributes where appropriate
Files:
src/Components/Loading/Loading.tsxsrc/Components/Loading/CreativeRecommendationLoading.tsxsrc/Components/Loading/CreativeSparkLoading.tsxdocs/demos/loading.tsxsrc/Components/Loading/index.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use const arrow functions instead of function declarations (e.g., 'const toggle = () =>') and define types where possible in React/TypeScript
Files:
src/Components/Loading/Loading.tsxsrc/Components/Loading/CreativeRecommendationLoading.tsxsrc/Components/Loading/CreativeSparkLoading.tsxdocs/demos/loading.tsxsrc/Components/Loading/index.tsx
🧬 Code graph analysis (4)
src/Components/Loading/Loading.tsx (2)
src/Components/Loading/index.tsx (1)
Loading(3-3)src/Components/lotties/LoadingLottie/index.tsx (2)
LoadingLottieProps(5-29)LoadingLottie(68-95)
src/Components/Loading/CreativeRecommendationLoading.tsx (2)
src/Components/lotties/LoadingLottie/index.tsx (1)
LoadingLottieProps(5-29)src/Components/effects/CreativeRecommendationEffect/index.tsx (1)
CreativeRecommendationEffect(42-58)
src/Components/Loading/CreativeSparkLoading.tsx (3)
src/Components/Loading/index.tsx (1)
CreativeSparkLoading(2-2)src/Components/lotties/LoadingLottie/index.tsx (1)
LoadingLottieProps(5-29)src/Components/lotties/CreativeSparkLottie/index.tsx (1)
CreativeSparkLottie(68-95)
docs/demos/loading.tsx (3)
src/Components/Loading/Loading.tsx (1)
Loading(27-29)src/Components/Loading/CreativeSparkLoading.tsx (1)
CreativeSparkLoading(27-34)src/Components/Loading/CreativeRecommendationLoading.tsx (1)
CreativeRecommendationLoading(28-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests and collect coverage
🔇 Additional comments (2)
src/Components/Loading/index.tsx (1)
1-3: Loading 相关组件的集中导出方式清晰易用将
CreativeRecommendationLoading、CreativeSparkLoading、Loading统一从同一入口导出,使外部引用路径更简洁,也方便后续扩展新的 Loading 变体,设计合理。docs/demos/loading.tsx (1)
127-137: I encountered a repository access issue that prevents me from directly verifying the codebase. However, based on the information provided:Review documentation mentions outdated tech stack details
The provided code snippet confirms the "技术栈" section (lines 127-137) currently documents "Framer Motion" and "CSS-in-JS" for the Loading component. The review comment indicates the implementation has been switched to use Lottie/EffectPlayer instead, making this documentation inconsistent with the actual implementation.
Without direct access to verify the current Loading component implementation and confirm it uses Lottie/EffectPlayer rather than Framer Motion, I cannot definitively confirm or refute whether this documentation is outdated. Manual verification of the Loading component code would be needed to confirm the tech stack section should be updated as suggested.
EffectPlayer组件CreativeSparkLottie和CreativeRecommendationEffect两种加载中动画组件LoadingLottie的动画效果Loading组件,新增type类型来切换动画效果Summary by CodeRabbit
New Features
Refactor
Accessibility / UX
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.