Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zqq-dora The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThe PR implements a theme-aware 1px inset bevel for enabled MenuItem highlights using DTK palettes and BoxInsetShadow layers, while also changing related menu geometry/separator styling and adding detailed rendering documentation. Sequence diagram for theme-aware MenuItem highlight renderingsequenceDiagram
participant MenuItem
participant ColorSelector
participant BoxInsetShadow
participant DTKPalette
MenuItem->>MenuItem: highlighted = enabled && (hovered || subMenu.visible)
MenuItem->>ColorSelector: menuItemInnerShadow
ColorSelector->>DTKPalette: select theme color
DTKPalette-->>ColorSelector: top or bottom shadow color
MenuItem->>BoxInsetShadow: show top inset shadow
MenuItem->>BoxInsetShadow: show bottom inset shadow
BoxInsetShadow-->>MenuItem: render rounded 1px bevel
Flow diagram for MenuItem highlight bevel statesflowchart TD
A[MenuItem state changes] --> B{enabled and hovered or subMenu.visible?}
B -- No --> C[Hide inset shadows]
B -- Yes --> D[Show HighlightPanel]
D --> E{Dark theme?}
E -- No --> F[Bottom shadow: black 20%]
E -- Yes --> G[Top highlight: white 10%]
E -- Yes --> H[Bottom shadow: black 24%]
F --> I[Render rounded 1px inner bevel]
G --> I
H --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @zqq-dora. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="qt6/src/docs/README.md" line_range="17" />
<code_context>
|---|---|
| 本主文档 | 统一目的、文档结构、状态维度、效果术语、颜色规则、维护和验收要求 |
| [Button.md](Button.md) | 普通 Button 的外形、内容、各渲染元素的显示条件、颜色和参数；不覆盖选中、高亮变体 |
+| [Menu.md](Menu.md) | MenuItem 悬浮高亮背景的填充与 1px 底部内阴影；不含菜单容器面板、分隔符等 |
| [QML 维护约束](../qml/.AGENTS) | 简短约束代码与文档同步，引用本规范，不重复详细规则 |
</code_context>
<issue_to_address>
**nitpick:** The documentation index describes Menu.md as covering only the 1px bottom inner shadow, omitting the dark-theme 1px top highlight that the new implementation and Menu.md itself document. Readers using the index receive an incomplete description of the supported rendering.
**Suggested fix:** Update the description to mention both the bottom shadow and the dark-theme top highlight.
```suggestion
| [Menu.md](Menu.md) | MenuItem 悬浮高亮背景的填充与 1px 底部内阴影、暗色主题 1px 顶部高光；不含菜单容器面板、分隔符等 |
```
</issue_to_address>| |---|---| | ||
| | 本主文档 | 统一目的、文档结构、状态维度、效果术语、颜色规则、维护和验收要求 | | ||
| | [Button.md](Button.md) | 普通 Button 的外形、内容、各渲染元素的显示条件、颜色和参数；不覆盖选中、高亮变体 | | ||
| | [Menu.md](Menu.md) | MenuItem 悬浮高亮背景的填充与 1px 底部内阴影；不含菜单容器面板、分隔符等 | |
There was a problem hiding this comment.
nitpick: The documentation index describes Menu.md as covering only the 1px bottom inner shadow, omitting the dark-theme 1px top highlight that the new implementation and Menu.md itself document. Readers using the index receive an incomplete description of the supported rendering.
Suggested fix: Update the description to mention both the bottom shadow and the dark-theme top highlight.
| | [Menu.md](Menu.md) | MenuItem 悬浮高亮背景的填充与 1px 底部内阴影；不含菜单容器面板、分隔符等 | | |
| | [Menu.md](Menu.md) | MenuItem 悬浮高亮背景的填充与 1px 底部内阴影、暗色主题 1px 顶部高光；不含菜单容器面板、分隔符等 | |
a69815d to
544b77b
Compare
| } | ||
| } | ||
| // 1px top inset highlight — white 10%, follows rounded corners. | ||
| BoxInsetShadow { |
There was a problem hiding this comment.
这个是所有的 HighlightPanel 都需要的，还是只有MenuItem才需要的呀，是不是可以放在HighlightPanel里去实现，另外
HighlightPanel 的innerShadowColor是不是可以完成这个效果，
There was a problem hiding this comment.
目前这个内阴影效果只 MenuItem 需要，不是所有 HighlightPanel 场景都适用，所以放在 MenuItem 里单独实现。HighlightPanel 里注释掉的 BoxInsetShadow 只有单方向（底部），而这里需要顶部 + 底部两道，且浅色/深色模式参数不同，在 HighlightPanel 里扩展会影响所有使用方。innerShadowColor 目前只有一道底部阴影的能力，不太适合直接用。
Thanks for the review! This inner shadow effect is currently only needed for MenuItem, not all HighlightPanel use cases, so it's implemented separately in MenuItem. The commented-out BoxInsetShadow in HighlightPanel only supports a single direction (bottom), while here we need top + bottom layers with different parameters for light/dark modes. Extending HighlightPanel would affect all consumers. innerShadowColor currently only has one bottom shadow capability, which doesn't quite fit this use case.
There was a problem hiding this comment.
能不能扩展HiightPanel这个 公共接口去实现MenuItem的效果，类似之前Button里的BoxPanel
544b77b to
a97c184
Compare
|
已按建议改为通过 HighlightPanel 公共接口实现。扩展了 HighlightPanel，新增 Updated per review suggestion to use the HighlightPanel public interface. Extended HighlightPanel with |
| } | ||
|
|
||
| // Top inner shadow (bevel highlight). | ||
| BoxInsetShadow { |
There was a problem hiding this comment.
这个也应该用Loader进行管理，增加性能，
| } | ||
|
|
||
| background: Item { | ||
| id: backgroundItem |
| Loader { | ||
| anchors.fill: backgroundRect | ||
| active: panel.innerShadowColor | ||
| z: D.DTK.AboveOrder |
| property D.Palette textColor: control.highlighted ? DS.Style.checkedButton.text | ||
| : DS.Style.menu.itemText | ||
| property D.Palette subMenuBackgroundColor: DS.Style.menu.subMenuOpenedBackground | ||
| property D.Palette menuItemInnerShadow: DS.Style.menu.itemHighlightInnerShadow |
There was a problem hiding this comment.
这个需要外面设置么？如果不需要，是不是可以放在不用暴露，放在background里定义即可，
a97c184 to
cedc6cf
Compare
|
已根据评审意见逐条修改：
Updated per review comments:
|
| // Dual-direction inner shadow colors for consumers that need a | ||
| // top + bottom bevel (e.g. MenuItem hover). Callers resolve palette | ||
| // colors via their own ColorSelector and pass plain colors here. | ||
| property color bevelShadowColor1: "transparent" |
There was a problem hiding this comment.
按照之前的这个来编写吧，
shadowColor: panel.D.ColorSelector.innerShadowColor
There was a problem hiding this comment.
已按这个写法修改：bevelShadowColor1/bevelShadowColor2 改为 property D.Palette（默认 null），shadowColor 绑定到 panel.D.ColorSelector.bevelShadowColor1/2，与同文件的 innerShadowColor 写法一致，颜色随主题自动切换。MenuItem 不再在外面解析颜色，直接把样式调色板传给 HighlightPanel。其余评审意见（移除多余 z 轴、不暴露不必要的控件属性）也已一并处理，并 squash 为单个提交。
Updated per this suggestion: bevelShadowColor1/bevelShadowColor2 are now property D.Palette (default null), with shadowColor bound to panel.D.ColorSelector.bevelShadowColor1/2, matching the innerShadowColor pattern in the same file so colors follow theme changes automatically. MenuItem no longer resolves colors itself; it passes the style palettes directly to HighlightPanel. The remaining review comments (removing the extra z axis, not exposing unnecessary control properties) are also addressed, all squashed into a single commit.
Add a subtle inner-shadow bevel to MenuItem hover/highlight state, matching the ToolButton checked-state approach using BoxInsetShadow. FlowStyle.qml: - Add itemHighlightInnerShadow palette (bottom): 20% black (light) / 24% black (dark) - Add itemHighlightInnerShadowTop palette (top): transparent (light) / 10% white (dark) HighlightPanel.qml: - Extend with bevelShadowColor1/2 (D.Palette, default null) for top + bottom bevel inner shadows, inactive by default so existing callers are unaffected - Resolve bevel colors via panel.D.ColorSelector.bevelShadowColor1/2, following the same pattern as innerShadowColor - Restore the legacy innerShadowColor Loader (managed by Loader for performance) MenuItem.qml: - Pass DS.Style.menu.itemHighlightInnerShadowTop/InnerShadow directly to HighlightPanel's bevelShadowColor1/2; colors resolve via HighlightPanel's ColorSelector and follow theme changes automatically Menu.md: - Document MenuItem hover-highlight rendering README.md: - Add Menu.md to the docs index, covering bottom inner shadow and dark-theme top highlight Light mode: only bottom 1px inner shadow (20% black). Dark mode: top 1px white 10% highlight + bottom 1px black 24% inner shadow. Color values use DTK palettes so they follow theme changes automatically.
14cd102 to
82884f6
Compare
This branch has not been deployed
3 participants
Summary
Add a subtle inner-shadow bevel to the MenuItem hover/highlight state, matching the ToolButton checked-state approach using
BoxInsetShadow.Changes
FlowStyle.qml — two new palettes under
menu:itemHighlightInnerShadow(bottom): 20% black (light) / 24% black (dark)itemHighlightInnerShadowTop(top): transparent (light) / 10% white (dark)MenuItem.qml — two
BoxInsetShadowelements in background, visible oncontrol.highlighted:shadowOffsetY: 1,blur: 1, white 10% (dark mode only)shadowOffsetY: -1,blur: 1, black 20% (light) / 24% (dark)cornerRadiusfollowsDS.Style.menu.item.radius(6)Menu.md — new documentation for MenuItem hover highlight rendering.
README.md — add Menu.md to docs index.
Result
Color values use DTK palettes so they follow theme changes automatically.
概述
新增 MenuItem 悬浮高亮的 1px 内阴影斜面效果，参照 ToolButton checked 状态使用
BoxInsetShadow实现。改动内容
FlowStyle.qml —
menu下新增两个调色板：itemHighlightInnerShadow（底部）：浅色 20% 黑 / 深色 24% 黑itemHighlightInnerShadowTop（顶部）：浅色透明 / 深色 10% 白MenuItem.qml — 在 background 中新增两个
BoxInsetShadow，仅在control.highlighted时显示：shadowOffsetY: 1，blur: 1，白色 10%（仅深色模式）shadowOffsetY: -1，blur: 1，黑色 20%（浅色）/ 24%（深色）cornerRadius跟随DS.Style.menu.item.radius（6）Menu.md — 新增菜单项悬浮高亮渲染说明文档。
README.md — 文档索引加入 Menu.md。
效果
色值使用 DTK 调色板定义，随主题自动变化。
Summary by Sourcery
Add theme-aware inset bevel shadows to the MenuItem highlight state.
New Features:
Enhancements: