Skip to content
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

fix(): set default mount point to empty, refine data icon #395

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

weareoutman
Copy link
Member

@weareoutman weareoutman commented Oct 24, 2024

依赖检查

组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。

请勾选以下两组选项其中之一:

  • 本次 MR 没有使用上游组件(例如框架、后台组件等)的较新版本提供的特性。

或者:

  • 本次 MR 使用了上游组件(例如框架、后台组件等)的较新版本提供的特性。
  • 在对应的文件中更新了该上游组件的依赖版本(或确认了当前声明的依赖版本已包含本次 MR 使用的新特性)。

提交信息检查

Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。

破坏性变更是针对于下游使用者而言,可以通过本次改动对下游使用者的影响来识别变更类型:

  • 下游使用者不做任何改动,仍可以正常工作时,那么它属于普通变更。
  • 反之,下游使用者不做改动就无法正常工作时,那么它属于破坏性变更。

例如,构件修改了一个属性名,小产品 Storyboard 中需要使用新属性名才能工作,那么它就是破坏性变更。
又例如,构件还没有任何下游使用者,那么它的任何变更都是普通变更。

破坏性变更:

  • ⚠️ 本次 MR 包含破坏性变更的提交,请继续确认以下所有选项:
  • 没有更好的兼容方案,必须做破坏性变更。
  • 使用了 feat 作为提交类型。
  • 标注了 BREAKING CHANGE: 你的变更说明
  • 同时更新了本仓库中所有下游使用者的调用。
  • 同时更新了本仓库中所有下游使用者对该子包的依赖为即将发布的 major 版本。
  • 同时为其它仓库的 Migrating 做好了准备,例如文档或批量改动的方法。
  • 手动验证过破坏性变更在 Migrate 后可以正常工作。
  • 破坏性变更所在的提交没有意外携带其它子包的改动。

新特性:

  • 本次 MR 包含新特性的提交,且该提交不带有破坏性变更,并使用了 feat 作为提交类型。
  • 给新特性添加了单元测试。
  • 手动验证过新特性可以正常工作。

问题修复:

  • 本次 MR 包含问题修复的提交,且该提交不带有新特性或破坏性变更,并使用了 fix 作为提交类型。
  • 给问题修复添加了单元测试。
  • 手动验证过问题修复得到解决。

杂项工作:

即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:

  • 本次 MR 包含杂项工作的提交,且该提交不带有问题修复、新特性或破坏性变更,并使用了 chore, docs, test 等作为提交类型。

Summary by CodeRabbit

  • 新功能

    • 更新了工作台数据树中图标的表示方式,以更好地反映状态。
    • 改进了拖放交互的处理逻辑,增强了节点分类的准确性。
  • 测试

    • 更新了测试用例,反映新的图标表示和预期输出结构。

Copy link

coderabbitai bot commented Oct 24, 2024

Walkthrough

此次更改涉及三个主要文件,主要集中在拖放功能和节点数据处理的改进。getWorkbenchDataTree.ts 文件中更新了图标属性的赋值,将图标从 "link" 改为 "database"PreviewContainer.tsx 文件和 WorkbenchBrickTree.tsx 文件中对 mountPoint 变量的赋值逻辑进行了调整,以更好地处理拖放事件。整体结构和逻辑保持不变,但增强了对节点交互和数据处理的准确性。

Changes

文件路径 更改摘要
bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts 更新 getWorkbenchDataTree 函数中的 icon 属性赋值,将 item.resolve 条件下的图标从 "link" 改为 "database"
bricks/next-builder/src/preview-container/PreviewContainer.tsx 修改 handleOnDrop 方法中的 mountPoint 属性赋值逻辑,调整 handleOnDrop 函数以改善拖放事件的数据处理。
bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx 更新 handleOnDrop 函数中的 mountPoint 赋值逻辑,增加对 matchBrickNode 函数的检查以排除挂载点节点。
bricks/next-builder/src/custom-processors/getWorkbenchDataTree.spec.ts 更新测试用例以反映图标属性的更改,并为特定对象添加了 expose 属性。

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1e3dde2 and 1f375c5.

📒 Files selected for processing (4)
  • bricks/next-builder/src/custom-processors/getWorkbenchDataTree.spec.ts (2 hunks)
  • bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts (1 hunks)
  • bricks/next-builder/src/preview-container/PreviewContainer.tsx (1 hunks)
  • bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts
  • bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx
🔇 Additional comments (2)
bricks/next-builder/src/custom-processors/getWorkbenchDataTree.spec.ts (1)

70-70: 图标更新符合预期

测试用例中的图标从 "link" 更新为 "database" 的改动与实现代码保持一致,确保了测试覆盖的准确性。

执行以下脚本验证实现代码中的图标更新:

Also applies to: 89-89

✅ Verification successful

Let me gather more context about the icon changes to ensure a comprehensive verification.


测试用例中的图标更改已验证正确

测试用例中的图标更改完全符合实现代码的逻辑。具体验证结果:

  • 实现代码中,当 item.resolve 存在时,图标被设置为 database,包含完整的图标配置(lib、theme、color)
  • 测试用例中的两处更改(对象 "b" 和 "c")都正确反映了这一实现逻辑,包括完整的图标属性集
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 getWorkbenchDataTree.ts 中的图标更新

# 测试:搜索图标相关的改动
rg -A 5 'icon.*database' bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts

Length of output: 245


Script:

#!/bin/bash
# 搜索所有与图标相关的代码上下文
rg -B 5 -A 5 'icon.*link|icon.*database' bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts

# 查找测试文件中的相关断言
rg -B 5 -A 5 'icon.*link|icon.*database' bricks/next-builder/src/custom-processors/getWorkbenchDataTree.spec.ts

Length of output: 1037

bricks/next-builder/src/preview-container/PreviewContainer.tsx (1)

364-364: 请确保空字符串的 mountPoint 在其他部分被正确处理

当前在 dragStatus"inside" 时,将 mountPoint 设置为空字符串。请验证代码库中所有使用 mountPoint 的地方,确保能够正确处理空字符串,避免可能的异常或错误。

运行以下脚本以检查 mountPoint 的使用情况,验证是否正确处理了空字符串:

✅ Verification successful

Let me generate additional scripts to search for how empty mountPoint is handled in the codebase:


Let me try searching for more specific patterns around the usage of mountPoint in the codebase:


从搜索结果中,我们可以看到在 PreviewContainer.tsx 中,当 dragStatus === "inside" 时设置空字符串作为 mountPoint 是有意的设计。这是在拖拽操作中的一个特定场景处理,表示将元素拖入到目标容器内部而不是挂载到特定的挂载点。

让我们再搜索一下相关的处理逻辑:


基于搜索结果,我们可以看到在 PreviewContainer.tsx 中对空 mountPoint 的处理是有意的设计。当拖拽状态为 "inside" 时,将 mountPoint 设置为空字符串是正常的业务逻辑,表示将元素直接放置在目标容器内部,而不是挂载到特定的挂载点。

让我们再确认一下这个处理在其他相关组件中的使用:


空字符串的 mountPoint 处理是安全的

根据代码分析,在 PreviewContainer.tsx 中将 mountPoint 设置为空字符串是拖拽功能的一个特定设计。当 dragStatus === "inside" 时,表示将元素直接放置在目标容器内部,此时使用空字符串作为 mountPoint 是预期的行为。

这个处理逻辑具有以下特点:

  • 这是一个有条件的赋值,只在特定拖拽状态下发生
  • 空字符串的 mountPoint 仅用于拖拽操作的数据传递
  • 这个处理是在预览容器的上下文中进行的,不会影响到其他组件的正常运作

因此,这里使用空字符串是安全的,不需要额外的处理。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:查找所有使用 `mountPoint` 的地方,检查对空字符串的处理

# 搜索所有 `mountPoint` 的使用,包含赋值和条件判断
rg 'mountPoint' -A 5

# 搜索是否有对 `mountPoint` 为空字符串的判断
rg 'mountPoint\s*===?\s*""' -A 5

Length of output: 325666


Script:

#!/bin/bash
# Search for code that handles empty mountPoint
ast-grep --pattern 'if ($mountPoint === "") {
  $$$
}'

# Search for code that handles empty mountPoint with different syntax
ast-grep --pattern 'if (!$mountPoint) {
  $$$
}'

# Search for code that handles empty mountPoint with different syntax
ast-grep --pattern 'if ($mountPoint?.length === 0) {
  $$$
}'

# Search for code that handles empty mountPoint with different syntax
ast-grep --pattern 'if ($mountPoint == "") {
  $$$
}'

# Search for code that handles empty mountPoint with different syntax
rg 'if.*!mountPoint.*\{' -A 5

# Search for code that handles empty mountPoint with different syntax
rg 'if.*mountPoint\s*===?\s*"".*\{' -A 5

# Search for code that handles empty mountPoint with different syntax
rg 'if.*mountPoint\.length\s*===?\s*0.*\{' -A 5

Length of output: 352


Script:

#!/bin/bash

# Search for files that handle mountPoint in PreviewContainer.tsx
rg -A 10 'dragStatus === "inside"' bricks/next-builder/src/preview-container/PreviewContainer.tsx

# Search for how mountPoint is used in storyboard assembly
rg -A 10 'mountPoint.*=""' bricks/next-builder/src/shared/storyboard/

# Search for mountPoint related tests
rg -A 10 'mountPoint.*="".*test' 

# Search for any error handling related to mountPoint
rg -A 10 'mountPoint.*error' 

# Search for any validation of mountPoint
rg -A 10 'mountPoint.*valid'

Length of output: 915


Script:

#!/bin/bash

# 搜索 PreviewContainer 中处理拖拽的相关代码
rg -B 5 -A 15 'onPreviewerDrop' bricks/next-builder/src/preview-container/PreviewContainer.tsx

# 搜索处理空 mountPoint 的相关代码
rg -B 5 -A 10 'onDrop.*mountPoint' bricks/next-builder/src/

# 搜索相关的测试用例
rg -B 5 -A 10 'test.*mountPoint.*empty' bricks/next-builder/src/

Length of output: 3584


Script:

#!/bin/bash

# 搜索处理拖拽状态的相关代码
rg -B 5 -A 10 '"inside".*dragStatus' bricks/next-builder/src/

# 搜索处理空 mountPoint 的组件代码
rg -B 5 -A 10 'mountPoint.*"".*component' bricks/next-builder/src/

# 搜索相关的状态处理逻辑
rg -B 5 -A 10 'dragStatus.*mountPoint' bricks/next-builder/src/

Length of output: 1853


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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)
bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx (1)

Line range hint 428-445: 优化了节点匹配逻辑

通过添加 node.data.type !== "mount-point" 的类型检查,避免了对挂载点节点进行不必要的 deepMatch 调用,提高了性能和准确性。

建议考虑将类型检查提取为一个独立的辅助函数,以提高代码的可维护性:

+function isMountPointNode(node: WorkbenchBrickTreeNode): boolean {
+  return node.type === "mount-point";
+}

function matchBrickNode(
  node: WorkbenchNodeData<WorkbenchBrickTreeNode>,
  lowerTrimmedQuery?: string
): boolean {
  return (
-   node.data.type !== "mount-point" &&
+   !isMountPointNode(node.data) &&
    deepMatch(
      isCustomTemplateNode(node.data)
        ? {
            name: node.data.name,
            proxy: node.data.proxy,
            previewSettings: node.data.previewSettings,
          }
        : isSnippetNode(node.data)
        ? node.name
        : [node.name, node.data.$$normalized],
      lowerTrimmedQuery
    )
  );
}
bricks/next-builder/src/preview-container/PreviewContainer.tsx (3)

Line range hint 363-370: 建议优化拖放状态的处理逻辑

当前的拖放状态判断和 mountPoint 赋值逻辑可以进一步优化,建议:

  1. 使用枚举类型定义拖放状态
  2. 将状态判断逻辑抽取为独立函数
+enum DragStatus {
+  Inside = 'inside',
+  Top = 'top',
+  Bottom = 'bottom'
+}

+const getDragStatus = (direction: Direction): DragStatus => {
+  if (direction === "inside") return DragStatus.Inside;
+  if (["top", "left"].includes(direction)) return DragStatus.Top;
+  return DragStatus.Bottom;
+};

-const dragStatus =
-  direction === "inside"
-    ? "inside"
-    : ["top", "left"].includes(direction)
-    ? "top"
-    : ["right", "bottom"].includes(direction)
-    ? "bottom"
-    : "";
+const dragStatus = getDragStatus(direction);

Line range hint 365-370: 建议优化父节点获取逻辑

parentNodes 的映射操作可以使用更简洁的方式实现。

-parentNodes: parentNodes.map((node) =>
-  omit(node, ["parent", "children"])
-),
+parentNodes: parentNodes.map(({ parent, children, ...rest }) => rest),

Line range hint 32-38: 建议添加性能优化

getAllParentEdges 函数中,可以使用 Set 来避免重复的边缘节点。

const getAllParentEdges = (
  edge: BuilderRuntimeEdge,
- list: number[] = []
+ visited: Set<number> = new Set()
): number[] => {
- list.push(edge.parent);
+ visited.add(edge.parent);
  const nodeEdge = edges.find((item) => item.child === edge.parent);
  if (nodeEdge) {
-   list = list.concat(getAllParentEdges(nodeEdge));
+   getAllParentEdges(nodeEdge, visited);
  }
- return list;
+ return Array.from(visited);
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 87f95b6 and 1e3dde2.

📒 Files selected for processing (3)
  • bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts (1 hunks)
  • bricks/next-builder/src/preview-container/PreviewContainer.tsx (1 hunks)
  • bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx (1 hunks)
🔇 Additional comments (3)
bricks/next-builder/src/custom-processors/getWorkbenchDataTree.ts (1)

73-73: 图标更改看起来合理,建议确认设计规范

图标从 "link" 改为 "database" 在语义上更准确地表达了解析(resolve)操作可能涉及数据库查询的含义。

建议运行以下脚本验证项目中图标使用的一致性:

✅ Verification successful

Let me gather more context about the usage of database icons and related components to ensure consistency.


根据搜索结果,我可以生成最终的验证结论了。

图标使用符合设计规范,无需调整

  • database 图标在项目中主要用于以下场景:
    • WorkbenchBrickTree 中用于数据相关节点
    • 在事件编辑器中用于 UseProvider 类型的处理器
    • 在工作台数据树中用于带有 resolve 属性的节点

这些使用场景都与数据获取和处理相关,显示为数据库图标符合直觉和设计语义。原来使用的 link 图标主要用于链接和跳转场景,改为 database 更准确地表达了数据操作的含义。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查项目中数据库相关操作是否统一使用 database 图标

# 测试:搜索其他使用 database 图标的地方
rg '"database"' --type ts

Length of output: 582


Script:

#!/bin/bash
# 检查 HandlerType.UseProvider 的定义和使用场景
rg "HandlerType.UseProvider" -A 2 -B 2

# 检查 WorkbenchBrickTree 中数据库图标的使用上下文
ast-grep --pattern 'icon = "database"'

# 检查是否有其他数据相关的图标使用
rg '"link"|"data"|"table"' --type ts

Length of output: 44398

bricks/next-builder/src/workbench-brick-tree/WorkbenchBrickTree.tsx (1)

325-325: 将 mountPoint 的默认值设置为空字符串

这个改动使得挂载点的处理更加灵活。空字符串作为默认值比硬编码的 "content" 更合适,并且与 PreviewContainer.tsx 中的变更保持一致。

让我们验证这个改动是否与其他相关文件保持一致:

✅ Verification successful

Let's gather more information about the mountPoint usage across the codebase to verify this change properly.


从搜索结果中我们可以看到关于 mountPoint 的关键信息:

  1. 在 PreviewContainer.tsx 中,当 dragStatus 为 "inside" 时,mountPoint 被设置为空字符串
  2. 在 buildStoryboardV2.ts 中,mountPoint 的默认值也是空字符串
  3. 这种处理方式在整个代码库中是一致的模式

让我们再确认一下这个改动的影响:


将 mountPoint 设置为空字符串是正确且一致的做法

从代码分析中可以看到:

  1. 在 PreviewContainer.tsx 中,当 dragStatus 为 "inside" 时,mountPoint 被设置为空字符串是有意为之的设计。
  2. 在 buildStoryboardV2.ts 中,mountPoint 的默认值也是空字符串。
  3. 这种处理方式在整个代码库中保持一致,表明这是一个标准的模式。

因此,将 mountPoint 从 "content" 改为空字符串的修改是合理的,它与现有代码库的处理逻辑保持一致。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查其他文件中的 mountPoint 赋值逻辑
# 预期:相关文件中的 mountPoint 赋值逻辑应该与此处保持一致

rg -A 3 'mountPoint.*=.*"content"'

Length of output: 36


Script:

#!/bin/bash
# 搜索所有包含 mountPoint 的相关代码
rg -A 3 'mountPoint.*=' 

# 特别关注 PreviewContainer.tsx 中的相关逻辑
fd -e tsx -e ts PreviewContainer --exec cat {}

# 查找可能的挂载点相关配置或常量定义
rg -A 3 'MOUNT_POINT|mountPoints?' 

Length of output: 292083


Script:

#!/bin/bash
# 检查 dragStatus 为 "inside" 时的 mountPoint 处理逻辑
rg -A 3 'dragStatus.*=.*"inside".*mountPoint'

# 检查默认的 mountPoint 赋值
rg -A 3 'mountPoint.*=.*""'

Length of output: 1427

bricks/next-builder/src/preview-container/PreviewContainer.tsx (1)

364-364: 修改了拖放操作中的挂载点逻辑

当拖放状态为 "inside" 时,将 mountPoint 设置为空字符串而不是使用 hoverEdge.mountPoint,这样可以避免不必要的嵌套层级。

@qiaofengxi qiaofengxi merged commit 4ec923a into master Oct 24, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants