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: improve style and fix breakpoints view init #2583

Merged
merged 5 commits into from
Apr 17, 2023

Conversation

erha19
Copy link
Member

@erha19 erha19 commented Apr 13, 2023

Types

  • 🐛 Bug Fixes
  • 💄 Style Changes

Background or solution

🤖 Generated by Copilot at e89ae06

  • Fix alignment and indentation issues of tree nodes in recycle tree and debug breakpoints views (link, link, link)
  • Avoid unnecessary rendering of icons and segments in tree nodes (link, link)
  • Ensure breakpoints are updated when ready and avoid potential race conditions in debug breakpoints service (link, link)
  • Improve performance and reliability of debug breakpoint view component and handle undefined expandable property (link, link, link, link, link)
  • Fix a bug where preference groups with no children would still be expandable (link)
  • Import isDefined utility function from @opensumi/ide-utils in tree-node.tsx (link)
  • Add margin to icons in debug breakpoints view (link)
  • Remove unused import and empty line in debug-breakpoints.service.ts (link)
  • Remove mistakenly added entry for terminal preference group (link)
  • Add empty line to separate imports in tree-node.tsx (link)

修复在调试断点面板下刷新后,断点消失问题
close #2552.

优化设置面板及调试断点面板样式
image

close #2581

Changelog

improve style and fix breakpoints view init

@erha19 erha19 requested review from bytemain and Ricbet April 13, 2023 05:09
@erha19
Copy link
Member Author

erha19 commented Apr 13, 2023

/publish

@opensumi opensumi bot added 🐞 bug Something isn't working 💄 style change labels Apr 13, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2023

ChatGPT Code Review:

Code Review:

  1. 在 BasicTreeNodeRenderer 中,paddingLeft 的计算方式有变化,增加了对 BasicCompositeTreeNode 的判断,使得对于不同的节点能够正确地计算 padding 值。

  2. 在 BasicTreeNodeRenderer 中,renderIcon 函数的实现发生了改变,加入了对 iconClassName 的检查,避免了一些意外的错误。

  3. DebugBreakpointsService 中,init 函数的处理顺序被修改,避免了 debugBreakpointsService.updateBreakpoints 函数在 plugin 初始化时执行而导致的错误。

  4. DebugBreakpointView 中,useEffect 的依赖项增加了 updateTreeData,避免了因为 treeData 的变化导致的无限循环。

  5. PreferenceIndexes 中,判断 data 是否可展开的逻辑有改变,解决了 data.children 为空时展开图标还显示的问题。

修改建议:

  1. 代码已经进行了修改,没有明显的错误和 typo。

  2. 对于 BasicTreeNodeRenderer 中的 padding 和 icon 相关的修改建议保留。

  3. 建议将 DebugBreakpointsService 中,updateBreakpoints 函数的调用放到 whenReady 的回调函数中,防止在插件初始化时就执行了该函数。

  4. 建议删除 DebugBreakpointView 中不必要的依赖项。

  5. 将 PreferenceIndexes 中的逻辑改为仅当 data.children 存在时才将 data.expandable 值设为 true,不必 elif 文段。

@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Patch coverage: 53.33% and project coverage change: -0.01 ⚠️

Comparison is base (6587653) 57.63% compared to head (297497d) 57.62%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2583      +/-   ##
==========================================
- Coverage   57.63%   57.62%   -0.01%     
==========================================
  Files        1326     1326              
  Lines       83553    83576      +23     
  Branches    17398    17406       +8     
==========================================
+ Hits        48153    48164      +11     
- Misses      32161    32174      +13     
+ Partials     3239     3238       -1     
Flag Coverage Δ
jsdom 52.64% <53.33%> (-0.01%) ⬇️
node 16.76% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...kages/core-browser/src/services/storage-service.ts 74.11% <0.00%> (-1.79%) ⬇️
...erences/src/browser/preference-settings.service.ts 72.19% <ø> (ø)
packages/storage/src/browser/storage.ts 62.14% <54.54%> (-1.24%) ⬇️
...wser/view/breakpoints/debug-breakpoints.service.ts 66.14% <100.00%> (+0.26%) ⬆️

... and 15 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Ricbet
Copy link
Member

Ricbet commented Apr 14, 2023

本地验了下,还是会存在刷新后断点消失的问题,复现步骤是打开一个没有断点的文件,然后刷新

@erha19
Copy link
Member Author

erha19 commented Apr 14, 2023

由于先前 Storage 移除了对于 WorkspaceService 的时序依赖,在获取缓存的时候可能会出现 this.workspaceService.workspace 无法及时取值导致(工作区 Storage) 缓存更新问题,最终导致数据丢失。

这里针对存储做了默认的 fallback 处理,即首屏加载(工作区 Storage)时默认采用 AppConfig.workspaceDir 作为工作区路径,加速首屏速度及准确性。

同时进一步优化了 Storage 的缓存逻辑,减少重复的 Storage 创建 @Ricbet @geekeren

@Ricbet
Copy link
Member

Ricbet commented Apr 17, 2023

这样改似乎影响了 ts 语言服务的启动?刷新后打开 ts 文件,会发现语法高亮和语言服务都没有生效

Copy link
Member

@Ricbet Ricbet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erha19 erha19 merged commit 1caa4a2 into main Apr 17, 2023
11 checks passed
@erha19 erha19 deleted the fix/debug/get-breakpoints branch April 17, 2023 07:25
@erha19 erha19 added this to the 2.24 milestone Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 💄 style change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] 设置面板(Settings)图标对齐样式问题 [BUG] 调试断点概率性丢失
2 participants