fix(reader): fix scrollbar invisible in dark mode by forcing light scrollbar palette - #406
Conversation
…rollbar palette 1. 主题更新时强制两个滚动条的 QPalette::Base 为浅色,使滑块恒为深色; 2. 构造函数中连接 themeTypeChanged 并立即应用,覆盖深色模式下的初始状态; 3. 不改变视口/护眼模式背景逻辑,避免与 EyeProtectionManager 冲突; ===================================== 1. forced QPalette::Base of both scrollbars to a light color on theme update so the handle is always drawn dark; 2. connected themeTypeChanged and applied it immediately in the constructor to cover the initial dark-mode state; 3. left the viewport/eye-protection background logic untouched to avoid conflicting with EyeProtectionManager; Log: 修复深色模式下文档查看器滚动条透明不可见的问题,放大文档后滚动条清晰可辨 PMS: BUG-374553
Reviewer's GuideFix dark-mode scrollbar visibility by synchronizing both scrollbar palettes on initialization and theme changes, forcing a white Base so DTK ChameleonStyle chooses a dark handle while retaining existing viewport background behavior. Sequence diagram for theme-aware scrollbar palette synchronizationsequenceDiagram
participant SheetBrowser
participant DGuiApplicationHelper
participant QScrollBar
participant ChameleonStyle
SheetBrowser->>DGuiApplicationHelper: applicationPalette()
SheetBrowser->>QScrollBar: setPalette(Base = Qt::white)
SheetBrowser->>QScrollBar: setPalette(Base = Qt::white)
DGuiApplicationHelper-->>SheetBrowser: themeTypeChanged
SheetBrowser->>DGuiApplicationHelper: applicationPalette()
SheetBrowser->>QScrollBar: setPalette(Base = Qt::white)
SheetBrowser->>QScrollBar: setPalette(Base = Qt::white)
ChameleonStyle->>QScrollBar: drawControl(CE_ScrollBarSlider)
ChameleonStyle-->>QScrollBar: draw dark scrollbar handle
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="reader/browser/SheetBrowser.cpp" line_range="2491" />
<code_context>
+ auto syncScrollBarPalette = [&appPalette](QScrollBar *bar) {
+ QPalette barPalette = appPalette;
+ barPalette.setColor(QPalette::Base, Qt::white);
+ bar->setPalette(barPalette);
+ };
+ syncScrollBarPalette(verticalScrollBar());
</code_context>
<issue_to_address>
**issue (broader_impact):** `barPalette.setColor(QPalette::Base, Qt::white)` forces ChameleonStyle to draw a dark scrollbar handle even when EyeProtectionManager is in Night mode, where the document page and viewport are dark (`#1E1E1E`/`#252525`); the handle therefore loses contrast and becomes difficult to see in the mode this fix is intended to cover.
**Triggers:** When EyeProtectionManager is in Night mode and a scrollbar is visible.
**Suggested fix:** Choose the scrollbar Base color based on the effective document/viewport background, or reapply a light Base only when the page background is light.
</issue_to_address>| auto syncScrollBarPalette = [&appPalette](QScrollBar *bar) { | ||
| QPalette barPalette = appPalette; | ||
| barPalette.setColor(QPalette::Base, Qt::white); | ||
| bar->setPalette(barPalette); |
There was a problem hiding this comment.
issue (broader_impact): barPalette.setColor(QPalette::Base, Qt::white) forces ChameleonStyle to draw a dark scrollbar handle even when EyeProtectionManager is in Night mode, where the document page and viewport are dark (#1E1E1E/#252525); the handle therefore loses contrast and becomes difficult to see in the mode this fix is intended to cover.
Triggers: When EyeProtectionManager is in Night mode and a scrollbar is visible.
Suggested fix: Choose the scrollbar Base color based on the effective document/viewport background, or reapply a light Base only when the page background is light.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 语法正确,逻辑清晰。构造函数中先 connect 再调用 onUpdateTheme() 覆盖初始状态,时序合理。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 代码结构清晰,注释完整。注释详细解释了 DTK ChameleonStyle 依据 Base 明暗决定滑块颜色的机制,有助于后续维护。lambda 避免了对两个滚动条的重复代码。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 性能良好,资源使用合理。onUpdateTheme() 仅在主题切换信号触发时调用,调色板操作为 O(1) 轻量操作,无性能瓶颈。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 存在0个安全漏洞。代码仅涉及 Qt 调色板操作,无用户输入处理、无网络/文件操作、无命令执行,安全合规。 💡 改进建议代码示例// 建议将方法名改为更准确的描述
void SheetBrowser::onUpdateScrollBarPalette()
{
const QPalette appPalette = DGuiApplicationHelper::instance()->applicationPalette();
auto syncScrollBarPalette = [&appPalette](QScrollBar *bar) {
QPalette barPalette = appPalette;
barPalette.setColor(QPalette::Base, Qt::white);
bar->setPalette(barPalette);
};
syncScrollBarPalette(verticalScrollBar());
syncScrollBarPalette(horizontalScrollBar());
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, Resurgamz 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 |
根因分析 / Root Cause
SheetBrowser继承DGraphicsView(实为typedef QGraphicsView),其滚动条是普通QScrollBar,由 DTK 的 ChameleonStyle 绘制。ChameleonStyle::drawControl(CE_ScrollBarSlider)依据滚动条QPalette::Base的明暗决定滑块颜色:深色模式下
QPalette::Base为深色,ChameleonStyle 因此绘制白色半透明滑块;而文档页面恒为白色,滑块在白色页面上几乎不可见(测试描述为“透明色”)。WA_TranslucentBackground使滚动条 groove 透出底层,进一步降低对比度。关键证据:
dde-qt5integration5.6.16styleplugins/chameleon/chameleonstyle.cpp中CE_ScrollBarSlider的取色逻辑仅依赖palette(Base),与State_Active无关——因此强加State_Active的QProxyStyle方案无效。SheetBrowserextendsDGraphicsView(atypedefofQGraphicsView); its scrollbars are plainQScrollBarpainted by DTK ChameleonStyle. The handle color is chosen from the scrollbar'sQPalette::Base: a dark Base (dark mode) makes ChameleonStyle draw a translucent white handle, which is nearly invisible on the always-white document page.修复方案 / Fix
在主题更新时强制两个滚动条的
QPalette::Base为浅色,使 ChameleonStyle 始终绘制深色滑块;同时保留themeTypeChanged同步,主题切换时重新应用滚动条调色板与视图背景。移除此前的ScrollBarActiveStyle(强加State_Active不影响滑块取色逻辑,已确认无效),并恢复WA_TranslucentBackground保持视图原有背景行为。Force
QPalette::Baseof both scrollbars to a light color on theme update so ChameleonStyle always draws a dark handle; keep thethemeTypeChangedsync to re-apply the scrollbar palette and view background. The previousScrollBarActiveStyle(forcingState_Active) is removed since it does not affect the handle color logic.WA_TranslucentBackgroundis restored to keep the original view background behavior.改动安全评估 / Change Safety
WA_TranslucentBackground恢复为原状。WA_TranslucentBackgroundis restored.验证建议 / Verification
Summary by Sourcery
Keep SheetBrowser scrollbars visible across dark mode and runtime theme changes.
Bug Fixes: