Dev/fixed vis dom render #254
Conversation
|
Summary of ChangesHello @XinyueDu, 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! 此拉取请求旨在解决图表在外部容器尺寸变化时无法正确响应和调整大小的问题。通过引入 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
|
|
🎊 PR Preview e01f62e has been successfully built and deployed to https://antvis-GPT-Vis-preview-pr-254.surge.sh 🕐 Build time: 16.866s
🤖 By surge-preview |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 PR 很好地解决了视口变化时图表重绘的问题。对于 G2 图表，通过 fitView 等方法实现自适应；对于 G6 图表，通过重新挂载 DOM 的方式来解决。代码实现了解耦，并增加了相应的演示案例。
我发现 VisChart.tsx 中处理图表 resize 的部分可以进行一些优化。具体来说，有两个 useEffect hook 存在代码重复，并且使用方式上有一些可以改进的地方，比如存在陈旧闭包（stale closure）和不推荐的依赖项。我提供了一个具体的代码建议来合并这两个 useEffect，以提高代码的可读性和健壮性。
There was a problem hiding this comment.
Pull request overview
This PR implements chart resizing functionality to automatically adapt chart dimensions when the viewport or container size changes. The implementation uses different strategies for G2 and G6 charts: G2 charts leverage the fitView method called in onReady, while G6 charts use a DOM remounting approach with a reset key mechanism. The PR also includes a new demo showcasing the resize behavior and renames the error demo file to follow kebab-case naming conventions.
Key Changes:
- Added ResizeObserver-based logic to detect and respond to container size changes
- Implemented chart resize methods for G2 charts and full re-render strategy for G6 charts
- Created new demo (
dom-render-resize.tsx) to demonstrate resize functionality
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
src/GPTVis/index.md |
Added new section for resize rendering demo and fixed demo file reference to kebab-case |
src/GPTVis/index.en.md |
Added English documentation for resize rendering demo and fixed demo file reference |
src/GPTVis/demos/error-render.tsx |
New error demo file with kebab-case naming showing flow diagram with invalid data |
src/GPTVis/demos/dom-render-resize.tsx |
New demo illustrating chart resize behavior with expandable/collapsible container |
src/ChartCodeRender/VisChart.tsx |
Core implementation of resize logic with dual ResizeObserver approach and chart adaptation methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Helper: Resize chart to match container (handles G2/G6) | ||
| const resizeChartToContainer = () => { | ||
| const chart = chartRef.current; | ||
| const container = chartContainerRef.current; | ||
| if (!chart || !container) return; | ||
| const rect = container.getBoundingClientRect(); | ||
| const width = Math.round(rect.width); | ||
| const height = Math.round(rect.height); | ||
| if (width <= 0 || height <= 0) return; | ||
| try { | ||
| if (typeof chart.forceFit === 'function') { | ||
| chart.forceFit(); | ||
| } else if (typeof chart.changeSize === 'function') { | ||
| chart.changeSize(width, height); | ||
| } else if (typeof chart.fitView === 'function') { | ||
| chart.fitView(); | ||
| } else if (typeof chart.layout === 'function') { | ||
| chart.layout(); | ||
| } else if (typeof chart.render === 'function') { | ||
| chart.render(); | ||
| } | ||
| } catch (error) { | ||
| console.error('GPT-Vis resize chart error:', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The resizeChartToContainer function is defined inside the component body but is not memoized with useCallback. Since it's used in multiple useEffect hooks and called from onReady callbacks, this function will be recreated on every render. This could cause the effects to re-run unnecessarily or reference stale closures. Wrap this function in useCallback with appropriate dependencies to ensure stable references.
PR includes
视口变化时图表 resize
G2 在 onReady 中调用 chart.fitView 方法实现图表适应窗口
G6 由于没有实现 onReady，使用 DOM 重新挂载的方法暂时实现
fixed #0
add / modify test cases
documents, demos
Screenshot