feat: make terminal draggable #4340
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
|
Walkthrough本次更改涉及多个模块的布局和交互逻辑调整。更新内容涵盖独立CSS样式的新增、组件属性及签名的调整、拖放事件与逻辑的增强以及布局服务的扩展。其中新增拖拽支持、标题栏和容器自定义配置，以及终端组件的渲染和事件处理的优化，共同提升了前端布局的模块化和交互能力。 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 用户
participant TB as TabbarService
participant LS as LayoutService
participant DA as DropArea
U->>TB: 开始拖拽(containerId)
TB->>LS: showDropAreaForContainer(containerId)
U->>DA: 拖动至目标区域
DA->>LS: moveContainerTo(containerId, 目标位置)
U->>TB: 触发onDragEnd事件
TB->>LS: hideDropArea()
Suggested labels
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 ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
packages/main-layout/src/browser/drop-area/drop-area.tsx (3)
8-10: 接口定义需要更严格的类型约束建议将 location 的类型从 string 改为更具体的联合类型，以避免传入无效的位置值。
interface IDropAreaProps { - location: string; + location: 'right' | 'bottom'; }
27-27: 建议使用国际化文本"drop here" 文本应该使用国际化处理，以支持多语言。
- drop here + {localize('dropArea.dropHere', 'Drop here')}
32-34: 组件导出方式可以优化建议将这两个组件合并为一个更通用的组件，通过配置对象导出，以提高代码的可维护性。
-export const RightDropArea = () => <DropArea location='right' />; -export const BottomDropArea = () => <DropArea location='bottom' />; +export const DropAreas = { + Right: () => <DropArea location='right' />, + Bottom: () => <DropArea location='bottom' /> +};packages/main-layout/src/browser/default-config.ts (2)
4-4: 建议优化导入路径从 '../common' 导入常量可能会导致模块依赖不清晰，建议使用完整的模块路径。
-import { DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER } from '../common'; +import { DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER } from '@opensumi/ide-main-layout/lib/common/constants';
24-24: 配置结构需要文档说明新增的拖放容器配置缺少注释说明其用途和影响。建议添加 JSDoc 注释来解释这些配置的作用。
[SlotLocation.right]: { + // 右侧拖放区域配置 modules: [DROP_RIGHT_CONTAINER], }, ... [SlotLocation.bottom]: { + // 底部拖放区域配置，保持终端等模块在拖放区域之后 modules: [ DROP_BOTTOM_CONTAINER, '@opensumi/ide-terminal-next', '@opensumi/ide-output', 'debug-console', '@opensumi/ide-markers', ],Also applies to: 30-36
packages/terminal-next/src/browser/contribution/terminal.view.ts (1)
59-59: 终端拖拽功能配置需要条件判断建议根据环境或配置来决定是否启用拖拽功能，而不是直接硬编码为 true。
- draggable: true, + draggable: this.preferenceService.get('terminal.enableDrag', true),packages/core-browser/src/layout/layout.interface.ts (1)
82-82: 新增容器拖拽配置选项通过在
ExtViewContainerOptions接口中添加draggable属性，使容器的拖拽行为可配置。建议为该属性添加 JSDoc 注释，说明其用途。+ /** 是否允许拖拽该容器 */ draggable?: boolean;packages/main-layout/src/common/main-layout.definition.ts (2)
89-92: 新增拖拽相关的服务方法为
IMainLayoutService接口添加了拖拽功能所需的核心方法。建议为这些方法添加 JSDoc 注释，说明其用途和参数含义。+ /** 将指定容器移动到目标位置 + * @param containerId 要移动的容器ID + * @param to 目标位置 + */ moveContainerTo(containerId: string, to: string): void; + /** 显示指定容器的放置区域 + * @param containerId 容器ID + */ showDropAreaForContainer(containerId: string): void; + /** 隐藏放置区域 */ hideDropArea(): void; + /** 根据容器ID查找对应的TabbarService + * @param containerId 容器ID + * @returns 对应的TabbarService实例，如果未找到则返回undefined + */ findTabbarServiceByContainerId(containerId: string): TabbarService | undefined;
136-137: 添加放置区域容器常量定义了底部和右侧放置区域的容器标识符常量。建议添加注释说明这些常量的用途。
+/** 底部放置区域容器ID */ export const DROP_BOTTOM_CONTAINER = 'drop-bottom'; +/** 右侧放置区域容器ID */ export const DROP_RIGHT_CONTAINER = 'drop-right';packages/ai-native/src/browser/layout/tabbar.view.tsx (1)
177-186: 优化标题栏的渲染结构通过
customTitleBar属性将标题栏的渲染逻辑抽离出来，使代码结构更清晰。建议考虑将标题栏组件抽离为独立组件，以提高复用性。+const TitleBar: React.FC<{ + title?: string; + onClose?: () => void; +}> = ({ title, onClose }) => ( + <div className={styles.header}> + <span className={styles.title}>{title}</span> + <div className={styles.side}> + <EnhancePopover id={'ai_right_panel_header_close'} title={localize('editor.title.context.close')}> + <EnhanceIcon icon='close' onClick={onClose} /> + </EnhancePopover> + </div> + </div> +); <ContainerView {...props} customTitleBar={ - <div className={styles.header}> - <span className={styles.title}>{options && options.title}</span> - <div className={styles.side}> - <EnhancePopover id={'ai_right_panel_header_close'} title={localize('editor.title.context.close')}> - <EnhanceIcon icon='close' onClick={handleClose} /> - </EnhancePopover> - </div> - </div> + <TitleBar title={options?.title} onClose={handleClose} /> } />🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/main-layout/src/browser/tabbar/panel.view.tsx (1)
96-105: 建议添加类型定义以增强类型安全性组件签名的更改看起来不错。为了进一步增强类型安全性，建议:
- 为
customTitleBar添加具体的类型定义，而不是使用React.ReactNode- 为
draggable选项添加类型定义到ComponentRegistryInfo接口中packages/main-layout/src/browser/main-layout.contribution.ts (1)
198-209: 建议提取公共配置以提高可维护性当前实现正确但可以通过以下方式优化:
- 提取公共配置到常量
- 使用工厂函数处理相似的注册逻辑
+const DROP_AREA_CONFIG = { + hideTab: true, +}; + +const createDropAreaOptions = (id: string, component: React.FC) => ({ + ...DROP_AREA_CONFIG, + component, + containerId: id, +}); + registerComponent(registry: ComponentRegistry): void { - registry.register(DROP_RIGHT_CONTAINER, [], { - component: RightDropArea, - hideTab: true, - containerId: DROP_RIGHT_CONTAINER, - }); - registry.register(DROP_BOTTOM_CONTAINER, [], { - component: BottomDropArea, - hideTab: true, - containerId: DROP_BOTTOM_CONTAINER, - }); + registry.register(DROP_RIGHT_CONTAINER, [], createDropAreaOptions(DROP_RIGHT_CONTAINER, RightDropArea)); + registry.register(DROP_BOTTOM_CONTAINER, [], createDropAreaOptions(DROP_BOTTOM_CONTAINER, BottomDropArea)); }packages/main-layout/src/browser/tabbar/bar.view.tsx (1)
171-173: 建议添加错误处理为了提高鲁棒性，建议在 onDragEnd 处理器中添加错误处理:
onDragEnd={(e) => { + try { tabbarService.handleDragEnd(e); + } catch (error) { + console.error('处理拖拽结束事件时发生错误:', error); + } }}packages/main-layout/src/browser/layout.service.ts (3)
229-239: 建议优化查找性能当前实现可以通过使用 Array.find 来优化:
findTabbarServiceByContainerId(containerId: string): TabbarService | undefined { - let tabbarService: undefined | TabbarService; - for (const value of this.tabbarServices.values()) { - if (value.containersMap.has(containerId)) { - tabbarService = value; - break; - } - } - return tabbarService; + return Array.from(this.tabbarServices.values()).find((service) => + service.containersMap.has(containerId) + ); }
282-291: 建议增强空值检查和状态管理当前实现在隐藏放置区域时可以更健壮。建议:
- 添加服务存在性检查
- 使用可选链操作符
- 添加状态转换日志
hideDropArea(): void { const bottomService = this.tabbarServices.get('bottom'); const rightService = this.tabbarServices.get('right'); + + if (!bottomService && !rightService) { + this.logger.warn('No services to hide drop areas'); + return; + } + if (bottomService?.currentContainerId.get() === DROP_BOTTOM_CONTAINER) { bottomService.updateCurrentContainerId(bottomService.previousContainerId || ''); + this.logger.debug('Hidden bottom drop area'); } if (rightService?.currentContainerId.get() === DROP_RIGHT_CONTAINER) { rightService.updateCurrentContainerId(rightService.previousContainerId || ''); + this.logger.debug('Hidden right drop area'); } }
328-347: 建议优化查找逻辑当前实现可以通过以下方式优化:
- 使用数组方法简化逻辑
- 缓存常用值
- 提前返回优化
private findNonDropContainerId(tabbarService: TabbarService): string { + const dropContainers = [DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER]; + const currentContainerId = tabbarService.currentContainerId.get(); - if (currentContainerId && ![DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER].includes(currentContainerId as string)) { + if (currentContainerId && !dropContainers.includes(currentContainerId as string)) { return currentContainerId; } - if ( - tabbarService.previousContainerId && - ![DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER].includes(tabbarService.previousContainerId as string) - ) { + + const { previousContainerId } = tabbarService; + if (previousContainerId && !dropContainers.includes(previousContainerId)) { return tabbarService.previousContainerId; } - for (const key of tabbarService.containersMap.keys()) { - if (![DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER].includes(key as string)) { - return key; - } - } - return ''; + + return Array.from(tabbarService.containersMap.keys()) + .find(key => !dropContainers.includes(key as string)) || ''; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/ai-native/src/browser/layout/layout.module.less(1 hunks)packages/ai-native/src/browser/layout/tabbar.view.tsx(1 hunks)packages/core-browser/src/layout/layout.interface.ts(1 hunks)packages/main-layout/src/browser/accordion/titlebar.view.tsx(1 hunks)packages/main-layout/src/browser/default-config.ts(2 hunks)packages/main-layout/src/browser/drop-area/drop-area.tsx(1 hunks)packages/main-layout/src/browser/drop-area/styles.module.less(1 hunks)packages/main-layout/src/browser/layout.service.ts(3 hunks)packages/main-layout/src/browser/main-layout.contribution.ts(3 hunks)packages/main-layout/src/browser/tabbar/bar.view.tsx(1 hunks)packages/main-layout/src/browser/tabbar/panel.view.tsx(2 hunks)packages/main-layout/src/browser/tabbar/tabbar.service.ts(3 hunks)packages/main-layout/src/common/main-layout.definition.ts(2 hunks)packages/terminal-next/src/browser/component/terminal.module.less(2 hunks)packages/terminal-next/src/browser/component/terminal.widget.tsx(2 hunks)packages/terminal-next/src/browser/contribution/terminal.view.ts(1 hunks)packages/terminal-next/src/browser/terminal.controller.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/main-layout/src/browser/drop-area/styles.module.less
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ai-native/src/browser/layout/tabbar.view.tsx
[error] 179-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
packages/terminal-next/src/browser/component/terminal.widget.tsx (2)
70-79: 优化了终端组件的元素处理逻辑改进后的代码更加健壮，通过先检查 widget.element 是否存在来决定是否需要创建新元素。这种方式可以避免不必要的 DOM 操作。
99-99: 样式类名更新更符合语义将
terminalContent更改为terminalContentWrapper更好地表达了该元素作为包装器的作用。packages/main-layout/src/browser/tabbar/panel.view.tsx (1)
148-155: 实现正确且符合预期TitleBar 组件正确接收并使用了 draggable 属性，实现了可拖拽的标题栏功能。
packages/main-layout/src/browser/main-layout.contribution.ts (1)
121-128: 类扩展合理且符合架构设计通过添加 ComponentContribution 到 @Domain 装饰器，正确扩展了类的功能以支持组件注册。
packages/terminal-next/src/browser/terminal.controller.ts (1)
200-202: 将 _tabBarHandler 重构为 getter 方法这个改动提高了代码的可维护性和可靠性：
- 每次访问都能获取最新的 handler
- 避免了状态存储，减少了潜在的状态不同步问题
- 符合按需获取的设计原则
packages/main-layout/src/browser/tabbar/tabbar.service.ts (3)
681-683: 实现拖拽结束时清理 drop area确保在拖拽结束后清理 UI，保持界面整洁。
699-704: 实现容器移除功能合理的容器清理实现：
- 清理相关资源
- 更新当前容器 ID
- 触发视图更新
706-710: 实现动态添加容器功能完整的容器添加流程：
- 注册新容器
- 更新当前容器 ID
- 触发视图更新
packages/terminal-next/src/browser/component/terminal.module.less (1)
42-45: 添加终端内容包装器样式新增的样式类确保终端内容能够正确填充其容器空间。
packages/ai-native/src/browser/layout/layout.module.less (1)
122-129: 添加标准化的头部样式新增的头部样式提供了统一的外观和布局：
- 固定高度确保一致性
- Flex 布局支持两端对齐
- 使用系统变量保持主题一致性
|
@Marckon 感谢 pr。麻烦签署一下 CLA |
Ricbet
reviewed
Feb 5, 2025
erha19
requested changes
Feb 5, 2025
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/main-layout/src/browser/accordion/titlebar.view.tsx (1)
20-32: 🛠️ Refactor suggestion拖拽事件处理需要增加错误处理
建议添加 try-catch 以防止未捕获的异常影响用户体验。
const handleDragStart = React.useCallback( (e: React.DragEvent) => { + try { tabbarService.handleDragStart(e, props.containerId); + } catch (error) { + console.error('拖拽开始失败:', error); + } }, [tabbarService], ); const handleDragEnd = React.useCallback( (e: React.DragEvent) => { + try { tabbarService.handleDragEnd(e); + } catch (error) { + console.error('拖拽结束失败:', error); + } }, [tabbarService], );
🧹 Nitpick comments (8)
packages/main-layout/src/browser/accordion/titlebar.view.tsx (2)
13-15: Props 类型定义需要完善建议为新增的属性添加类型注释以提高代码可维护性。
- draggable?: boolean; - side: string; - containerId: string; + /** 是否支持拖拽 */ + draggable?: boolean; + /** 容器位置：'left' | 'right' | 'bottom' */ + side: 'left' | 'right' | 'bottom'; + /** 容器唯一标识 */ + containerId: string;
36-41: 建议增加拖拽相关的无障碍支持为了提升无障碍性，建议添加 aria 属性和键盘事件支持。
- {!!props.draggable && ( - <h1 draggable style={{ cursor: 'pointer' }} onDragStart={handleDragStart} onDragEnd={handleDragEnd}> + {!!props.draggable && ( + <h1 + draggable + style={{ cursor: 'pointer' }} + onDragStart={handleDragStart} + onDragEnd={handleDragEnd} + aria-grabbed={false} + role="application" + aria-label={`可拖拽的标题栏: ${props.title}`} + > {props.title} </h1> )}packages/main-layout/src/browser/layout.service.ts (6)
235-245: 建议优化 findTabbarServiceByContainerId 方法的实现当前实现遍历所有 TabbarService 直到找到匹配项。可以使用 Array.prototype.find 方法使代码更简洁。
findTabbarServiceByContainerId(containerId: string): TabbarService | undefined { - let tabbarService: undefined | TabbarService; - for (const value of this.tabbarServices.values()) { - if (value.containersMap.has(containerId)) { - tabbarService = value; - break; - } - } - - return tabbarService; + return Array.from(this.tabbarServices.values()).find( + (service) => service.containersMap.has(containerId) + ); }
272-286: 建议优化代码结构和类型安全性
- 使用 early return 模式提高代码可读性
- 使用 SlotLocation 枚举替代字符串字面量
- 添加类型守卫
showDropAreaForContainer(containerId: string): void { const tabbarService = this.findTabbarServiceByContainerId(containerId); - const bottomService = this.tabbarServices.get('bottom'); - const rightService = this.tabbarServices.get('right'); - if (!tabbarService) { - this.logger.error(`cannot find container: ${containerId}`); - return; - } - if (tabbarService?.location === 'right') { - bottomService?.updateCurrentContainerId(DROP_BOTTOM_CONTAINER); - } - if (tabbarService?.location === 'bottom') { - rightService?.updateCurrentContainerId(DROP_RIGHT_CONTAINER); - } + if (!tabbarService) { + this.logger.error(`cannot find container: ${containerId}`); + return; + } + + const bottomService = this.tabbarServices.get(SlotLocation.bottom); + const rightService = this.tabbarServices.get(SlotLocation.right); + + switch (tabbarService.location) { + case SlotLocation.right: + bottomService?.updateCurrentContainerId(DROP_BOTTOM_CONTAINER); + break; + case SlotLocation.bottom: + rightService?.updateCurrentContainerId(DROP_RIGHT_CONTAINER); + break; + } }
288-297: 建议优化代码结构和复用逻辑可以将重复的逻辑抽取为一个辅助方法，并使用 SlotLocation 枚举。
+ private hideDropAreaForService(service: TabbarService | undefined, dropContainerId: string): void { + if (service?.currentContainerId.get() === dropContainerId) { + service.updateCurrentContainerId(service.previousContainerId || ''); + } + } + hideDropArea(): void { - const bottomService = this.tabbarServices.get('bottom'); - const rightService = this.tabbarServices.get('right'); - if (bottomService?.currentContainerId.get() === DROP_BOTTOM_CONTAINER) { - bottomService.updateCurrentContainerId(bottomService.previousContainerId || ''); - } - if (rightService?.currentContainerId.get() === DROP_RIGHT_CONTAINER) { - rightService.updateCurrentContainerId(rightService.previousContainerId || ''); - } + this.hideDropAreaForService( + this.tabbarServices.get(SlotLocation.bottom), + DROP_BOTTOM_CONTAINER + ); + this.hideDropAreaForService( + this.tabbarServices.get(SlotLocation.right), + DROP_RIGHT_CONTAINER + ); }
334-336: 建议优化性能和可维护性
- 使用 Set 代替数组以提高查找性能
- 将常量集合提升为类成员以便重用
+ private readonly DROP_CONTAINERS = new Set([ + DROP_BOTTOM_CONTAINER, + DROP_RIGHT_CONTAINER + ]); + private isDropContainer(containerId: string): boolean { - return [DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER].includes(containerId); + return this.DROP_CONTAINERS.has(containerId); }
338-354: 建议优化代码结构和性能使用 Array 方法简化代码，提高可读性。
private findNonDropContainerId(tabbarService: TabbarService): string { - const currentContainerId = tabbarService.currentContainerId.get(); - if (currentContainerId && !this.isDropContainer(currentContainerId)) { - return currentContainerId; - } - if (tabbarService.previousContainerId && !this.isDropContainer(tabbarService.previousContainerId)) { - return tabbarService.previousContainerId; - } - - for (const key of tabbarService.containersMap.keys()) { - if (!this.isDropContainer(key)) { - return key; - } - } - - return ''; + const candidates = [ + tabbarService.currentContainerId.get(), + tabbarService.previousContainerId, + ...tabbarService.containersMap.keys() + ]; + + return candidates.find(id => id && !this.isDropContainer(id)) || ''; }
175-233: 建议添加方法文档和改进注释当前方法的复杂度随着拖放功能的添加而增加，建议添加详细的方法文档说明各种情况的处理逻辑。
+ /** + * 恢复 TabbarService 的状态 + * @param service - 要恢复的 TabbarService + * + * 处理以下场景: + * 1. currentId 为 undefined: 使用默认容器 + * 2. currentId 为非空字符串: + * - 如果是非 drop 容器，直接使用 + * - 如果容器不存在，尝试从其他 tabbar 移动 + * 3. currentId 为空字符串或 drop 容器: 清空当前容器 + */ restoreTabbarService = async (service: TabbarService) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 186-188: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/design/src/browser/style/design.module.less(2 hunks)packages/main-layout/src/browser/accordion/titlebar.view.tsx(1 hunks)packages/main-layout/src/browser/layout.service.ts(4 hunks)
🔇 Additional comments (4)
packages/main-layout/src/browser/accordion/titlebar.view.tsx (1)
18-18: 服务注入实现正确TabbarService 的注入实现合理且符合依赖注入模式。
packages/design/src/browser/style/design.module.less (2)
604-607: 滚动条交互优化合理鼠标悬停时增加滚动条宽度的设计可以提升用户体验。
721-738: 标签项样式实现合理样式实现保持了与其他交互元素的一致性，提供了良好的视觉反馈。
packages/main-layout/src/browser/layout.service.ts (1)
247-270:⚠️ Potential issue建议增强错误处理和添加原子操作
- 需要验证目标 TabbarService (toTabbar) 是否存在
- 建议实现原子操作以确保状态一致性
moveContainerTo(containerId: string, to: string): void { + const toTabbar = this.getTabbarService(to); + if (!toTabbar) { + this.logger.error(`cannot find tabbar service: ${to}`); + return; + } + const fromTabbar = this.findTabbarServiceByContainerId(containerId); if (!fromTabbar) { this.logger.error(`cannot find container: ${containerId}`); return; } + const container = fromTabbar.getContainer(containerId); if (!container) { this.logger.error(`cannot find container: ${containerId}`); return; } - const toTabbar = this.getTabbarService(to); + try { + const savedState = { + container, + currentId: fromTabbar.currentContainerId.get() + }; - fromTabbar.removeContainer(containerId); + fromTabbar.removeContainer(containerId); - if (!fromTabbar.visibleContainers.length || fromTabbar.currentContainerId.get() === containerId) { - this.toggleSlot(fromTabbar.location, false); - } - toTabbar.dynamicAddContainer(containerId, container); - const newHandler = this.injector.get(TabBarHandler, [containerId, this.getTabbarService(toTabbar.location)]); - this.handleMap.set(containerId, newHandler!); + if (!fromTabbar.visibleContainers.length || fromTabbar.currentContainerId.get() === containerId) { + this.toggleSlot(fromTabbar.location, false); + } + + toTabbar.dynamicAddContainer(containerId, container); + const newHandler = this.injector.get(TabBarHandler, [containerId, toTabbar]); + this.handleMap.set(containerId, newHandler!); + } catch (error) { + // 回滚操作 + fromTabbar.dynamicAddContainer(containerId, container); + if (savedState.currentId === containerId) { + fromTabbar.updateCurrentContainerId(containerId); + } + throw error; + } }Likely invalid or redundant comment.
|
/next |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/main-layout/src/browser/drop-area/drop-area.tsx (3)
17-23: 建议增加空值检查在处理拖放事件时，建议增加对 containerId 的空值检查，以提高代码的健壮性。
const handleDrop = React.useCallback( (e: React.DragEvent) => { const containerId = e.dataTransfer?.getData('containerId'); + if (!containerId) { + return; + } layoutService.moveContainerTo(containerId, location); }, [layoutService, location], );
25-35: 建议增加无障碍支持为了提高组件的可访问性，建议添加适当的 ARIA 属性。
<div className={styles.drop_area} onDrop={handleDrop} onDragOver={(e) => { e.preventDefault(); }} + role="region" + aria-label={localize('main-layout.drop-area.tip')} > {localize('main-layout.drop-area.tip')} </div>
25-35: 建议添加测试属性为了便于自动化测试，建议添加 data-testid 属性。
<div className={styles.drop_area} onDrop={handleDrop} onDragOver={(e) => { e.preventDefault(); }} + data-testid={`drop-area-${location}`} > {localize('main-layout.drop-area.tip')} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/i18n/src/common/en-US.lang.ts(1 hunks)packages/i18n/src/common/zh-CN.lang.ts(1 hunks)packages/main-layout/src/browser/drop-area/drop-area.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/src/common/zh-CN.lang.ts
🧰 Additional context used
📓 Learnings (1)
packages/main-layout/src/browser/drop-area/drop-area.tsx (1)
Learnt from: Marckon
PR: opensumi/core#4340
File: packages/main-layout/src/browser/drop-area/drop-area.tsx:19-22
Timestamp: 2025-02-06T02:12:48.137Z
Learning: The `moveContainerTo` method in IMainLayoutService already implements internal error handling, including logging errors when containers are not found. Additional try-catch blocks at the UI layer are unnecessary.
🔇 Additional comments (2)
packages/i18n/src/common/en-US.lang.ts (1)
1329-1329: 代码变更看起来不错！本地化字符串简洁明了，符合拖放区域的提示要求。
packages/main-layout/src/browser/drop-area/drop-area.tsx (1)
1-40: 组件实现看起来不错！组件结构清晰，代码组织合理，功能实现完整。
|
/next |
1 similar comment
|
/next |
Marckon
force-pushed
the
feat/move-terminal
branch
from
385d23a to
8e4dceb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/main-layout/src/browser/drop-area/drop-area.tsx (1)
29-31: 建议将onDragOver处理函数提取为记忆化的回调函数为了保持一致性和优化性能，建议将内联的
onDragOver处理函数提取为使用useCallback记忆化的回调函数。+ const handleDragOver = React.useCallback((e: React.DragEvent) => { + e.preventDefault(); + }, []); + return ( <div className={styles.drop_area} onDrop={handleDrop} - onDragOver={(e) => { - e.preventDefault(); - }} + onDragOver={handleDragOver} >packages/main-layout/src/browser/accordion/titlebar.view.tsx (2)
20-24: 建议将handleDragStart提取为记忆化的回调函数为了保持一致性和优化性能，建议将
handleDragStart处理函数使用useCallback进行记忆化。- const handleDragStart = (e: React.DragEvent) => { + const handleDragStart = React.useCallback((e: React.DragEvent) => { if (props.containerId && props.side) { tabbarService.handleDragStart(e, props.containerId); } - }; + }, [props.containerId, props.side, tabbarService]);
35-40: 建议优化条件渲染逻辑
!!props.draggable是多余的，因为props.draggable已经是布尔类型- 可以使用三元运算符使代码更简洁
- {!props.draggable && <h1>{props.title}</h1>} - {!!props.draggable && ( - <h1 draggable style={{ cursor: 'pointer' }} onDragStart={handleDragStart} onDragEnd={handleDragEnd}> - {props.title} - </h1> - )} + <h1 + {...(props.draggable && { + draggable: true, + style: { cursor: 'pointer' }, + onDragStart: handleDragStart, + onDragEnd: handleDragEnd, + })} + > + {props.title} + </h1>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/i18n/src/common/en-US.lang.ts(1 hunks)packages/i18n/src/common/zh-CN.lang.ts(1 hunks)packages/main-layout/src/browser/accordion/titlebar.view.tsx(1 hunks)packages/main-layout/src/browser/drop-area/drop-area.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/i18n/src/common/en-US.lang.ts
- packages/i18n/src/common/zh-CN.lang.ts
🧰 Additional context used
📓 Learnings (1)
packages/main-layout/src/browser/drop-area/drop-area.tsx (1)
Learnt from: Marckon
PR: opensumi/core#4340
File: packages/main-layout/src/browser/drop-area/drop-area.tsx:19-22
Timestamp: 2025-02-06T02:12:48.137Z
Learning: The `moveContainerTo` method in IMainLayoutService already implements internal error handling, including logging errors when containers are not found. Additional try-catch blocks at the UI layer are unnecessary.
🔇 Additional comments (4)
packages/main-layout/src/browser/drop-area/drop-area.tsx (3)
17-23:handleDrop回调函数实现正确！
handleDrop回调函数使用useCallback正确地进行了记忆化，并且依赖项列表完整。由于moveContainerTo已经实现了内部错误处理，因此不需要额外的错误处理。
38-38: 实现正确！
RightDropArea组件正确地封装了DropArea，并传递了固定的location属性。
40-40: 实现正确！
BottomDropArea组件正确地封装了DropArea，并传递了固定的location属性。packages/main-layout/src/browser/accordion/titlebar.view.tsx (1)
13-15: 🛠️ Refactor suggestion需要完善 Props 类型定义
当
draggable为true时，side和containerId应该是必需的。建议添加类型注释说明属性用途，并使用条件类型确保类型安全。- draggable?: boolean; - side?: string; - containerId?: string; + /** 是否可拖动 */ + draggable?: boolean; + /** 容器所在位置，仅在可拖动时必需 */ + side: draggable extends true ? string : string | undefined; + /** 容器唯一标识，仅在可拖动时必需 */ + containerId: draggable extends true ? string : string | undefined;Likely invalid or redundant comment.
|
/next |
|
🎉 PR Next publish successful! 3.7.1-next-1738899837.0 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4340 +/- ##
=======================================
Coverage ? 54.13%
=======================================
Files ? 1639
Lines ? 100313
Branches ? 21766
=======================================
Hits ? 54302
Misses ? 38222
Partials ? 7789
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: 0
🧹 Nitpick comments (4)
packages/core-browser/src/layout/layout.interface.ts (1)
84-84: 建议为 draggable 属性添加 JSDoc 注释为了保持代码文档的一致性和完整性，建议添加 JSDoc 注释来说明该属性的用途。
建议添加如下注释：
+ // 是否允许拖拽该容器 draggable?: boolean;packages/main-layout/src/browser/layout.service.ts (3)
35-36: 建议将拖拽区域常量合并到一个对象中为了更好地管理拖拽区域相关的常量，建议将它们合并到一个对象中。这样可以:
- 减少重复代码
- 便于后续添加新的拖拽区域
- 提高代码的可维护性
-export const DROP_BOTTOM_CONTAINER = 'drop-bottom'; -export const DROP_RIGHT_CONTAINER = 'drop-right'; +export const DROP_CONTAINERS = { + BOTTOM: 'drop-bottom', + RIGHT: 'drop-right', +} as const;
276-301: 建议增加日志记录以便调试为了更好地追踪拖拽区域的显示和隐藏状态，建议添加调试日志。这将有助于:
- 排查问题
- 监控状态变化
- 提高可维护性
showDropAreaForContainer(containerId: string): void { const tabbarService = this.findTabbarServiceByContainerId(containerId); const bottomService = this.tabbarServices.get('bottom'); const rightService = this.tabbarServices.get('right'); if (!tabbarService) { this.logger.error(`cannot find container: ${containerId}`); return; } if (tabbarService?.location === 'right') { + this.logger.debug(`显示底部拖拽区域: ${containerId}`); bottomService?.updateCurrentContainerId(DROP_BOTTOM_CONTAINER); } if (tabbarService?.location === 'bottom') { + this.logger.debug(`显示右侧拖拽区域: ${containerId}`); rightService?.updateCurrentContainerId(DROP_RIGHT_CONTAINER); } } hideDropArea(): void { const bottomService = this.tabbarServices.get('bottom'); const rightService = this.tabbarServices.get('right'); + this.logger.debug('隐藏拖拽区域'); if (bottomService?.currentContainerId.get() === DROP_BOTTOM_CONTAINER) { bottomService.updateCurrentContainerId(bottomService.previousContainerId || ''); } if (rightService?.currentContainerId.get() === DROP_RIGHT_CONTAINER) { rightService.updateCurrentContainerId(rightService.previousContainerId || ''); } }
338-358: 建议使用 Set 优化性能为了提高
isDropContainer方法的性能，建议使用 Set 代替数组。这样可以:
- 提高查找效率
- 减少内存使用
- 提供更清晰的语义
+private readonly DROP_CONTAINERS_SET = new Set([DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER]); private isDropContainer(containerId: string): boolean { - return [DROP_BOTTOM_CONTAINER, DROP_RIGHT_CONTAINER].includes(containerId); + return this.DROP_CONTAINERS_SET.has(containerId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core-browser/src/layout/layout.interface.ts(1 hunks)packages/main-layout/src/browser/layout.service.ts(4 hunks)packages/main-layout/src/browser/tabbar/bar.view.tsx(3 hunks)packages/terminal-next/src/browser/contribution/terminal.view.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/main-layout/src/browser/tabbar/bar.view.tsx
- packages/terminal-next/src/browser/contribution/terminal.view.ts
🔇 Additional comments (2)
packages/core-browser/src/layout/layout.interface.ts (1)
77-78: 代码实现清晰明确！属性定义和注释说明都很完整，能够清楚地表达隐藏指定位置图标的功能。
packages/main-layout/src/browser/layout.service.ts (1)
247-274: 建议实现原子操作以确保状态一致性当前的容器移动实现可能在失败时导致状态不一致。建议:
- 在移动开始前保存状态
- 如果任何步骤失败，回滚到保存的状态
- 使用事务模式确保操作的原子性
moveContainerTo(containerId: string, to: string): void { + const savedState = { + fromTabbar: this.findTabbarServiceByContainerId(containerId), + container: null, + currentId: null, + }; + + try { const fromTabbar = this.findTabbarServiceByContainerId(containerId); if (!fromTabbar) { this.logger.error(`cannot find container: ${containerId}`); return; } const container = fromTabbar.getContainer(containerId); + savedState.container = container; + savedState.currentId = fromTabbar.currentContainerId.get(); + if (!container) { this.logger.error(`cannot find container: ${containerId}`); return; } if (!container.options?.draggable) { this.logger.warn(`container: ${containerId} is not draggable`); return; } const toTabbar = this.getTabbarService(to); fromTabbar.removeContainer(containerId); if (!fromTabbar.visibleContainers.length || fromTabbar.currentContainerId.get() === containerId) { this.toggleSlot(fromTabbar.location, false); } toTabbar.dynamicAddContainer(containerId, container); const newHandler = this.injector.get(TabBarHandler, [containerId, this.getTabbarService(toTabbar.location)]); this.handleMap.set(containerId, newHandler!); + } catch (error) { + // 回滚操作 + if (savedState.fromTabbar && savedState.container) { + savedState.fromTabbar.dynamicAddContainer(containerId, savedState.container); + if (savedState.currentId === containerId) { + savedState.fromTabbar.updateCurrentContainerId(containerId); + } + } + throw error; + } }
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Types
Background or solution
users wants to drag terminal from bottom to right;
Changelog
Summary by CodeRabbit
新功能
DropArea组件，支持拖放操作。样式更新
.header和.drop_area样式类，提升布局和交互体验。.terminalContentWrapper样式类，增强终端组件的布局效果。