Skip to content

fix: remove redundant palette setup in RoundScrollArea#351

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-315535
Aug 18, 2025
Merged

fix: remove redundant palette setup in RoundScrollArea#351
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-315535

Conversation

@wjyrich

@wjyrich wjyrich commented Aug 15, 2025 •
edited by sourcery-ai Bot
Loading

Copy link
Copy Markdown
Contributor
  1. Removed unnecessary palette initialization in constructor that was setting window brush to transparent
  2. Changed paintEvent to directly use Qt::transparent instead of palette().window()
  3. This simplifies the code while maintaining the same visual effect of transparency
  4. The palette setup was redundant since we're manually painting the background anyway

fix: 移除RoundScrollArea中多余的调色板设置

  1. 移除构造函数中不必要的调色板初始化（将窗口画刷设置为透明）
  2. 修改paintEvent直接使用Qt::transparent替代palette().window()
  3. 在保持相同透明视觉效果的同时简化了代码
  4. 调色板设置是多余的，因为我们已经手动绘制背景

Pms: bug-315535

Summary by Sourcery

Simplify RoundScrollArea by removing redundant palette setup and directly using Qt::transparent in paintEvent for background transparency

Enhancements:

  • Remove unnecessary palette initialization in RoundScrollArea constructor
  • Use Qt::transparent directly in paintEvent instead of palette().window()

@sourcery-ai

sourcery-ai Bot commented Aug 15, 2025

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Removed redundant palette configuration and directly used Qt::transparent in paintEvent to simplify background drawing in RoundScrollArea.

Class diagram for updated RoundScrollArea transparency handling

classDiagram
    class RoundScrollArea {
        - int m_radius
        + RoundScrollArea(QWidget *parent)
        + setRadius(int radius)
        + paintEvent(QPaintEvent *e)
    }
    QScrollArea <|-- RoundScrollArea
    %% Note: Palette setup removed from constructor; paintEvent now uses Qt::transparent directly
Loading

File-Level Changes

Change Details Files
Removed redundant palette initialization from constructor
  • Deleted DPalette instantiation
  • Removed setting Window brush to transparent
  • Removed DPaletteHelper setPalette call
 plugins/dde-dock/util/roundscrollarea.cpp
Simplified paintEvent to use Qt::transparent
  • Replaced painter.fillPath parameter palette().window() with Qt::transparent
 plugins/dde-dock/util/roundscrollarea.cpp
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[bot]
sourcery-ai Bot reviewed Aug 15, 2025
View reviewed changes

@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 there - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

18202781743
18202781743 previously approved these changes Aug 15, 2025
View reviewed changes
18202781743
18202781743 reviewed Aug 15, 2025
View reviewed changes
Comment thread plugins/dde-dock/util/roundscrollarea.cpp Outdated
path.addRoundedRect(viewport()->rect(), m_radius, m_radius);
painter.setClipPath(path);
painter.fillPath(path, palette().window());
painter.fillPath(path, Qt::transparent);

@18202781743 18202781743 Aug 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这个还有必要绘制么？？都是透明的了，

@wjyrich
fix: remove redundant palette setup in RoundScrollArea
5c11855 
1. Removed unnecessary palette initialization in constructor that was
setting window brush to transparent
2. Changed paintEvent to directly use Qt::transparent instead of
palette().window()
3. This simplifies the code while maintaining the same visual effect
of transparency
4. The palette setup was redundant since we're manually painting the
background anyway

fix: 移除RoundScrollArea,使用QScrollArea.

1. 移除RoundScrollArea
2. 修改paintEvent直接使用Qt::transparent替代palette().window()
3. 在保持相同透明视觉效果的同时简化了代码
4. 调色板设置是多余的，因为我们已经手动绘制背景

Pms: bug-315535
@deepin-ci-robot

deepin-ci-robot commented Aug 18, 2025

Copy link
Copy Markdown

deepin pr auto review

根据提供的git diff，我来进行代码审查，并提出改进意见：

1. 代码逻辑分析

这次修改主要是将自定义的RoundScrollArea替换为Qt标准的QScrollArea，并移除了相关的圆角滚动区域实现。具体变化包括：

  • 从CMakeLists.txt中移除了roundscrollarea.h和roundscrollarea.cpp的引用
  • 在brightnessapplet.h中将RoundScrollArea替换为QScrollArea
  • 在brightnessapplet.cpp中将RoundScrollArea实例化为QScrollArea
  • 移除了设置圆角的代码
  • 删除了roundscrollarea.h和roundscrollarea.cpp文件

2. 语法逻辑评估

语法上没有问题，代码可以正常编译和运行。但是需要注意：

  • 移除了圆角功能可能导致UI外观发生变化
  • 原有的圆角滚动区域可能有一些特殊行为被移除

3. 代码质量改进

  1. UI一致性：

    • 如果其他地方还在使用圆角滚动区域，这次修改会导致UI不一致
    • 建议确认是否所有滚动区域都需要统一改为直角设计

  2. 代码复用：

    • 删除的自定义组件可能在其他地方也有使用
    • 建议全局搜索确认roundscrollarea是否在其他地方也有引用

  3. 注释维护：

    • 删除文件时保留了版权注释，这是好的实践
    • 建议在提交信息中说明删除原因，方便后续维护

4. 性能评估

  1. 性能影响：

    • 使用标准QScrollArea通常比自定义实现性能更好
    • 移除了自定义绘制逻辑，减少了渲染开销

  2. 内存使用：

    • 删除自定义类可能略微减少内存占用
    • 影响很小，可以忽略不计

5. 安全性评估

  1. 潜在风险：

    • 如果有其他代码依赖RoundScrollArea的特定行为，可能导致问题
    • 建议确保所有依赖都已更新

  2. 类型安全：

    • 从自定义类型改为标准Qt类型，提高了类型安全性
    • 减少了潜在的类型转换问题

改进建议

  1. UI一致性检查：

    // 建议在修改前检查所有使用滚动区域的地方
// 确认是否需要统一设计风格

  2. 渐进式迁移：

    // 考虑先保留RoundScrollArea，标记为deprecated
// 然后逐步将使用处改为QScrollArea
// 最后再完全删除

  3. 文档更新：

    • 更新相关UI设计文档，说明滚动区域样式的变更
    • 在代码注释中说明此次修改的原因

  4. 测试覆盖：

    • 增加UI测试用例，确保滚动区域功能正常
    • 特别测试边界情况，如内容超出显示区域时的行为

总体而言，这次修改简化了代码实现，提高了维护性，但需要注意UI一致性和潜在依赖问题。建议在全面测试后再合并此修改。

@deepin-ci-robot

deepin-ci-robot commented Aug 18, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, wjyrich

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

@wjyrich

wjyrich commented Aug 18, 2025

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit cdab9da into linuxdeepin:master Aug 18, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sourcery-ai sourcery-ai[bot] sourcery-ai[bot] left review comments

@18202781743 18202781743 18202781743 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wjyrich @deepin-ci-robot @18202781743