-
Notifications
You must be signed in to change notification settings - Fork 0
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
Steve/line-points-snap #1464
Steve/line-points-snap #1464
Conversation
概述这组更改主要关注于在图表绘制画布中引入可编辑线映射(Editable Line Map)的功能。这些修改涉及多个文件,旨在改进边缘(Edge)和线条的交互和管理方式,通过引入新的数据结构和状态管理方法来增强绘图组件的灵活性。 变更
详细解析这组变更的核心目标是引入一个更灵活、更动态的方式来管理图表中的边缘和线条。主要变化包括:
这些变更提高了图表绘制组件的灵活性和可维护性,为未来的功能扩展奠定了基础。 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 (
|
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.
Copilot reviewed 5 out of 13 changed files in this pull request and generated no comments.
Files not reviewed (8)
- bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.spec.ts: Evaluated as low risk
- bricks/diagram/src/draw-canvas/SmartConnectLineComponent.tsx: Evaluated as low risk
- bricks/diagram/src/draw-canvas/HoverStateContext.ts: Evaluated as low risk
- bricks/diagram/src/display-canvas/index.tsx: Evaluated as low risk
- bricks/diagram/src/draw-canvas/CellComponent.tsx: Evaluated as low risk
- bricks/diagram/src/draw-canvas/LineEditorComponent.tsx: Evaluated as low risk
- bricks/diagram/src/draw-canvas/EditingLineComponent.tsx: Evaluated as low risk
- bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.ts: Evaluated as low risk
Comments suppressed due to low confidence (3)
bricks/diagram/src/draw-canvas/interfaces.ts:388
- [nitpick] The name
EditableLine
might be ambiguous. Consider renaming it toEditableEdgeLine
for better clarity.
export interface EditableLine {
bricks/diagram/src/draw-canvas/LineConnectorComponent.tsx:38
- Add a null check for
hoverState
before accessing its properties.
setHoverState((prev) => prev && prev.activePointIndex !== index ? { ...hoverState!, activePointIndex: index } : prev);
bricks/diagram/src/draw-canvas/LineConnectorComponent.tsx:54
- Add null checks for
hoverState
androotRef.current
before accessing their properties.
const rect = rootRef.current!.getBoundingClientRect();
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1464 +/- ##
=======================================
Coverage 85.44% 85.45%
=======================================
Files 524 525 +1
Lines 15670 15677 +7
Branches 2349 2352 +3
=======================================
+ Hits 13389 13396 +7
Misses 1829 1829
Partials 452 452
|
🚀 Deployed on https://docs-preview-1464--next-bricks.netlify.app |
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 (6)
bricks/diagram/src/draw-canvas/EditingLineComponent.tsx (2)
71-72
: 注释中的日志
考虑移除或使用调试工具,以保持正式代码整洁。- // console.log(cells.filter(isEdgeCell));
85-96
: 在循环中重复向 otherPoints 推入控制点
如果线条数量多,这段逻辑可能带来性能开销。可考虑使用更高效的数据结构或空间索引加速查找。bricks/diagram/src/shared/canvas/useEditableLineMap.ts (2)
25-26
: 可考虑在大型数据集下的性能。
for (const edge of cells)
对大规模cells
反复执行遍历时,可能影响性能。若性能为瓶颈,可配合分段处理或自定义优化逻辑。
56-64
: 对于空点集的处理方式合理,但可考虑日志或提示。
当points
为空时跳过设置可编辑线,逻辑稳健。然而如果是由于缺乏数据或配置错误导致,或许需要在开发环境给出调试信息。bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.spec.ts (2)
11-16
: 测试数据可适度拆分为高内聚测试对象。
source
、target
、activeEditableEdge
的视图坐标可抽取到beforeEach
块或测试工厂方法,减少重复定义并提高可读性。
48-58
: 细节逻辑的断言看起来精确,但请确保场景完备。
当前仅验证hoverState.activePointIndex
为 0 的情况,如需覆盖更多情形(如无效索引或多点索引),可考虑额外添加用例。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
bricks/diagram/src/display-canvas/index.tsx
(3 hunks)bricks/diagram/src/draw-canvas/CellComponent.tsx
(7 hunks)bricks/diagram/src/draw-canvas/EdgeComponent.tsx
(2 hunks)bricks/diagram/src/draw-canvas/EditingLineComponent.tsx
(8 hunks)bricks/diagram/src/draw-canvas/HoverStateContext.ts
(3 hunks)bricks/diagram/src/draw-canvas/LineConnectorComponent.tsx
(10 hunks)bricks/diagram/src/draw-canvas/LineEditorComponent.tsx
(5 hunks)bricks/diagram/src/draw-canvas/SmartConnectLineComponent.tsx
(1 hunks)bricks/diagram/src/draw-canvas/index.tsx
(6 hunks)bricks/diagram/src/draw-canvas/interfaces.ts
(2 hunks)bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.spec.ts
(9 hunks)bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.ts
(3 hunks)bricks/diagram/src/shared/canvas/useEditableLineMap.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
bricks/diagram/src/draw-canvas/LineConnectorComponent.tsx
[error] 74-74: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (79)
bricks/diagram/src/draw-canvas/EditingLineComponent.tsx (11)
4-9
: 导入新的 EdgeCell 和 EditableLine 类型
这段导入在组件内清晰地引入了用于可编辑边的类型,能更好地管理可编辑线的状态。实现方式无明显问题。
25-26
: 在 props 中新增 cells 与 editableLineMap
这为组件提供了必要的上下文信息,用于跟踪多个边及其可编辑信息。设计合理。
32-33
: 新增参数在组件中解构
能够清晰引用 cells 与 editableLineMap,以便在组件中使用。无明显问题。
41-41
: 从上下文中获取新的 activeEditableEdge
通过 useHoverStateContext 提供的上下文值来管理可编辑边的状态,保持全局状态同步。实现合理。
58-58
: 在 useEffect 中检查 activeEditableEdge 的存在
在依赖条件下仅在必要时执行逻辑,避免不必要的渲染和事件处理,提高性能。
74-75
: 直接使用可选链断言符号 (!)
如果editableLineMap.get(activeEditableEdge)
结果为空,.points
会触发运行时错误。需确保上层逻辑保证此值存在。
127-136
: 在鼠标松开时更新边的视图
逻辑清晰,能根据编辑状态更新边的顶点数据。但请确保在 onChangeEdgeView 中妥善处理空值或异常情况。
151-159
: useEffect 依赖项完整
确保组件在 activeEditableEdge、editableLineMap 等状态更新时重新运行逻辑。无明显问题。
162-162
: 早期返回
在 activeEditableEdge 不存在时跳过后续逻辑,可提高可读性与性能。
175-175
: 清理事件监听器
在组件卸载或 effect 更新时正确移除 click 事件,可避免内存泄漏。
179-199
: 在 useMemo 中生成编辑线的路径
依赖清晰,包含 activeEditableEdge、lineEditorState、editableLineMap 等关键字段,可减少多余计算。无明显问题。bricks/diagram/src/draw-canvas/LineEditorComponent.tsx (11)
5-5
: 导入 ControlPoint、EdgeCell、EditableLine
这些类型有助于更好地处理边的可编辑性和控制点信息。实现无冲突。
18-18
: 在 props 中新增 editableLineMap
提供了可编辑线的映射关系,便于在组件内根据 EdgeCell 查找并管理对应的线信息。
23-23
: 新增 scale 与 editableLineMap
在组件解构参数时呈现缩放级别及可编辑线映射,可支持多层级缩放与灵活编辑。
25-26
: 使用上下文获取 activeEditableEdge
通过 HoverStateContext 获取当前激活的可编辑边,利于集中管理并在事件中处理编辑逻辑。
34-34
: 检查必需引用是否已存在
如 exitRef、entryRef 或 activeEditableEdge 不存在则提前返回,能避免事件绑定时出现异常。
57-57
: useEffect 依赖项
仅当 activeEditableEdge 等发生变化时才重新绑定事件,符合逻辑需求。
60-63
: 在 useMemo 中计算控制点
若边非直线,则生成控制点数组,否则返回空数组。逻辑明了。
66-66
: 在控制点事件中检查 activeEditableEdge
防止在不存在可编辑边时执行多余操作,有效规避空引用问题。
94-94
: useEffect 依赖项
对 controlPoints 与 activeEditableEdge 的变化实时监听,保证事件回调一致性。
109-109
: 在渲染时检查 activeEditableEdge
若无可编辑边则返回 null,避免渲染无用 DOM。
112-113
: 从 editableLineMap 中获取 linePoints
请确认 map.get(activeEditableEdge) 不会返回 undefined,否则对.points
的调用会出错。bricks/diagram/src/draw-canvas/EdgeComponent.tsx (6)
15-15
: 新引入的 EditableLine 类型
为管理可编辑线段提供更灵活的方式。无明显冲突。
32-32
: 对 EdgeComponentProps 增加 editableLineMap
通过映射可在 EdgeComponent 中获取对应的可编辑线信息。
40-40
: 在 EdgeComponent 中解构 editableLineMap
组件可直接使用外部传入的可编辑线映射,减少重复查询。
45-45
: 非空断言使用
请确认 lineConfMap 必然包含对应边的信息,否则需要额外的容错处理。
46-46
: 可选链降级
若 editableLineMap 命中失败则使用空对象,避免了对象解构报错。
51-51
: 三元运算符区分曲线类型
根据 lineConf.type 灵活使用 curveType 或 "curveLinear",简化代码并提升可读性。bricks/diagram/src/draw-canvas/CellComponent.tsx (12)
15-15
: 导入 EditableLine 类型
为 CellComponent 中对可编辑线的管理奠定类型基础。
47-47
: 在 CellComponentProps 中新增 editableLineMap
能够直接关联 EdgeCell 与可编辑线信息,结构更具可扩展性。
76-76
: 从 props 中解构 editableLineMap
可轻松在组件内部访问并操作可编辑线映射。
97-97
: 解构 activeEditableEdge 与 lineEditorState
便于在当前组件内对可编辑边执行相关交互逻辑。
197-197
: 在鼠标抬起事件中处理可编辑边
若不处于 smartConnectLineState,则根据 activeEditableEdge 更新当前连线视图。逻辑清晰可读。
198-198
: 从 lineEditorState 获取 type
根据不同的 type 决定后续操作,便于区分入口或控制点编辑。
199-199
: 从 editableLineMap 提取 source 与 target
需确认 editableLineMap 中已包含对应条目,避免潜在空引用。
205-205
: 更新 entryPosition
通过 onChangeEdgeView 置空 entryPosition,代表重置连线入口。
211-211
: 更新 exitPosition
同理清空 exitPosition,保留逻辑一致性。
226-226
: useEffect 依赖 activeEditableEdge
监听可编辑边变更,及时重绑鼠标事件,避免状态不同步。
227-227
: useEffect 中依赖 editableLineMap
当可编辑线映射变化时,同步更新事件逻辑,避免编辑状态失效。
309-309
: 向 EdgeComponent 传递 editableLineMap
统一由 CellComponent 负责传递可编辑线映射,增强组件间数据共享。bricks/diagram/src/draw-canvas/HoverStateContext.ts (3)
5-5
: 新增对 EdgeCell 的类型引用
用 EdgeCell 替换之前的可编辑线信息,集中管理边的编辑状态。
26-26
: 使用 activeEditableEdge 取代旧的 activeEditableLine
通过 EdgeCell 直观表示当前编辑对象,令上下文更简洁明了。
51-51
: 上下文默认值
将 activeEditableEdge 初始化为 null,确保初始状态可被安全访问。bricks/diagram/src/shared/canvas/useEditableLineMap.ts (3)
1-2
: 导入内容正常,无需额外调整。
导入语句简洁清晰,没有看到多余依赖或版本冲突的迹象。
22-23
: 建议校验依赖数组的完整性。
useMemo
仅依赖cells
和lineConfMap
,如果在其他地方会改变与数据结构或渲染逻辑相关的状态,可能需要一并纳入依赖,否则会出现缓存同步问题。
32-40
: 检查逆向连线的逻辑以确保不会产生重复边。
此段逻辑通过同时判断source === edge.target
和target === edge.source
来判断是否存在反向边。若部分上层逻辑允许多种变形/别名,需确保不会误判或遗漏。bricks/diagram/src/draw-canvas/SmartConnectLineComponent.tsx (1)
70-74
: 曲线类型选择逻辑清晰,但需留意扩展性。
透过lineSettings?.type
判断是否为 “curve”,再决定使用何种插值方式,这种写法简洁易懂。若后续需要更多自定义类型,可抽取逻辑统一管理。bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.ts (6)
5-8
: 新加类型导入提高可读性。
引入了EdgeCell
与EditableLine
,让后续函数的可维护性更好,类型信息更明确。
13-15
: 接口参数明确化,增强了可扩展性。
相比以往的函数签名,增加了activeEditableEdge
与editableLineMap
能更直接地取用相关数据。
20-20
: 对空或缺失参数的判定较完善。
若activeEditableEdge
未定义即返回null
,能防止出现空引用异常。
31-33
: 请验证editableLineMap.get
返回值是否可能为undefined
。
此处直接使用!
非空断言,若上游逻辑未能保证editableLineMap
一定存在对应项目,可能导致潜在错误。
39-44
: 把getNewLineVertices
与后续的getSmartLinePoints
分离处理,逻辑简洁清晰。
若需额外调整多段控制点,可继续在此函数中细化。整体结构可读性较好。
90-96
: 对控制点的索引修改需要注意越界或异常情况。
在getNewLineVertices
内部根据control.index
判定并修改相邻顶点时,若数组长度不足或点索引不在合理范围内,可能引发不可预期的渲染错误。bricks/diagram/src/shared/canvas/processors/getEditingLinePoints.spec.ts (3)
3-7
: 测试类型导入真实地匹配了生产代码逻辑。
通过添加Cell
,EdgeCell
等类型声明,让单测对核心数据结构的模拟更贴近实际配置。
41-43
: 对空值处理的测试用例非常重要。
当所有参数都为null
,能很好地覆盖函数的安全返回分支,可避免空引用导致的异常。
247-265
: 此测试覆盖「合并冗余顶点」的场景很有价值。
确认当相邻点位置相同且继续“拉动”时,被简化后的结果符合预期。 如果后期还需进一步合并多段同样坐标的其实顶点,也可考虑再添加测试。bricks/diagram/src/draw-canvas/interfaces.ts (3)
353-353
: 对端点编辑状态接口的定义没有明显问题
此接口的定义较为简洁,希望后续在使用时能与整体的编辑流程相匹配。
359-359
: 对控制点编辑状态接口的定义良好
与端点编辑状态区分清晰,暂未发现明显问题。
388-394
: 确认 parallelGap 是否应为可选属性
从上下文看,在BaseEdgeLineConf
中parallelGap
是可选项,这里声明为必填可能会导致类型不一致。建议确认业务逻辑是否总能提供parallelGap
。bricks/diagram/src/draw-canvas/LineConnectorComponent.tsx (10)
4-10
: 导入类型的改动无安全或功能性影响
这些导入声明看起来符合项目结构,没有发现问题。
23-23
: 新增 props 参数 editableLineMap
此属性能帮助组件获取可编辑连线的相关数据,逻辑看起来合理。
30-30
: 在函数参数中使用 editableLineMap
该属性已成功注入到组件内部,当前改动无异常。
39-39
: 对 activeEditableEdge、lineEditorState 的使用
这些新增依赖在组件内部使用合理,未发现潜在问题。
111-111
: 属性传递: editableLineMap
此处将属性继续下传,通过子组件访问相应数据,不存在明显问题。
126-126
: 新增 ConnectPointComponentProps 字段 editableLineMap
配合上层使用场景,此字段的用途明确,可帮助定位边的编辑信息。
135-135
: 在 ConnectPointComponent 的参数中使用 editableLineMap
传参保留了父组件上下文,符合组件间数据交互需求。
149-149
: 对 activeEditableEdge、lineEditorState 等依赖的使用
在事件处理及状态管理中使用这些新增属性,逻辑清晰无异常。
216-221
: 通过 editableLineMap 获取 source、target 并更新 EdgeView
此段逻辑能准确定位当前边并更新坐标,暂未发现明显问题。
246-248
: 在依赖列表中添加 activeEditableEdge、lineEditorState、editableLineMap
确保函数在这些状态变更时重新执行,符合预期。bricks/diagram/src/display-canvas/index.tsx (3)
46-46
: 从 useEditableLineMap 导入新 hook
此改动为新增功能,不涉及兼容性问题,逻辑正常。
398-398
: 使用 useEditableLineMap 创建 editableLineMap
直接与 cells、lineConfMap 结合生成映射,代码简洁明确。
435-435
: 通过 props 将 editableLineMap 传递给 CellComponent
有助于组件间共享可编辑连线信息,无明显问题。bricks/diagram/src/draw-canvas/index.tsx (7)
100-101
: 从 useEditableLineMap 与 targetIsActive 引入方法
这两个新导入可增强对边与目标激活状态的识别,合理无冲突。
1051-1063
: 使用 editableLineMap 与 activeTarget 联合计算 activeEditableEdge
该逻辑能在用户操作时动态确定可编辑边,代码良好。
1184-1184
: 在 hoverStateContextValue 中新增 activeEditableEdge
有利于在上下文中统一访问当前激活的可编辑边,保持状态一致性。
1290-1290
: 将 editableLineMap 传递给 CellComponent
便于子组件使用可编辑连线信息,无额外风险。
1336-1337
: 在 EditingLineComponent 中导入 cells 与 editableLineMap
将线编辑功能合并到同一组件,符合组件职责分离原则。
1355-1360
: 在 LineEditorComponent 中使用 editableLineMap
用以实现对线段的编辑操作,本改动可提高灵活度。
1365-1365
: 再次在 LineConnectorComponent 中注入 editableLineMap
与前文保持一致,保证组件间编辑逻辑的统一。
📐🤏 Size check result (ac76ace...6e6696c): Load all bricks together
Critical changes: None. See full changes
Load bricks by each packageCritical changes: None. See full changes
Load by each brickCritical changes: None. See full changes
|
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat
作为提交类型。BREAKING CHANGE: 你的变更说明
。新特性:
feat
作为提交类型。问题修复:
fix
作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore
,docs
,test
等作为提交类型。Summary by CodeRabbit
新功能
性能优化
useEditableLineMap
钩子提升了线条渲染性能用户体验改进