-
Notifications
You must be signed in to change notification settings - Fork 1.6k
docs: react节点添加一个组合scroll-canvas的FAQ #7520
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: v5
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @XiaoMing-xmg666, 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! 此拉取请求旨在通过在React节点文档中添加一个常见问题解答(FAQ)部分来改进��户体验。该FAQ解决了在使用自定义React节点时 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
这个 PR 很好地为 React 节点和 scroll-canvas 行为的组合添加了 FAQ 文档,解决了社区中一个常见的问题。代码示例清晰地展示了如何通过事件转发来解决滚动问题。
我发现了一些可以改进的地方:
- 在
Node组件的useEffect中,添加的wheel事件监听器没有在组件卸载时被移除,这可能导致内存泄漏。建议添加一个清理函数。 - 在
onWheel事件处理器中,获取 canvas DOM 元素的方式graph.context.graph.getCanvas()是不正确的,因为graphprop 实际上是ReactNode的实例,而不是 G6 的Graph实例,这会导致运行时错误。我建议了一种替代方法来正确获取 canvas 元素。
这些修改将使示例代码更加健壮和正确。
| const canvas = graph.context.graph.getCanvas().getContextService().getDomElement(); | ||
| const evt = new WheelEvent('wheel', e.nativeEvent); | ||
| canvas.dispatchEvent(evt); |
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.
获取画布 DOM 元素的方式不正确。graph prop 是一个 ReactNode 图形实例,因此 graph.context.graph 是 undefined,这将导致运行时错误。你可以通过 ReactNode 实例的 ownerDocument 属性来获取画布 DOM 元素。
const canvas = graph.ownerDocument.defaultView.getContextService().getDomElement();
const evt = new WheelEvent('wheel', e.nativeEvent);
canvas.dispatchEvent(evt);| useEffect(() => { | ||
| if (ref.current) { | ||
| ref.current.parentNode.addEventListener('wheel', (e) => { | ||
| e.preventDefault(); | ||
| }); | ||
| } | ||
| }, []); |
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.
useEffect hook 中添加的事件监听器在组件卸载时没有被移除,这可能会导致内存泄漏。应该从 useEffect 返回一个清理函数来移除事件监听器。
useEffect(() => {
const parent = ref.current?.parentNode;
const onWheel = (e) => {
e.preventDefault();
};
if (parent) {
parent.addEventListener('wheel', onWheel);
}
return () => {
if (parent) {
parent.removeEventListener('wheel', onWheel);
}
};
}, []);| useEffect(() => { | ||
| if (ref.current) { | ||
| ref.current.parentNode.addEventListener('wheel', (e) => e.preventDefault()); | ||
| } | ||
| }, []); |
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.
useEffect 中添加的事件监听器在组件卸载时没有被移除,这可能导致内存泄漏。应该从 useEffect 返回一个清理函数来移除事件监听器。作为文档示例,展示最佳实践非常重要。
| useEffect(() => { | |
| if (ref.current) { | |
| ref.current.parentNode.addEventListener('wheel', (e) => e.preventDefault()); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| const parent = ref.current?.parentNode; | |
| const onWheel = (e) => e.preventDefault(); | |
| if (parent) { | |
| parent.addEventListener('wheel', onWheel); | |
| } | |
| return () => { | |
| if (parent) { | |
| parent.removeEventListener('wheel', onWheel); | |
| } | |
| }; | |
| }, []); |
| const canvas = graph.context.graph.getCanvas().getContextService().getDomElement(); | ||
| const evt = new WheelEvent('wheel', e.nativeEvent); | ||
| canvas.dispatchEvent(evt); |
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.
获取画布 DOM 元素的方式不正确。graph prop 是一个 ReactNode 图形实例,graph.context.graph 的值是 undefined,这将导致运行时错误。你可以通过 ReactNode 实例的 ownerDocument 属性来获取画布 DOM 元素。
| const canvas = graph.context.graph.getCanvas().getContextService().getDomElement(); | |
| const evt = new WheelEvent('wheel', e.nativeEvent); | |
| canvas.dispatchEvent(evt); | |
| const canvas = graph.ownerDocument.defaultView.getContextService().getDomElement(); | |
| const evt = new WheelEvent('wheel', e.nativeEvent); | |
| canvas.dispatchEvent(evt); |
| useEffect(() => { | ||
| if (ref.current) { | ||
| ref.current.parentNode.addEventListener('wheel', (e) => e.preventDefault()); | ||
| } | ||
| }, []); |
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.
useEffect 中添加的事件监听器在组件卸载时没有被移除,这可能导致内存泄漏。应该从 useEffect 返回一个清理函数来移除事件监听器。作为文档示例,展示最佳实践非常重要。
| useEffect(() => { | |
| if (ref.current) { | |
| ref.current.parentNode.addEventListener('wheel', (e) => e.preventDefault()); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| const parent = ref.current?.parentNode; | |
| const onWheel = (e) => e.preventDefault(); | |
| if (parent) { | |
| parent.addEventListener('wheel', onWheel); | |
| } | |
| return () => { | |
| if (parent) { | |
| parent.removeEventListener('wheel', onWheel); | |
| } | |
| }; | |
| }, []); |
| const canvas = graph.context.graph.getCanvas().getContextService().getDomElement(); | ||
| const evt = new WheelEvent('wheel', e.nativeEvent); | ||
| canvas.dispatchEvent(evt); |
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.
获取画布 DOM 元素的方式不正确。graph prop 是一个 ReactNode 图形实例,graph.context.graph 的值是 undefined,这将导致运行时错误。你可以通过 ReactNode 实例的 ownerDocument 属性来获取画布 DOM 元素。
| const canvas = graph.context.graph.getCanvas().getContextService().getDomElement(); | |
| const evt = new WheelEvent('wheel', e.nativeEvent); | |
| canvas.dispatchEvent(evt); | |
| const canvas = graph.ownerDocument.defaultView.getContextService().getDomElement(); | |
| const evt = new WheelEvent('wheel', e.nativeEvent); | |
| canvas.dispatchEvent(evt); |
fixes: #6846
