Skip to content

fix: clip settings dialog list views to prevent scroll bleed-through#675

Open
JWWTSL wants to merge 1 commit into
linuxdeepin:masterfrom
JWWTSL:master
Open

fix: clip settings dialog list views to prevent scroll bleed-through#675
JWWTSL wants to merge 1 commit into
linuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL
Copy link
Copy Markdown
Contributor

@JWWTSL JWWTSL commented May 27, 2026

log: Fix settings dialog showing scrolled items over the title bar

pms: bug-345159

Summary by Sourcery

Bug Fixes:

  • Prevent settings dialog list items from scrolling visibly over the dialog title bar by clipping the content and navigation views.

log: Fix settings dialog showing scrolled items over the title bar

pms: bug-345159
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL

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

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 27, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Sets clip on settings dialog list container views to prevent scrolling list items from rendering over the dialog title bar when combined with the transparent dialog background.

File-Level Changes

Change Details Files
Ensure settings dialog content and navigation list views are clipped to their bounds to avoid scroll bleed-through into the title bar.
  • Hook into the dialog onOpened lifecycle to apply additional view configuration after initial setup signals are connected.
  • Set control.contentView.clip = true so scrolled items cannot render outside the content view area.
  • Set control.navigationView.clip = true so navigation items are similarly constrained within the navigation view.
src/music-player/dialogs/SettingsDialog.qml

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这段代码的主要目的是修复一个UI渲染问题:通过给 contentViewnavigationView 设置 clip: true,防止滚动时内容透出标题栏。

以下是针对这段代码的详细审查意见:

1. 语法逻辑 (✅ 正确)

代码的语法完全符合QML规范,放置在 Component.onCompleted 中也是合理的,因为此时视图组件已经实例化完毕,可以安全地访问和修改其属性。逻辑上,设置 clip: true 确实能截断超出边界的内容,解决内容透出标题栏的问题。

2. 代码质量 (⚠️ 需要改进)

问题:直接访问内部私有属性,耦合度高,易碎性大
代码中使用了 control.contentViewcontrol.navigationView。从命名习惯来看,这通常是 Qt Quick Controls 2 中 Dialog 或自定义组件的内部私有属性(通常带有 __ 前缀或未在公开 API 文档中暴露)。

  • 风险:如果底层组件库在版本升级时重命名或移除了这些内部属性,这段代码将直接报错导致应用崩溃(Cannot read property 'clip' of undefined)。
  • 改进建议
    1. 优先使用公开 API:检查 control 是否有公开的 contentItemfooter 等属性可以替代。
    2. 防御性编程:如果确实只能通过内部属性解决,必须加上非空判断,防止升级时崩溃。
    3. 声明式替代命令式:在 QML 中,尽量使用声明式绑定而非命令式赋值。如果能在组件定义时使用 Binding 绑定会更符合 QML 的范式。

3. 代码性能 (⚠️ 需要关注)

问题:过度使用 clip: true 导致性能损耗
在 QML 渲染引擎(Scene Graph)中,设置 clip: true 会强制底层 OpenGL/Vulkan 为该 Item 开启模板缓冲或裁剪图层。

  • 风险:如果 contentViewnavigationView 包含复杂的列表、大量的动画或高分辨率图片,开启裁剪会显著增加 GPU 的渲染开销,可能导致滚动时掉帧(卡顿)。
  • 改进建议
    1. 验证必要性:确认是否两个 View 都必须开启 clip?有时候只需要给最外层的容器设置 clip 即可。
    2. 优化 UI 层级:更好的做法是不要依赖透明背景叠加,而是给标题栏设置一个不透明的背景色,让标题栏在 Z 轴上自然遮挡住下方内容,这样完全不需要开启 clip,性能最优。

4. 代码安全 (✅ 无明显安全隐患)

此部分代码仅涉及前端 UI 渲染逻辑,不涉及数据存储、网络请求或敏感信息处理,无安全风险。但如上文提到的,访问私有属性可能带来运行时崩溃的“安全性”问题。


💡 最终修改建议

结合以上分析,我建议你采用防御性编程来改进这段代码,同时评估是否可以通过调整 Z 轴和背景色来彻底避免使用 clip。如果必须使用 clip,改进后的代码如下:

     Presenter.valueChangedFromSettings.connect(settingsValueChanged)
     // 连接重置完成信号,确保 UI 同步更新
     Presenter.resetedFromSettings.connect(refreshAllSettings)

-    // 未设 clip:true,叠加 DialogWindow 的透明背景,滚动时 item 会透出标题栏。
-    control.contentView.clip = true
-    control.navigationView.clip = true
+    // 未设 clip:true,叠加 DialogWindow 的透明背景,滚动时 item 会透出标题栏。
+    // 注意:访问内部私有属性需进行非空判断,防止组件库升级导致崩溃
+    if (control.contentView) {
+        control.contentView.clip = true;
+    } else {
+        console.warn("SettingsDialog: control.contentView is undefined, clip setting skipped.");
+    }
+    
+    if (control.navigationView) {
+        control.navigationView.clip = true;
+    } else {
+        console.warn("SettingsDialog: control.navigationView is undefined, clip setting skipped.");
+    }
 }

更优的架构级修复方案(如果条件允许):
去修改 DialogWindow 的 QML 定义,给标题栏增加不透明背景,并确保标题栏的 z 值高于内容区域,然后移除这里的 clip 设置。这是解决 UI 叠层问题的最正统且性能最好的方式。

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

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

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 28, 2026

TAG Bot

New tag: 7.0.58
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #677

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.

2 participants