-
Notifications
You must be signed in to change notification settings - Fork 34
fix: Remove hardcoded colors in disk health detection dialog for theme compatibility #184
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors DiskHealthDetectionDialog to rely on theme-driven palettes instead of hardcoded colors while keeping explicit semantic colors only for health state indicators and table delegate rendering. Sequence diagram for theme-driven text color in DiskHealthDetectionDelegatesequenceDiagram
actor "User" as User
participant "Application" as App
participant "DiskHealthDetectionDialog" as Dialog
participant "QTableView" as TableView
participant "DiskHealthDetectionDelegate" as Delegate
participant "QStyle" as Style
"User" ->> "App" : "Open disk health detection dialog"
"App" ->> "Dialog" : "create() and initUI()"
"Dialog" ->> "TableView" : "setItemDelegate(Delegate)"
"User" ->> "System" : "Change theme (light/dark)"
"App" ->> "TableView" : "update() triggered by theme change"
"TableView" ->> "Delegate" : "paint(option, index) for each cell"
"Delegate" ->> "option.palette" : "color(QPalette::Text)"
"option.palette" -->> "Delegate" : "theme-derived text color"
"Delegate" ->> "Style" : "drawControl() with palette text color"
"Style" -->> "TableView" : "cell rendered with correct text color"
"TableView" -->> "User" : "SMART attributes text matches current theme"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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:
- Since the delegate no longer has its text color set from the dialog, consider removing the setTextColor API and any related state from DiskHealthDetectionDelegate if it has become unused to keep the delegate interface minimal and avoid dead code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the delegate no longer has its text color set from the dialog, consider removing the setTextColor API and any related state from DiskHealthDetectionDelegate if it has become unused to keep the delegate interface minimal and avoid dead code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…e compatibility - Remove hardcoded DPalette definitions (palette1-6) in DiskHealthDetectionDialog - Remove all setPalette() calls that prevented theme color following - Use option.palette.color(QPalette::Text) in DiskHealthDetectionDelegate for dynamic theme color - Preserve semantic colors for health status indicators (good/damaged/unknown) Log: Fix disk health panel font color not following dark mode theme Bug: https://pms.uniontech.com/bug-view-341105.html
deepin pr auto review我来对这段代码的变更进行审查:
建议:
总的来说,这是一个很好的重构,提高了代码的可维护性和主题适配性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: itsXuSt, max-lvs 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 |
|
/merge |
Log: Fix disk health panel font color not following dark mode theme
Bug: https://pms.uniontech.com/bug-view-341105.html
Summary by Sourcery
Align disk health detection dialog colors with the active application theme to fix dark mode font visibility issues.
Bug Fixes:
Enhancements: