-
Notifications
You must be signed in to change notification settings - Fork 42
fix: Fix the issue with about window link styles #551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fix the issue with about window link styles Log: Fix the issue with about window link styles pms: BUG-342613
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#551
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the About dialog’s website label to render rich text link markup correctly and ensure it shows a hand cursor on hover without interfering with click handling. Flow diagram for about dialog website link rendering and hover behaviorflowchart TD
A[AboutDialog opened] --> B[websiteLink and websiteName evaluated]
B --> C{text nonempty?}
C -->|No| D[Label text set to empty string]
C -->|Yes| E[Label text set using __websiteLinkTemplate with RichText]
E --> F[Label displayed with hyperlink styling]
F --> G[Mouse hovers over Label]
G --> H[MouseArea covers Label]
H --> I[Cursor shape set to PointingHandCursor]
I --> J["Clicks handled by rich text link in Label (MouseArea does not accept buttons)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这个diff进行代码审查:
建议改进后的代码: Label {
id: websiteLabel
font: D.DTK.fontManager.t8
textFormat: Text.RichText
text: (control.websiteLink === "" || control.websiteName === "") ?
"" : control.__websiteLinkTemplate.arg(websiteLink).arg(control.websiteName)
wrapMode: Text.WordWrap
Layout.fillWidth: true
MouseArea {
id: mouseArea
anchors.fill: parent
cursorShape: Qt.PointingHandCursor
acceptedButtons: Qt.NoButton
hoverEnabled: true
ToolTip.text: qsTr("Visit website")
ToolTip.visible: mouseArea.containsMouse
}
}这些改进主要是为了提升用户体验,添加了tooltip提示,并显式声明了hoverEnabled属性使代码意图更明确。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Setting
textFormat: Text.RichTextunconditionally may cause unexpected HTML interpretation (e.g.,&,<,>in translated strings); consider only enablingRichTextwhen the template actually contains HTML markup or when bothwebsiteLinkandwebsiteNameare present. - The
MouseAreacovers the label even whenwebsiteLabel.textis empty; consider bindingvisible/enabledto whether a link is actually present so you don’t add a redundant pointer handler in the non-link case.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Setting `textFormat: Text.RichText` unconditionally may cause unexpected HTML interpretation (e.g., `&`, `<`, `>` in translated strings); consider only enabling `RichText` when the template actually contains HTML markup or when both `websiteLink` and `websiteName` are present.
- The `MouseArea` covers the label even when `websiteLabel.text` is empty; consider binding `visible`/`enabled` to whether a link is actually present so you don’t add a redundant pointer handler in the non-link case.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, mhduiy, pengfeixx 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: unstable) |
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#551
Fix the issue with about window link styles
Log: Fix the issue with about window link styles
pms: BUG-342613
Summary by Sourcery
Bug Fixes: