feat: support file dialog renderCustomMsg #4352
Conversation
Walkthrough此次 PR 对 Changes
Sequence Diagram(s)sequenceDiagram
participant FD as FileDialog
participant FS as FileDialogService
FD->>FS: getDefaultFilePath(defaultPath)
FS-->>FD: 返回处理后的路径
FD->>FS: renderCustomMsg()
FS-->>FD: 返回自定义消息或 null
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
yarn install v1.22.22 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/overlay/src/common/index.ts (1)
60-60: 建议为新方法添加文档注释建议为
renderWarningMsg方法添加 JSDoc 文档注释，说明该方法的用途、返回值含义以及使用场景。这将有助于其他开发者理解和正确使用该方法。+ /** + * 渲染警告消息 + * @returns 返回要渲染的警告消息节点，如果没有警告消息则返回 null + */ renderWarningMsg(): React.ReactNode | null;packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (2)
214-218: 实现逻辑正确，但建议优化依赖项
renderWarningMsg函数的实现逻辑正确，通过安全检查确保方法存在才调用。建议将
dialogService添加到useCallback的依赖数组中：const renderWarningMsg = useCallback(() => { if (dialogService.renderWarningMsg) { return dialogService.renderWarningMsg(); } - }, []); + }, [dialogService]);
297-306: 建议更新 useMemo 依赖项DialogContent 组件使用了
renderWarningMsg函数，但未将其添加到useMemo的依赖数组中。const DialogContent = useMemo( () => ( <> <div>{renderWarningMsg()}</div> <div className={styles.file_dialog_directory}>{renderDirectorySelection()}</div> <div className={styles.file_dialog_content}>{renderDialogTree()}</div> </> ), - [renderDirectorySelection, renderDialogTree], + [renderDirectorySelection, renderDialogTree, renderWarningMsg], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(2 hunks)packages/overlay/src/common/index.ts(1 hunks)
Walkthrough此次更改在文件对话框组件中引入了新的 Changes
Sequence Diagram(s)sequenceDiagram
participant FD as FileDialog
participant DS as DialogService
FD->>DS: 检查是否实现 renderWarningMsg 方法
DS-->>FD: 返回警告消息 (React.ReactNode 或 null)
FD->>FD: 在对话框内容中渲染警告消息
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
305-305: 需要更新 useMemo 的依赖数组DialogContent 的 useMemo 依赖数组中缺少
renderWarningMsg，这可能导致警告消息不能及时更新。- [renderDirectorySelection, renderDialogTree], + [renderDirectorySelection, renderDialogTree, renderWarningMsg],
🧹 Nitpick comments (2)
packages/overlay/src/common/index.ts (1)
60-60: 接口定义清晰且符合预期新增的
renderWarningMsg方法定义合理，返回类型React.ReactNode | null能够满足不同场景的渲染需求。建议考虑添加 JSDoc 注释来说明该方法的用途和使用场景。
+ /** 渲染警告信息，当无需显示警告时返回 null */ renderWarningMsg(): React.ReactNode | null;packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
300-300: 建议优化警告消息的渲染逻辑当没有警告消息时，当前实现仍会创建一个空的 div 元素。建议使用条件渲染来优化。
- <div>{renderWarningMsg()}</div> + {renderWarningMsg() && <div>{renderWarningMsg()}</div>}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(2 hunks)packages/overlay/src/common/index.ts(1 hunks)
🔇 Additional comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
214-218: 安全地实现了警告消息渲染实现使用了
useCallback并正确地进行了空值检查，符合 React 最佳实践。
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx
Outdated
Show resolved
Hide resolved
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx
Outdated
Show resolved
Hide resolved
Ricbet
changed the title
|
/next |
|
🎉 PR Next publish successful! 3.7.1-next-1739252693.0 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4352 +/- ##
==========================================
- Coverage 54.19% 54.17% -0.02%
==========================================
Files 1635 1639 +4
Lines 99973 100306 +333
Branches 21703 21761 +58
==========================================
+ Hits 54177 54341 +164
- Misses 38049 38196 +147
- Partials 7747 7769 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
277-277: 建议简化布尔表达式当前的三元表达式可以简化为更清晰的形式。
建议修改为：
- showSearch={fileService.showFilePathSearch === false ? false : true} + showSearch={fileService.showFilePathSearch !== false}packages/main-layout/src/browser/accordion/styles.module.less (1)
83-91: 建议优化样式复用kt_split_warning_message 类与 kt_split_panel_message 类存在大量重复样式，建议重构以提高可维护性。
建议修改为:
.kt_split_panel_message { padding: 4px 12px 4px 18px; line-height: 14px; user-select: text; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .kt_split_warning_message { - padding: 4px 12px 4px 18px; - line-height: 14px; - user-select: text; - overflow: hidden; - text-overflow: ellipsis; + &:extend(.kt_split_panel_message); white-space: normal; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/file-tree-next/src/browser/dialog/file-dialog.service.ts(2 hunks)packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(5 hunks)packages/main-layout/src/browser/accordion/accordion.service.ts(1 hunks)packages/main-layout/src/browser/accordion/section.view.tsx(1 hunks)packages/main-layout/src/browser/accordion/styles.module.less(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-tree-next/src/browser/dialog/file-dialog.service.ts
🔇 Additional comments (3)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (2)
98-106: 代码结构清晰，实现合理！函数实现了良好的关注点分离，将获取默认路径的逻辑独立出来，并正确处理了可选的
fileService.getDefaultFilePath方法。
226-232: 很好地处理了空值情况！函数实现符合之前的代码评审建议，通过返回
null来避免渲染空标签。同时使用了简洁的条件返回结构，代码清晰易读。packages/main-layout/src/browser/accordion/section.view.tsx (1)
215-217: 代码实现清晰且符合最佳实践！警告消息的条件渲染实现得当:
- 正确放置在 ErrorBoundary 内部
- 与现有消息保持一致的渲染模式
- 使用统一的样式类
packages/main-layout/src/browser/accordion/accordion.service.ts
Outdated
Show resolved
Hide resolved
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
316-325: 🛠️ Refactor suggestion添加缺失的 useMemo 依赖
DialogContent的 useMemo 依赖数组中缺少renderWarningMsg，这可能导致警告消息无法及时更新。), - [renderDirectorySelection, renderDialogTree], + [renderDirectorySelection, renderDialogTree, renderWarningMsg], );
🧹 Nitpick comments (3)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (3)
98-106: 优化getDefaultPath方法的实现建议进行以下改进：
- 使用
useCallback包装函数以避免不必要的重新渲染- 添加类型注解以提高代码可维护性
- 添加空值检查以提高代码健壮性
- const getDefaultPath = (model) => { + const getDefaultPath = useCallback((model: FileTreeDialogModel) => { + if (!model?.treeModel?.root) { + return ''; + } + let defaultPath = (model.treeModel.root as Directory).uri.codeUri.fsPath; if (fileService.getDefaultFilePath) { defaultPath = fileService.getDefaultFilePath(defaultPath); } return defaultPath; - }; + }, [fileService]);
226-232: 优化renderWarningMsg的依赖数组当前的依赖数组仅包含
fileService.renderWarningMsg，建议将整个fileService添加到依赖数组中，以确保当fileService发生变化时能够正确触发重新渲染。}, [fileService.renderWarningMsg]); + }, [fileService]);
267-270: 简化showFilePathSearch的实现当前实现使用双重否定来确保布尔值，可以简化为更直观的形式。
const showFilePathSearch = useMemo( - () => (fileService.showFilePathSearch === false ? false : true), + () => fileService.showFilePathSearch !== false, [fileService.showFilePathSearch], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(6 hunks)
🔇 Additional comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
282-282: 代码实现正确
showSearch属性的设置符合需求，实现清晰明了。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (2)
98-106: 建议使用 useCallback 包装 getDefaultPath 函数该函数被用作
ensureIsReady的依赖，应该使用 useCallback 进行记忆化以避免不必要的重渲染。同时建议添加错误处理。建议按如下方式重构:
- const getDefaultPath = (model) => { + const getDefaultPath = useCallback((model) => { + if (!model?.treeModel?.root) { + return ''; + } + let defaultPath = (model.treeModel.root as Directory).uri.codeUri.fsPath; if (fileService.getDefaultFilePath) { defaultPath = fileService.getDefaultFilePath(defaultPath); } return defaultPath; - }; + }, [fileService]);
267-270: 建议简化 showFilePathSearch 的逻辑当前实现可以更简洁地表达默认值逻辑。
建议按如下方式重构:
- const showFilePathSearch = useMemo( - () => (fileService.showFilePathSearch === false ? false : true), - [fileService.showFilePathSearch], - ); + const showFilePathSearch = useMemo( + () => fileService.showFilePathSearch !== false, + [fileService.showFilePathSearch], + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/file-tree-next/src/browser/dialog/file-dialog.service.ts(2 hunks)packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-tree-next/src/browser/dialog/file-dialog.service.ts
🔇 Additional comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
226-232: 实现合理，符合之前的代码评审建议该实现正确处理了空值检查，并使用了 useCallback 进行优化。
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (1)
98-106: 建议使用 useCallback 包装 getDefaultPath 函数为了保持一致性和优化性能，建议将 getDefaultPath 函数使用 useCallback 包装。
- const getDefaultPath = (model) => { + const getDefaultPath = useCallback((model) => { let defaultPath = (model.treeModel.root as Directory).uri.codeUri.fsPath; if (fileService.getDefaultFilePath) { defaultPath = fileService.getDefaultFilePath(defaultPath); } return defaultPath; - }; + }, [fileService.getDefaultFilePath]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/file-tree-next/src/browser/dialog/file-dialog.service.ts(3 hunks)packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-tree-next/src/browser/dialog/file-dialog.service.ts
🔇 Additional comments (4)
packages/file-tree-next/src/browser/dialog/file-dialog.view.tsx (4)
226-232: renderCustomMsg 函数实现正确函数实现了正确的空值检查，并且使用了 useCallback 进行了适当的性能优化。
267-270: showFilePathSearch 的默认值处理合理使用 useMemo 缓存计算结果，并且对 undefined 情况做了合理的默认值处理。
282-282: Select 组件的 showSearch 属性集成正确showSearch 属性与 showFilePathSearch 状态正确关联。
319-319: 建议添加空值检查为了避免渲染空标签，建议添加空值检查。
- {renderCustomMsg()} + {renderCustomMsg() && renderCustomMsg()}
Ricbet
changed the title
|
/next |
|
🎉 PR Next publish successful! 3.7.1-next-1739439717.0 |
Types
Background or solution
Changelog
Summary by CodeRabbit