Skip to content

fix(reader): fix scrollbar invisible in dark mode by forcing light scrollbar palette - #406

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/snipefrom
Resurgamz:agent/pms-bug-bot/bug-374553-snipe
Sep 22, 2026
Merged

deepin-bot[bot] merged 1 commit into
linuxdeepin:release/snipefrom
Resurgamz:agent/pms-bug-bot/bug-374553-snipe

Conversation

@Resurgamz

@Resurgamz Resurgamz commented Sep 22, 2026 •

Copy link
Copy Markdown

根因分析 / Root Cause

SheetBrowser 继承 DGraphicsView(实为 typedef QGraphicsView),其滚动条是普通 QScrollBar,由 DTK 的 ChameleonStyle 绘制。ChameleonStyle::drawControl(CE_ScrollBarSlider) 依据滚动条 QPalette::Base 的明暗决定滑块颜色:

QColor lineColor(opt->palette.color(QPalette::Base));
if (toColorType(lineColor) == LightType)
    // 浅色背景 -> 绘制深色滑块
else
    // 深色背景 -> 绘制白色半透明滑块

深色模式下 QPalette::Base 为深色,ChameleonStyle 因此绘制白色半透明滑块;而文档页面恒为白色,滑块在白色页面上几乎不可见(测试描述为“透明色”)。WA_TranslucentBackground 使滚动条 groove 透出底层,进一步降低对比度。

关键证据:dde-qt5integration 5.6.16 styleplugins/chameleon/chameleonstyle.cpp 中 CE_ScrollBarSlider 的取色逻辑仅依赖 palette(Base),与 State_Active 无关——因此强加 State_Active 的 QProxyStyle 方案无效。

SheetBrowser extends DGraphicsView (a typedef of QGraphicsView); its scrollbars are plain QScrollBar painted by DTK ChameleonStyle. The handle color is chosen from the scrollbar's QPalette::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::Base of both scrollbars to a light color on theme update so ChameleonStyle always draws a dark handle; keep the themeTypeChanged sync to re-apply the scrollbar palette and view background. The previous ScrollBarActiveStyle (forcing State_Active) is removed since it does not affect the handle color logic. WA_TranslucentBackground is restored to keep the original view background behavior.

改动安全评估 / Change Safety

  • 风险等级:低。仅调整两个滚动条的 palette,不改变任何函数签名、类成员或调用方;WA_TranslucentBackground 恢复为原状。
  • Risk: low. Only the two scrollbars' palettes are adjusted; no signature, member or caller changes; WA_TranslucentBackground is restored.

验证建议 / Verification

  • 深色模式下打开多页 PDF,放大到出现水平/垂直滚动条,确认滑块清晰可见;
  • 运行中切换浅色/深色主题,确认滚动条颜色随之刷新;
  • 浅色模式无回归;覆盖 PDF / DOCX / DJVU 等格式。

Summary by Sourcery

Keep SheetBrowser scrollbars visible across dark mode and runtime theme changes.

Bug Fixes:

  • Ensure document-view scrollbars remain clearly visible in dark mode by applying a light scrollbar palette.
  • Refresh scrollbar palettes and the viewport background when the application theme changes.

…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
@sourcery-ai

sourcery-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

Fix 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 synchronization

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Force scrollbars to use a light Base palette so ChameleonStyle renders a dark, visible handle against the always-light document viewport.
  • Add a theme-update slot that applies the application palette to both scrollbars while overriding QPalette::Base to white.
  • Invoke palette synchronization during construction and on DGuiApplicationHelper theme changes.
  • Preserve viewport background handling and remove reliance on ineffective active-state styling.
reader/browser/SheetBrowser.cpp
reader/browser/SheetBrowser.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

auto syncScrollBarPalette = [&appPalette](QScrollBar *bar) {
QPalette barPalette = appPalette;
barPalette.setColor(QPalette::Base, Qt::white);
bar->setPalette(barPalette);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 99 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 99 分,大于 70 分通过阈值,代码质量优秀。修复深色模式下滚动条不可见问题,实现清晰,注释详尽,无安全漏洞。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 语法正确,逻辑清晰。构造函数中先 connect 再调用 onUpdateTheme() 覆盖初始状态,时序合理。


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. reader/browser/SheetBrowser.cpp:2480 - 方法名 onUpdateTheme 略显笼统,未准确反映其仅更新滚动条调色板的功能,建议命名为 onUpdateScrollBarPalette

建议: 代码结构清晰,注释完整。注释详细解释了 DTK ChameleonStyle 依据 Base 明暗决定滑块颜色的机制,有助于后续维护。lambda 避免了对两个滚动条的重复代码。


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 性能良好,资源使用合理。onUpdateTheme() 仅在主题切换信号触发时调用,调色板操作为 O(1) 轻量操作,无性能瓶颈。


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 存在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 代码审查工具自动生成

@deepin-ci-robot

Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Resurgamz

Copy link
Copy Markdown
Author

/merge

@deepin-bot
deepin-bot Bot merged commit 7249a6f into linuxdeepin:release/snipe Sep 22, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants