fix(network): improve copy button interaction and icon on network details page#578
Open
iCancely wants to merge 2 commits into
Open
fix(network): improve copy button interaction and icon on network details page#578iCancely wants to merge 2 commits into
iCancely wants to merge 2 commits into
Conversation
1. Set left label text to T6 font size on detail list items 2. Set right content text to T7 font size on detail list items Log: Adjusted font sizes for labels and content on network details page Influence: Detail page text follows design spec font sizes fix(network): 调整网络详情页面字体大小 1. 将详情列表左侧标签文字字号设置为 T6 2. 将详情列表右侧内容文字字号设置为 T7 Log: 调整网络详情页面的标签和内容字体大小 PMS: BUG-370973 Influence: 详情页文字符合设计规范字号
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: iCancely 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 GuideRefactors the copy action on the Network Details page to use a proper action button with hover/pressed feedback, a dedicated background hit area, and updated typography for list items to align with design guidelines. Sequence diagram for updated copy action on Network Details pagesequenceDiagram
actor User
participant PageDetails
participant ActionButton
participant dccData
participant ToolTip
User->>ActionButton: click
ActionButton->>ActionButton: onClicked
ActionButton->>ActionButton: build text from infoItem.name and infoItem.details
ActionButton->>dccData: setClipboard(text)
ActionButton->>ToolTip: show("Details has been copied", 2000)
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 - I've found 2 issues, and left some high level feedback:
- The
Rectanglebackground definespressedColorandhoveredColorasD.Paletteproperties but thecolorbinding usesD.ColorSelector.pressedColor/hoveredColorinstead of those properties, so either wire theD.PaletteintoD.ColorSelectoror remove the unused properties and rely consistently on one mechanism. - The hover/pressed background colors are hard-coded with
Qt.rgba(...); consider deriving these from the existing theme/color system (e.g., viaD.ColorSelectoror palette tokens) so the button appearance remains consistent with the rest of the UI and adapts to theme changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `Rectangle` background defines `pressedColor` and `hoveredColor` as `D.Palette` properties but the `color` binding uses `D.ColorSelector.pressedColor/hoveredColor` instead of those properties, so either wire the `D.Palette` into `D.ColorSelector` or remove the unused properties and rely consistently on one mechanism.
- The hover/pressed background colors are hard-coded with `Qt.rgba(...)`; consider deriving these from the existing theme/color system (e.g., via `D.ColorSelector` or palette tokens) so the button appearance remains consistent with the rest of the UI and adapts to theme changes.
## Individual Comments
### Comment 1
<location path="dcc-network/qml/PageDetails.qml" line_range="80-89" />
<code_context>
- }
- dccData.setClipboard(text.join("\n"))
- tip.show(qsTr("Details has been copied"), 2000)
+ property D.Palette pressedColor: D.Palette {
+ normal: Qt.rgba(0, 0, 0, 0.2)
+ normalDark: Qt.rgba(1, 1, 1, 0.25)
+ }
+ property D.Palette hoveredColor: D.Palette {
+ normal: Qt.rgba(0, 0, 0, 0.1)
+ normalDark: Qt.rgba(1, 1, 1, 0.1)
+ }
+ radius: 6
+ color: parent.pressed ? D.ColorSelector.pressedColor : (parent.hovered ? D.ColorSelector.hoveredColor : "transparent")
+ }
+ focusPolicy: Qt.StrongFocus
</code_context>
<issue_to_address>
**issue (bug_risk):** Rectangle color binding ignores the local pressed/hovered palettes and may use the wrong type/value.
`pressedColor` and `hoveredColor` are declared as `D.Palette`, but the `color` binding uses `D.ColorSelector.pressedColor/hoveredColor` instead, leaving the local palettes unused and risking a type mismatch if `D.ColorSelector.*` isn’t a plain color. Bind `color` to the local palettes instead, e.g. `color: parent.pressed ? pressedColor.normal : parent.hovered ? hoveredColor.normal : "transparent"`, or whichever variant matches your theme API.
</issue_to_address>
### Comment 2
<location path="dcc-network/qml/PageDetails.qml" line_range="98" />
<code_context>
- text.push(infoItem.details[i][0] + "\t" + infoItem.details[i][1])
- }
- dccData.setClipboard(text.join("\n"))
- tip.show(qsTr("Details has been copied"), 2000)
+ property D.Palette pressedColor: D.Palette {
+ normal: Qt.rgba(0, 0, 0, 0.2)
</code_context>
<issue_to_address>
**nitpick (typo):** Tooltip text is slightly ungrammatical and might be clearer as plural.
Minor wording nit: consider changing `qsTr("Details has been copied")` to something like `qsTr("Details have been copied")` or `qsTr("Details copied to clipboard")` for more natural, grammatical text.
```suggestion
tip.show(qsTr("Details copied to clipboard"), 2000)
```
</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
+80
to
+89
| property D.Palette pressedColor: D.Palette { | ||
| normal: Qt.rgba(0, 0, 0, 0.2) | ||
| normalDark: Qt.rgba(1, 1, 1, 0.25) | ||
| } | ||
| property D.Palette hoveredColor: D.Palette { | ||
| normal: Qt.rgba(0, 0, 0, 0.1) | ||
| normalDark: Qt.rgba(1, 1, 1, 0.1) | ||
| } | ||
| radius: 6 | ||
| color: parent.pressed ? D.ColorSelector.pressedColor : (parent.hovered ? D.ColorSelector.hoveredColor : "transparent") |
There was a problem hiding this comment.
issue (bug_risk): Rectangle color binding ignores the local pressed/hovered palettes and may use the wrong type/value.
pressedColor and hoveredColor are declared as D.Palette, but the color binding uses D.ColorSelector.pressedColor/hoveredColor instead, leaving the local palettes unused and risking a type mismatch if D.ColorSelector.* isn’t a plain color. Bind color to the local palettes instead, e.g. color: parent.pressed ? pressedColor.normal : parent.hovered ? hoveredColor.normal : "transparent", or whichever variant matches your theme API.
| text.push(infoItem.details[i][0] + "\t" + infoItem.details[i][1]) | ||
| } | ||
| dccData.setClipboard(text.join("\n")) | ||
| tip.show(qsTr("Details has been copied"), 2000) |
There was a problem hiding this comment.
nitpick (typo): Tooltip text is slightly ungrammatical and might be clearer as plural.
Minor wording nit: consider changing qsTr("Details has been copied") to something like qsTr("Details have been copied") or qsTr("Details copied to clipboard") for more natural, grammatical text.
Suggested change
| tip.show(qsTr("Details has been copied"), 2000) | |
| tip.show(qsTr("Details copied to clipboard"), 2000) |
…ails page Replace IconLabel with ActionButton for better hover/pressed feedback. Add 30x30px hover/pressed background with 6px radius. Right-align the copy button in the header. Update copy icon resource to match foreground color. Log: Improved copy button UX on network details page PMS: https://pms.uniontech.com/bug-view-370975.html Influence: Network details page copy button interaction and appearance fix(network): 优化网络详情页复制按钮交互和图标 将 IconLabel 替换为 ActionButton 以支持 hover/pressed 反馈。 添加 30x30px hover/pressed 背景，圆角 6px。 复制按钮在标题栏中右对齐。 更新 copy 图标资源以匹配前景色。 Log: 优化网络详情页复制按钮交互和外观 PMS: https://pms.uniontech.com/bug-view-370975.html Influence: 网络详情页复制按钮的交互和外观
iCancely
force-pushed
the
fix/network-details-copy-button
branch
from
July 22, 2026 08:35
5b660db to
a706fb2
Compare
Contributor
|
TAG Bot New tag: 2.0.97 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
fix(network): improve copy button interaction and icon on network details page
Replace IconLabel with ActionButton for better hover/pressed feedback.
Add 30x30px hover/pressed background with 6px radius.
Right-align the copy button in the header.
Update copy icon resource to match foreground color.
Log: Improved copy button UX on network details page
PMS: https://pms.uniontech.com/bug-view-370975.html
Influence: Network details page copy button interaction and appearance
fix(network): 优化网络详情页复制按钮交互和图标
将 IconLabel 替换为 ActionButton 以支持 hover/pressed 反馈。
添加 30x30px hover/pressed 背景，圆角 6px。
复制按钮在标题栏中右对齐。
更新 copy 图标资源以匹配前景色。
Log: 优化网络详情页复制按钮交互和外观
PMS: https://pms.uniontech.com/bug-view-370975.html
Influence: 网络详情页复制按钮的交互和外观