fix: keep dock indicator distance constant when icons scale down #1372
Conversation
This file contains hidden or 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
|
Hi @Ivy233. Thanks for your PR. 😃 |
|
Hi @Ivy233. 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. |
|
CLA Assistant Lite bot: |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts dock window-indicator anchor margins so the visual gap between app icons and their underline indicator remains constant when icons scale with dock size and orientation. Class diagram for AppItem and AppItemWithTitle indicator anchoringclassDiagram
class AppItem {
+int root_height
+int root_width
+int iconSize
+DockPosition dockPosition
+WindowIndicator windowIndicator
+updateIndicatorAnchors()
}
class AppItemWithTitle {
+int root_height
+int root_width
+int iconSize
+DockPosition dockPosition
+Item iconContainer
+WindowIndicator windowIndicator
+updateIndicatorAnchors()
}
class WindowIndicator {
+AnchorGroup anchors
}
class AnchorGroup {
+Anchor top
+Anchor bottom
+Anchor left
+Anchor right
+Anchor horizontalCenter
+Anchor verticalCenter
+Binding topMargin
+Binding bottomMargin
+Binding leftMargin
+Binding rightMargin
}
class DockPosition {
<<enumeration>>
Top
Bottom
Left
Right
}
class Binding {
+expression
}
AppItem --> WindowIndicator : contains
AppItemWithTitle --> WindowIndicator : contains
AppItemWithTitle --> AnchorGroup : uses
AppItem --> AnchorGroup : uses
AppItem --> DockPosition : uses
AppItemWithTitle --> DockPosition : uses
AnchorGroup --> Binding : sets
note for AppItem "For Top/Bottom:\nmargin = (root.height - iconSize) / 2 - 8\nFor Left/Right:\nmargin = (root.width - iconSize) / 2 - 8"
note for AppItemWithTitle "Same margin formulas,\nbut centers on iconContainer\ninstead of parent"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The hard-coded
8px offset for the indicator is now duplicated in multiple places; consider extracting it into a shared constant (e.g. onrootor a theme object) so its value is defined in one place and can be adjusted consistently. - The new formula
(root.height - iconSize) / 2 - 8(and width variant) can become negative for some size combinations; it may be worth clamping the result to a minimum of 0 to avoid the indicator overlapping the icon or being pulled too far inward at extreme sizes. - The four nearly identical
Dock.Top/Bottom/Left/Rightbranches in both QML files repeat the same margin-binding pattern; consider factoring the margin calculation into a small helper or property function to reduce duplication and the chance of inconsistent future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded `8`px offset for the indicator is now duplicated in multiple places; consider extracting it into a shared constant (e.g. on `root` or a theme object) so its value is defined in one place and can be adjusted consistently.
- The new formula `(root.height - iconSize) / 2 - 8` (and width variant) can become negative for some size combinations; it may be worth clamping the result to a minimum of 0 to avoid the indicator overlapping the icon or being pulled too far inward at extreme sizes.
- The four nearly identical `Dock.Top/Bottom/Left/Right` branches in both QML files repeat the same margin-binding pattern; consider factoring the margin calculation into a small helper or property function to reduce duplication and the chance of inconsistent future changes.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/package/AppItemWithTitle.qml:310-319` </location>
<code_context>
+ windowIndicator.anchors.topMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - 8})
</code_context>
<issue_to_address>
**suggestion:** The repeated `- 8` offset in all branches might be better centralized or parameterized.
This same `(root.[height|width] - iconSize) / 2 - 8` pattern appears across all four orientations and in `AppItem.qml`. If `8` is a design token (e.g., padding/baseline offset), please extract it into a named property or helper (e.g., `indicatorPadding`) and reuse it, to keep spacing consistent and simplify future visual tweaks.
Suggested implementation:
```
case Dock.Top: {
windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter
windowIndicator.anchors.top = parent.top
windowIndicator.anchors.topMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - indicatorPadding})
return
}
```
```
case Dock.Bottom: {
windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter
windowIndicator.anchors.bottom = parent.bottom
windowIndicator.anchors.bottomMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - indicatorPadding})
return
}
```
1. Define a design-token-like property on the root of `AppItemWithTitle.qml`, for example:
`readonly property int indicatorPadding: 8`
and ensure the `root` id refers to that root item.
2. Replace the remaining occurrences of `- 8` used in `(root.[height|width] - iconSize) / 2 - 8` within this file (e.g., `Dock.Left` and `Dock.Right` branches) with `- indicatorPadding`.
3. Apply the same pattern to `AppItem.qml`: add the same `indicatorPadding` property on its root and replace the repeated `- 8` offsets in the indicator positioning expressions with `- indicatorPadding`, so spacing can be tuned from a single place in each component.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
310
to
319
| windowIndicator.anchors.topMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - 8}) | ||
| return | ||
| } | ||
| case Dock.Bottom: { | ||
| windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter | ||
| windowIndicator.anchors.bottom = parent.bottom | ||
| windowIndicator.anchors.bottomMargin = Qt.binding(() => {return (root.height - iconSize) / 2 / 3}) | ||
| windowIndicator.anchors.bottomMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - 8}) | ||
| return | ||
| } | ||
| case Dock.Left: { |
There was a problem hiding this comment.
suggestion: The repeated - 8 offset in all branches might be better centralized or parameterized.
This same (root.[height|width] - iconSize) / 2 - 8 pattern appears across all four orientations and in AppItem.qml. If 8 is a design token (e.g., padding/baseline offset), please extract it into a named property or helper (e.g., indicatorPadding) and reuse it, to keep spacing consistent and simplify future visual tweaks.
Suggested implementation:
case Dock.Top: {
windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter
windowIndicator.anchors.top = parent.top
windowIndicator.anchors.topMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - indicatorPadding})
return
}
case Dock.Bottom: {
windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter
windowIndicator.anchors.bottom = parent.bottom
windowIndicator.anchors.bottomMargin = Qt.binding(() => {return (root.height - iconSize) / 2 - indicatorPadding})
return
}
- Define a design-token-like property on the root of
AppItemWithTitle.qml, for example:
readonly property int indicatorPadding: 8
and ensure therootid refers to that root item. - Replace the remaining occurrences of
- 8used in(root.[height|width] - iconSize) / 2 - 8within this file (e.g.,Dock.LeftandDock.Rightbranches) with- indicatorPadding. - Apply the same pattern to
AppItem.qml: add the sameindicatorPaddingproperty on its root and replace the repeated- 8offsets in the indicator positioning expressions with- indicatorPadding, so spacing can be tuned from a single place in each component.
BLumia
reviewed
Dec 19, 2025
Member
BLumia
left a comment
BLumia
left a comment
There was a problem hiding this comment.
目前 - 8 这个有点 magic number 了。如果任务栏和图标等高，会导致指示器和图标重叠吧？
可以考虑给原本的逻辑加个 Math.min 保底，最高不大于 8 之类。另外记得确认下高分屏上的行为。
Ivy233
force-pushed
the
fix/dock-indicator-distance-constant
branch
from
7cc0d9a to
6e0545e
Compare
BLumia
previously approved these changes
Dec 22, 2025
Member
|
需要 rebase 一下，appitemwithtitle 现在没了（（ |
Ivy233
force-pushed
the
fix/dock-indicator-distance-constant
branch
from
6e0545e to
53acc8a
Compare
Contributor
|
在做了应用图标拆分功能后，设计师更改了指示器与应用图标之间的位置关系，目前还在调整这些细节， |
18202781743
approved these changes
Dec 25, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, Ivy233 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 |
|
TAG Bot New tag: 2.0.24 |
Ivy233
force-pushed
the
fix/dock-indicator-distance-constant
branch
3 times, most recently
from
b3c33b2 to
75ad2a1
Compare
1. 修改指示器边距计算，对所有任务栏位置使用 Math.max() 2. 添加最小距离约束（iconSize/2 - 8）防止指示器过于靠近图标边缘 3. 对所有四个任务栏位置应用修复：顶部、底部、左侧、右侧 4. 确保图标缩小时指示器保持一致的视觉距离 Log: 修复图标缩小时任务栏指示器距离变得过小的问题 Influence: 1. 测试图标正常状态下的任务栏指示器显示 2. 验证悬停图标时指示器距离保持恒定 3. 在所有四个任务栏位置测试（顶部、底部、左侧、右侧） 4. 检查不同图标大小设置下的指示器定位 5. 验证不同屏幕分辨率下的视觉一致性 PMS: BUG-309165
Ivy233
force-pushed
the
fix/dock-indicator-distance-constant
branch
from
75ad2a1 to
a6e3959
Compare
deepin pr auto review我来对这个代码变更进行审查：
改进建议： Item {
readonly property int minIndicatorMargin: 8 // 定义最小边距常量
function calculateMargin(size, iconSize) {
return Math.max((size - iconSize) / 2 - minIndicatorMargin,
(size - iconSize) / 2 / 3)
}
// 在case语句中使用
case Dock.Top: {
windowIndicator.anchors.horizontalCenter = iconContainer.horizontalCenter
windowIndicator.anchors.top = parent.top
windowIndicator.anchors.topMargin = Qt.binding(() => {
return calculateMargin(root.height, iconSize)
})
return
}
// 其他case类似
}这样的改进：
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
Fixed a bug where the dock indicator (underline) distance from icons increases when icons are scaled down due to many docked apps.
Problem
When the dock is at maximum height and filled with many app icons:
(height - iconSize) / 2 / 3Solution
Changed the formula to
(height - iconSize) / 2 - 8to maintain a constant 8px distance between icon and indicator regardless of icon size.Modified Files
panels/dock/taskmanager/package/AppItem.qmlpanels/dock/taskmanager/package/AppItemWithTitle.qmlTesting
Summary by Sourcery
Bug Fixes: