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

feat: support icon menubar #2728

Merged
merged 4 commits into from
May 25, 2023
Merged

feat: support icon menubar #2728

merged 4 commits into from
May 25, 2023

Conversation

Ricbet
Copy link
Member

@Ricbet Ricbet commented May 18, 2023

Types

  • 🎉 New Features

Background or solution

🤖 Generated by Copilot at 997db4c

  • Import and use the getIcon and getExternalIcon functions to get the icon class name for each command (link, link)
  • Register a new menu item group for the icon menubar context with the IconMenubarContext menu id (link, link)
  • Add a new style rule for the icon_menubar_container class name to style the container element for the icon menubar component (link)
  • Add a new component IconMenuBar that uses the menu service to create a menu object for the icon menubar context and passes it to the InlineActionBar component with the isFlattenMenu prop set to true (link, link)
  • Add a new import for the IconMenuBar component and a commented out code that renders it in the MenuBarView component in the sample module (link, link)
  • Optimize the rendering of the inline action bar component by replacing the useCallback hook with the useMemo hook (link)
  • Add a conditional rendering for the separator menu item node as a vertical divider span element with the verticalDivider class name (link, link)
  • Add a new prop isFlattenMenu to the InlineActionBarProps interface and use it to determine how to render the menu items as a single group or two groups (link, link)

新增 iconMenubar 组件,集成方需要往 IconMenubarContext 注册菜单项(icon 字段必填),然后导入 IconMenuBar 即可
示例文档稍候补充

image

Changelog

🤖 Generated by Copilot at 997db4c

This pull request adds a new icon menubar feature to the core-browser and menu-bar packages, which allows displaying and executing common editor commands with icons in the menu bar. It also improves the performance and flexibility of the inline action bar component, which is used to render the menu items. It modifies several files in the core-browser, menu-bar, and startup packages to implement and test the new feature and its styles.

@Ricbet Ricbet self-assigned this May 18, 2023
@opensumi opensumi bot added the 🎨 feature feature required label May 18, 2023
@opensumi
Copy link
Contributor

opensumi bot commented May 18, 2023

ChatGPT Code Review:

代码功能:此次提交添加了一个新的 icon menubar 功能。通过在菜单栏上添加图标,可以快速执行一些常用的编辑命令。除了添加新功能之外,还通过使用 useMemo 钩子函数来优化内联操作栏组件的性能和灵活性,同时还优化了内联操作栏中的菜单项分组方式以及样式。此次修改主要涉及 InlineActionBarIconMenuBar 两个组件,具体修改内容如下:

  • 新增 iconMenubar 组件,集成方需要往 IconMenubarContext 注册菜单项(icon 字段必填),然后导入 IconMenuBar 即可
  • 优化内联操作栏组件的渲染方式,使用 `useMemo

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

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

Comparison is base (b085e3f) 57.72% compared to head (e193882) 57.72%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2728      +/-   ##
==========================================
- Coverage   57.72%   57.72%   -0.01%     
==========================================
  Files        1327     1327              
  Lines       83563    83581      +18     
  Branches    17381    17378       -3     
==========================================
+ Hits        48238    48246       +8     
- Misses      32104    32114      +10     
  Partials     3221     3221              
Flag Coverage Δ
jsdom 52.72% <100.00%> (-0.01%) ⬇️
node 16.79% <100.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
packages/core-browser/src/menu/next/menu-id.ts 100.00% <100.00%> (ø)

... and 20 files with indirect coverage changes

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

@bytemain
Copy link
Member

多一个 IconMenuBar 很奇怪,似乎应该是注册 Menu 的时候,如果该 MenuId 的类型是 icon,那就只展示 Icon?

@bytemain
Copy link
Member

要我看的话,不如做一个 MenuBarNg,既支持 icon,又支持 text 又支持 icon text,还支持 button,就像现在的 toolbar 一样。

@Ricbet
Copy link
Member Author

Ricbet commented May 19, 2023

多一个 IconMenuBar 很奇怪,似乎应该是注册 Menu 的时候,如果该 MenuId 的类型是 icon,那就只展示 Icon?

他和现在的 MenuBar 是不一样的,你这样处理就破坏了原来的顶部菜单了(比如会造成有的是文字有的是 icon)。

这里需要单独抽一份 MenuId Context 出来去注册图标菜单,这个菜单项可以是已有的 MenuBar,也可以是用户自己新注册的 command

@Ricbet
Copy link
Member Author

Ricbet commented May 19, 2023

image

而原来的 MenuBar 可以使用 compact 模式在左侧顶部显示

pipiiiiii
pipiiiiii previously approved these changes May 22, 2023
@Ricbet
Copy link
Member Author

Ricbet commented May 25, 2023

文档已补充:opensumi/docs#118

@Ricbet Ricbet merged commit e2270b4 into main May 25, 2023
12 checks passed
@Ricbet Ricbet deleted the feat/support-icon-menubar branch May 25, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants