feat: add hover background and adjust window indicator position#1437
feat: add hover background and adjust window indicator position#1437wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Reviewer's GuideIntroduces an AppletItemBackground hover/active background for dock app items, aligns the window indicator margins to this new background size across all dock edges, and removes inter-item spacing in the TaskManager layout to accommodate the new visual design. Sequence diagram for hover-triggered AppletItemBackground behaviorsequenceDiagram
actor User
participant HoverHandler as hoverHandler
participant AppItem as appItem
participant AppletItemBackground as hoverBackground
participant WindowIndicator as windowIndicator
User->>HoverHandler: move_pointer_over_app_item
HoverHandler->>AppItem: hovered_changed_true
AppItem->>hoverBackground: set_hovered_true
hoverBackground->>hoverBackground: compute_width_height_based_on_title_and_icon
hoverBackground->>hoverBackground: set_opacity_1_with_NumberAnimation_150ms
hoverBackground->>hoverBackground: update_palette_for_hovered_state
User->>HoverHandler: move_pointer_away
HoverHandler->>AppItem: hovered_changed_false
AppItem->>hoverBackground: set_hovered_false
hoverBackground->>hoverBackground: set_opacity_0_with_NumberAnimation_150ms
AppItem->>windowIndicator: update_margins_using_hoverBackground_nonSplitHeight_and_nonSplitWidth
windowIndicator->>windowIndicator: align_to_dock_edge_with_consistent_spacing
Class diagram for AppItem, AppletItemBackground, WindowIndicator, and TaskManager changesclassDiagram
class AppItem {
int iconSize
bool titleActive
bool active
var windows
AppletItemBackground hoverBackground
Item iconContainer
void updateWindowIndicatorAnchors(DockEdge edge)
}
class AppletItemBackground {
int verticalSpacing
int horizontalSpacing
int nonSplitHeight
int hoverPadding
int splitWidth
int nonSplitWidth
bool enabled
int z
int width
int height
int radius
real opacity
bool visible
bool isActive
DPalette backgroundColor
DPalette insideBorderColor
DPalette outsideBorderColor
void animateOpacity(NumberAnimation animation)
}
class WindowIndicator {
AnchorGroup anchors
void setTopMargin(int margin)
void setBottomMargin(int margin)
void setLeftMargin(int margin)
void setRightMargin(int margin)
}
class TaskManager {
bool useColumnLayout
int appItemSpacing
}
class AppContainer {
bool useColumnLayout
int spacing
}
AppItem --> AppletItemBackground : owns_hoverBackground
AppItem --> WindowIndicator : positions
AppItem --> AppContainer : contained_in
TaskManager --> AppContainer : configures_useColumnLayout_and_spacing
class DPalette {
color normal_common
color normal_crystal
color normalDark_crystal
color hovered_crystal
color hoveredDark_crystal
color pressed_crystal
color pressedDark_crystal
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
AppletItemBackgroundonly hasanchors.verticalCenterset and itsxbinding is commented out, so it will default tox: 0; consider restoring or simplifying the horizontal positioning binding so the background is centered/aligned with the icon as intended. - The
windowIndicatormargin bindings now duplicate similar expressions for each dock edge usinghoverBackground.nonSplitHeight/nonSplitWidth; consider extracting this into a small helper or shared binding to avoid subtle inconsistencies between edges when tweaking the layout in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `AppletItemBackground` only has `anchors.verticalCenter` set and its `x` binding is commented out, so it will default to `x: 0`; consider restoring or simplifying the horizontal positioning binding so the background is centered/aligned with the icon as intended.
- The `windowIndicator` margin bindings now duplicate similar expressions for each dock edge using `hoverBackground.nonSplitHeight/nonSplitWidth`; consider extracting this into a small helper or shared binding to avoid subtle inconsistencies between edges when tweaking the layout in the future.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/package/AppItem.qml:145-151` </location>
<code_context>
+ common: ("transparent")
+ crystal: Qt.rgba(1.0, 1.0, 1.0, 0.20)
+ }
+ normalDark: {
+ crystal: Qt.rgba(1.0, 1.0, 1.0, 0.05)
+ }
+ hovered: normal
+ hoveredDark: normalDark
+ pressed: hovered
+ pressedDark: normalDark
+ }
+ outsideBorderColor: D.Palette {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `normalDark:` syntax here likely breaks QML object syntax for the palette group.
Other `D.Palette` groups use `stateName { ... }` without a colon. `normalDark: { ... }` is not valid QML object syntax and may fail to parse or be ignored, which also makes `hoveredDark: normalDark` and `pressedDark: normalDark` unreliable. This should be `normalDark { ... }` to match the other palette groups and ensure these bindings work correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a7bd732 to
034a04f
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds visual feedback improvements to dock app items by introducing a hover/active background component and adjusting window indicator positioning to align with the new background layout.
Changes:
- Added AppletItemBackground component to AppItem with dynamic sizing based on title visibility and smooth opacity transitions
- Updated window indicator positioning formulas to align with hover background dimensions instead of icon size
- Modified TaskManager spacing from calculated value to zero to accommodate the new background-based layout
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| panels/dock/taskmanager/package/TaskManager.qml | Changed app container spacing from taskmanager.appItemSpacing to 0 to accommodate new background layout |
| panels/dock/taskmanager/package/AppItem.qml | Added AppletItemBackground component with dynamic sizing and updated window indicator positioning to align with background dimensions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
034a04f to
afe4c43
Compare
Added AppletItemBackground component to AppItem for better visual feedback during hover and active states. The background features dynamic sizing based on title visibility with smooth opacity transitions. Updated window indicator positioning to align with the new background dimensions instead of icon size, ensuring consistent spacing. Modified TaskManager spacing to zero to accommodate new background layout. Log: Improved dock app item visual feedback with hover background and adjusted window indicator positioning Influence: 1. Test hover effects on dock app items in both normal and dark themes 2. Verify window indicator positioning at all dock edges (top, bottom, left, right) 3. Check active state visual feedback for running applications 4. Test background appearance with and without title display 5. Verify smooth opacity transitions during hover state changes 6. Test layout consistency when dock items are rearranged or dragged feat: 添加悬停背景并调整窗口指示器位置 为AppItem添加AppletItemBackground组件,提供悬停和激活状态下的视觉反 馈。背景根据标题可见性动态调整尺寸,具有平滑的透明度过渡效果。更新窗 口指示器定位,使其与新的背景尺寸对齐而非图标尺寸,确保间距一致性。修改 TaskManager间距为零以适应新的背景布局。 Log: 改进任务栏应用项视觉反馈,添加悬停背景并调整窗口指示器定位 Influence: 1. 在正常和深色主题下测试任务栏应用项的悬停效果 2. 验证窗口指示器在所有任务栏边缘(顶部、底部、左侧、右侧)的定位 3. 检查运行中应用的激活状态视觉反馈 4. 测试带标题和不带标题显示时的背景外观 5. 验证悬停状态变化时的平滑透明度过渡 6. 测试任务栏项目重新排列或拖动时的布局一致性 PMS: BUG-345931
afe4c43 to
2c2646d
Compare
deepin pr auto review这份代码变更主要针对 Dock(任务栏)的应用图标项(AppItem)进行了视觉和布局逻辑的重构,重点在于引入了 以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议
总体而言,这次修改使得布局结构更加模块化(引入了独立的背景组件),但需要仔细校对尺寸计算的一致性,以保证视觉效果的稳定性。 |
| anchors.fill: parent | ||
| id: appItem | ||
| implicitWidth: root.titleActive ? (iconContainer.width + titleLoader.width + root.appTitleSpacing) : iconContainer.width | ||
| implicitWidth: root.titleActive ? (iconContainer.width + titleLoader.width + root.appTitleSpacing) : iconContainer.width + root.appTitleSpacing |
There was a problem hiding this comment.
把textcalcator这个计算方式也同步一下呀,
Added AppletItemBackground component to AppItem for better visual feedback during hover and active states. The background features dynamic sizing based on title visibility with smooth opacity transitions. Updated window indicator positioning to align with the new background dimensions instead of icon size, ensuring consistent spacing. Modified TaskManager spacing to zero to accommodate new background layout.
Log: Improved dock app item visual feedback with hover background and adjusted window indicator positioning
Influence:
feat: 添加悬停背景并调整窗口指示器位置
为AppItem添加AppletItemBackground组件,提供悬停和激活状态下的视觉反
馈。背景根据标题可见性动态调整尺寸,具有平滑的透明度过渡效果。更新窗
口指示器定位,使其与新的背景尺寸对齐而非图标尺寸,确保间距一致性。修改
TaskManager间距为零以适应新的背景布局。
Log: 改进任务栏应用项视觉反馈,添加悬停背景并调整窗口指示器定位
Influence:
PMS: BUG-345931
Summary by Sourcery
Improve dock app item visual feedback with a dynamic hover background and align window indicators with the new background layout.
New Features:
Enhancements: