-
-
Notifications
You must be signed in to change notification settings - Fork 179
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
docs: add GPT-Vis demo for bubble #278
Conversation
📝 Walkthrough📝 WalkthroughWalkthrough此次更改主要涉及对 Bubble 组件文档的增强,新增了自定义图表渲染示例代码,分别在英文和中文文档中添加了相关内容。此外,更新了 Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
Bundle ReportBundle size has no change ✅ |
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: 2
🧹 Outside diff range and nitpick comments (6)
components/bubble/demo/chart.tsx (4)
4-4
: 建议:请说明禁用 ESLint 规则的原因
eslint-disable
注释应该包含具体原因说明,以便其他开发者理解为什么需要禁用这个规则。建议修改为:
-/* eslint-disable react/no-danger */ +/* eslint-disable react/no-danger -- GPTVis 组件需要渲染 markdown 内容 */
7-20
: 建议:优化数据处理方式当前实现存在以下可以改进的地方:
- 建议将图表数据抽离为单独的数据文件
- 可以为数据添加 TypeScript 类型定义
建议重构如下:
// types.ts interface RevenueData { time: number; value: number; } // data.ts export const haidilaoRevenue: RevenueData[] = [ { time: 2013, value: 59.3 }, // ... 其他数据 ]; // chart.tsx import { haidilaoRevenue } from './data';
33-33
: 建议:删除或启用注释代码不建议保留被注释的代码。如果这是一个有用的样式配置,建议启用它;如果不需要,则应该删除。
31-31
: 建议:将静态资源配置抽离头像 URL 应该通过配置文件或环境变量管理,而不是硬编码在组件中。
建议创建配置文件:
// config.ts export const CONFIG = { avatar: { src: 'https://mdn.alipayobjects.com/huamei_qa8qxu/afts/img/A*2Q5LRJ3LFPUAAAAAAAAAAAAADmJ7AQ/fmt.webp', }, }; // 在组件中使用 import { CONFIG } from './config'; // ... avatar={CONFIG.avatar}components/bubble/index.zh-CN.md (1)
33-33
: 建议为新增的图表渲染示例添加描述说明新增的图表渲染示例与PR目标相符。建议在代码示例前添加简要说明,以帮助用户更好地理解此示例的用途。
建议添加如下内容:
+### 图表渲染 + +展示如何使用 Bubble 组件结合 GPT-Vis 进行图表渲染。 + <code src="./demo/chart.tsx">自定义图表渲染</code>components/bubble/index.en-US.md (1)
32-32
: 示例代码添加正确,建议优化描述新增的图表渲染示例与现有文档结构保持一致,符合 PR 的预期目标。不过建议将描述改得更具体一些,以便用户更好地理解这个示例的用途。
建议将描述修改如下:
-<code src="./demo/chart.tsx">Custom chart render</code> +<code src="./demo/chart.tsx">Custom chart render with visualization</code>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
components/bubble/demo/chart.md
(1 hunks)components/bubble/demo/chart.tsx
(1 hunks)components/bubble/index.en-US.md
(1 hunks)components/bubble/index.zh-CN.md
(1 hunks)package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/bubble/demo/chart.md
components/bubble/demo/chart.tsx
Outdated
\`\`\` | ||
`; | ||
|
||
const RenderMarkdown: BubbleProps['messageRender'] = (content) => <GPTVis>{content}</GPTVis>; |
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
建议:添加错误处理机制
建议为 RenderMarkdown
函数添加错误边界处理,以防止渲染失败影响整个应用。
建议修改为:
const RenderMarkdown: BubbleProps['messageRender'] = (content) => {
try {
return <GPTVis>{content}</GPTVis>;
} catch (error) {
console.error('渲染 markdown 内容失败:', error);
return <div>内容渲染失败,请稍后重试</div>;
}
};
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
🧹 Outside diff range and nitpick comments (4)
components/bubble/demo/gpt-vis.tsx (4)
1-5
: 移除不必要的 ESLint 禁用注释代码中包含了
/* eslint-disable react/no-danger */
注释,但实际上组件并未使用dangerouslySetInnerHTML
,建议移除这个不必要的注释。-/* eslint-disable react/no-danger */
7-20
: 优化图表配置和数据结构建议对图表配置进行以下改进:
- 轴标题应更具描述性,当前的 "sale" 和 "year" 过于简单
- 建议添加单位信息以提高数据可读性
"data": [{"time":2013,"value":59.3},{"time":2014,"value":64.4},{"time":2015,"value":68.9},{"time":2016,"value":74.4},{"time":2017,"value":82.7},{"time":2018,"value":91.9},{"time":2019,"value":99.1},{"time":2020,"value":101.6},{"time":2021,"value":114.4},{"time":2022,"value":121}], - "axisXTitle": "year", - "axisYTitle": "sale" + "axisXTitle": "年份", + "axisYTitle": "营业收入(亿元)"
22-22
: 建议增加错误处理机制当前的
RenderMarkdown
实现较为简单,建议添加错误处理以提高组件的健壮性。-const RenderMarkdown: BubbleProps['messageRender'] = (content) => <GPTVis>{content}</GPTVis>; +const RenderMarkdown: BubbleProps['messageRender'] = (content) => { + try { + return <GPTVis>{content}</GPTVis>; + } catch (error) { + console.error('渲染失败:', error); + return <div>图表渲染失败,请检查数据格式</div>; + } +};
24-36
: 优化性能和代码结构建议对组件实现进行以下优化:
- 将打字效果的配置抽取为常量
- 使用
useMemo
优化性能+const TYPING_CONFIG = { + step: 30, + interval: 300, +} as const; + +const AVATAR_CONFIG = { + src: 'https://mdn.alipayobjects.com/huamei_qa8qxu/afts/img/A*2Q5LRJ3LFPUAAAAAAAAAAAAADmJ7AQ/fmt.webp', +} as const; const App = () => { + const memoizedContent = React.useMemo(() => text, []); return ( <Bubble - typing={{ step: 30, interval: 300 }} + typing={TYPING_CONFIG} - content={text} + content={memoizedContent} messageRender={RenderMarkdown} - avatar={{ - src: 'https://mdn.alipayobjects.com/huamei_qa8qxu/afts/img/A*2Q5LRJ3LFPUAAAAAAAAAAAAADmJ7AQ/fmt.webp', - }} + avatar={AVATAR_CONFIG} variant="outlined" /> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
.dumirc.ts
(1 hunks)components/bubble/demo/gpt-vis.md
(1 hunks)components/bubble/demo/gpt-vis.tsx
(1 hunks)components/bubble/index.en-US.md
(1 hunks)components/bubble/index.zh-CN.md
(1 hunks)package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/bubble/demo/gpt-vis.md
🚧 Files skipped from review as they are similar to previous changes (3)
- components/bubble/index.en-US.md
- components/bubble/index.zh-CN.md
- package.json
🔇 Additional comments (3)
components/bubble/demo/gpt-vis.tsx (1)
38-38
: 导出语句符合规范
默认导出语句符合 React 组件的常见实践。
.dumirc.ts (2)
43-47
: 构建优化配置看起来不错!
通过将 mapbox-gl 和 maplibre-gl 配置为外部依赖,可以有效减小构建包的大小。这是一个很好的优化方案。
43-47
: 建议验证全局变量的可用性
请确保在使用这些地图库的页面中正确加载了 mapboxgl 和 maplibregl 的全局变量。
运行以下脚本来检查相关的 HTML 模板或入口文件是否包含必要的脚本引用:
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
🧹 Outside diff range and nitpick comments (1)
components/bubble/__tests__/demo-extend.test.ts (1)
4-4
: 建议补充环境要求说明当前注释简单说明了需要浏览器环境,建议添加更详细的说明,包括所需的具体环境要求和测试建议。
建议修改注释如下:
- // Ignore gpt-vis demo, need browser env not jsdom + // 跳过 gpt-vis demo 测试: + // 1. 该 demo 需要真实浏览器环境,不支持 jsdom + // 2. 如需本地测试,请使用 `npm run test:client` 命令
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/bubble/__tests__/demo-extend.test.ts
(1 hunks)components/bubble/__tests__/demo.test.ts
(1 hunks)
🔇 Additional comments (2)
components/bubble/__tests__/demo.test.ts (1)
3-6
: 测试配置更改合理且文档清晰
配置更改正确处理了 GPT-Vis 演示在测试环境中的兼容性问题。注释明确说明了跳过测试的原因,这是处理需要浏览器环境的组件演示的最佳实践。
components/bubble/__tests__/demo-extend.test.ts (1)
3-6
: 配置跳过 GPT-Vis 测试用例是合理的做法
由于 GPT-Vis 需要浏览器环境而不是 jsdom,跳过相关测试用例是正确的处理方式。这样可以避免在 CI 环境中出现不必要的测试失败。
https://umijs.org/docs/api/config#exportstatic 我试了下,这个配置倒是可以跳过 SSR 预渲染,但是是将 '/components/bubble' 一整个路由都跳过,感觉不太好。 |
|
可视化图表环境 SSR 成本太高了,不知道 Dumi 有种配置没有,这这样代码块可以类似于 ifram 单独渲染 |
找了圈,貌似没有 |
应该有办法,首页那个 lottie 动画也是一样的 |
@lvisei 我合早了,gpt-vis 里最好判断一下,不要直接调用 document。 |
@lvisei 可以重新来个 PR,看看 SSR 的问题怎么解决一下。 |
GPT-Vis 这边找到解决方案了,等下再单独提一个 PR |
Summary by CodeRabbit