feat: add hover background and adjust spacing for dock app items#1435
feat: add hover background and adjust spacing for dock app items#1435deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideAdds a hover/active background and refines spacing behavior for dock app items by introducing a reusable background component, centralizing spacing constants in TaskManager, wiring them into AppItem, and updating text width calculations in textcalculator.cpp. Sequence diagram for new hover background behavior on dock app itemssequenceDiagram
actor User
participant Dock
participant AppItem
participant HoverHandler
participant AppletItemBackground
User->>Dock: move_cursor_over_app_item
Dock->>HoverHandler: set_hovered(true)
HoverHandler->>AppItem: onHoveredChanged()
AppItem->>AppletItemBackground: update_opacity(1.0)
AppletItemBackground->>AppletItemBackground: NumberAnimation_opacity(duration 150ms)
User->>Dock: move_cursor_away
Dock->>HoverHandler: set_hovered(false)
HoverHandler->>AppItem: onHoveredChanged()
AppItem->>AppletItemBackground: update_opacity(0.0)
AppletItemBackground->>AppletItemBackground: NumberAnimation_opacity(duration 150ms)
Class diagram for updated dock TaskManager, AppItem, and TextCalculator spacing and hover backgroundclassDiagram
class TaskManager {
int dockOrder
real remainingSpacesForTaskManager
int appItemIconSize
int appItemCellWidth
int appItemSpacing
int appTitleSpaceing
real remainingSpacesForSplitWindow
}
class AppItem {
int iconSize
bool enableTitle
bool titleActive
int appTitleSpaceing
}
class TextCalculator {
qreal m_iconSize
qreal m_itemPadding
qreal m_cellSize
void calculateOptimalTextWidth()
}
class AppletItemBackground {
int hoverHeight
int nonSplitWidth
int hoverPadding
int splitWidth
bool enabled
real opacity
}
class HoverHandler {
bool hovered
}
TaskManager "1" --> "*" AppItem : providesSpacing
AppItem "1" o-- "1" AppletItemBackground : hoverBackground
AppItem "1" --> "1" HoverHandler : hoverHandler
TextCalculator "1" --> "1" TaskManager : mirrorsSpacingLogic
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
99fd32d to
e9c6b65
Compare
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
appTitleSpaceingproperty is misspelled and used in multiple places (QML and C++); consider renaming it consistently toappTitleSpacingto avoid confusion and future mistakes. - The title spacing logic is duplicated:
TaskManager.qmlcomputesappTitleSpaceingwhiletextcalculator.cpprecomputes a similar value (qMax(10.0, m_iconSize / 3.0)); aligning both to a single shared source of truth would make future layout changes less error-prone. - The constants used in spacing (e.g.,
+ 2inappItemSpacing +2, the0.8multiplier, and the/ 3divisor) look like magic numbers; consider extracting them into named properties or adding brief inline comments to clarify their intent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `appTitleSpaceing` property is misspelled and used in multiple places (QML and C++); consider renaming it consistently to `appTitleSpacing` to avoid confusion and future mistakes.
- The title spacing logic is duplicated: `TaskManager.qml` computes `appTitleSpaceing` while `textcalculator.cpp` recomputes a similar value (`qMax(10.0, m_iconSize / 3.0)`); aligning both to a single shared source of truth would make future layout changes less error-prone.
- The constants used in spacing (e.g., `+ 2` in `appItemSpacing +2`, the `0.8` multiplier, and the `/ 3` divisor) look like magic numbers; consider extracting them into named properties or adding brief inline comments to clarify their intent.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/package/AppItem.qml:45` </location>
<code_context>
property bool enableTitle: false
property bool titleActive: enableTitle && titleLoader.active
-
+ property int appTitleSpaceing: 0
property var iconGlobalPoint: {
var a = icon
</code_context>
<issue_to_address>
**suggestion (typo):** The `appTitleSpaceing` property name has a typo that may propagate inconsistently through the codebase.
The property is misspelled as `Spaceing` instead of `Spacing`. If you can still change the API, please rename it to `appTitleSpacing` and update all references to keep naming clear and consistent.
Suggested implementation:
```
property bool enableTitle: false
property bool titleActive: enableTitle && titleLoader.active
property int appTitleSpacing: 0
property var iconGlobalPoint: {
```
```
id: appItem
implicitWidth: root.titleActive ? (iconContainer.width + titleLoader.width + root.appTitleSpacing) : iconContainer.width
visible: !root.Drag.active // When in dragging, hide app item
```
There may be other usages of `appTitleSpaceing` across the codebase (QML bindings, JS helpers, or tests). A project-wide search/replace from `appTitleSpaceing` to `appTitleSpacing` should be performed to keep the API consistent and avoid runtime binding errors.
</issue_to_address>
### Comment 2
<location> `panels/dock/taskmanager/package/AppItem.qml:82-91` </location>
<code_context>
anchors.fill: parent
id: appItem
- implicitWidth: root.titleActive ? (iconContainer.width + 4 + titleLoader.width) : iconContainer.width
+ implicitWidth: root.titleActive ? (iconContainer.width + titleLoader.width + root.appTitleSpaceing) : iconContainer.width
visible: !root.Drag.active // When in dragging, hide app item
+ background:AppletItemBackground {
+ id: hoverBackground
+
+ readonly property int hoverHeight: Math.round(root.height * 5 / 6)
+ readonly property int nonSplitWidth: Math.max(hoverHeight - 2, 0)
+ readonly property int hoverPadding: Math.max(Math.round((Panel.rootObject.dockItemMaxSize * 0.8 - root.iconSize) / 2), 0)
+ readonly property int splitWidth: Math.round(icon.width + titleLoader.width + hoverPadding * 3)
+
+ enabled: false
</code_context>
<issue_to_address>
**issue (bug_risk):** Hover background width for split mode doesn’t account for the same title spacing as `implicitWidth`, which may cause visual mismatch.
`implicitWidth` uses `iconContainer.width + titleLoader.width + root.appTitleSpaceing`, while `hoverBackground.splitWidth` uses `icon.width + titleLoader.width + hoverPadding * 3`, omitting `appTitleSpaceing`. This can make the hover background misaligned with the visible/control width. Please derive `splitWidth` from the same components as `implicitWidth` (or centralize the calculation) so the hover state matches the rendered item width.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
f7ae5de to
66a9ef6
Compare
66a9ef6 to
6e1501e
Compare
1. Added AppletItemBackground component to AppItem.qml for visual feedback on hover and active states 2. Introduced appTitleSpaceing property to control spacing between icon and title 3. Updated implicitWidth calculation to use appTitleSpaceing instead of fixed value 4. Added hoverHandler ID to HoverHandler for background control 5. Added new properties in TaskManager.qml for consistent spacing calculations 6. Modified appContainer spacing to adjust based on textCalculator state 7. Updated textcalculator.cpp to include appTitleSpacing in width calculations 8. Implemented smooth opacity transitions for hover background Log: Added hover effect and improved spacing for dock application items Influence: 1. Test hover effects on dock app items with and without titles 2. Verify spacing between icons and titles in different display modes 3. Check background appearance during hover, press, and active states 4. Test with both light and dark themes 5. Verify smooth opacity transitions 6. Test with multiple apps in dock to ensure consistent spacing 7. Check behavior when dragging app items feat: 为任务栏应用项添加悬停背景并调整间距 1. 在 AppItem.qml 中添加 AppletItemBackground 组件,为悬停和活动状态提供 视觉反馈 2. 引入 appTitleSpaceing 属性以控制图标和标题之间的间距 3. 更新 implicitWidth 计算以使用 appTitleSpaceing 替代固定值 4. 为 HoverHandler 添加 hoverHandler ID 以控制背景 5. 在 TaskManager.qml 中添加新属性以实现一致的间距计算 6. 修改 appContainer 间距以根据 textCalculator 状态进行调整 7. 更新 textcalculator.cpp 以在宽度计算中包含 appTitleSpacing 8. 为悬停背景实现平滑的不透明度过渡效果 Log: 新增任务栏应用项悬停效果并优化间距显示 Influence: 1. 测试带标题和不带标题的任务栏应用项悬停效果 2. 验证不同显示模式下图标和标题之间的间距 3. 检查悬停、按下和活动状态下的背景外观 4. 测试浅色和深色主题下的显示效果 5. 验证平滑的不透明度过渡效果 6. 测试多个应用在任务栏中的间距一致性 7. 检查拖拽应用项时的行为表现 PMS: TASK-384101
6e1501e to
ffa2eb0
Compare
deepin pr auto review这段,我将从语法逻辑、代码质量、代码性能和代码安全四个方面对这段 Git diff 进行审查。 1. 语法逻辑审查意见:通过。
2. 代码质量审查意见:良好,有改进空间。
3. 代码性能审查意见:影响较小,需注意绑定频繁更新。
4. 代码安全审查意见:安全。
总结与建议这段代码主要目的是优化 Dock 任务栏中应用图标的间距和标题布局,使其更加动态化和可配置。 建议修改点:
总体而言,这是一次质量较高的代码提交,消除了魔法数字,增强了布局的灵活性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
Log: Added hover effect and improved spacing for dock application items
Influence:
feat: 为任务栏应用项添加悬停背景并调整间距
Log: 新增任务栏应用项悬停效果并优化间距显示
Influence:
PMS: TASK-384101
Summary by Sourcery
Add hover background feedback and spacing refinements for dock task manager application items.
New Features:
Enhancements: