Skip to content

fix: add close button hint to timezone popup#2911

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:master
Dec 26, 2025
Merged

fix: add close button hint to timezone popup#2911
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743

@18202781743 18202781743 commented Dec 26, 2025

Copy link
Copy Markdown
Contributor

The SearchableListViewPopup for timezone selection was missing a
close button. The window flags were set to Qt.Dialog, which on some
window managers does not automatically provide a close button. Added
Qt.WindowCloseButtonHint to the flags to explicitly request a window
close button. This ensures the popup has a standard close button for
better user experience and consistency with other dialogs.

Log: Added close button to timezone selection popup window

Influence:

  1. Open the datetime settings and click on the timezone selector
  2. Verify the popup window has a close button in the title bar
  3. Test that the close button functions correctly to dismiss the popup
  4. Ensure the popup still appears centered and maintains its blur effect
  5. Verify the popup behavior is consistent across different window
    managers

fix: 为时区选择弹窗添加关闭按钮提示

时区选择使用的 SearchableListViewPopup 弹窗缺少关闭按钮。窗口标志之前仅
设置为 Qt.Dialog,在某些窗口管理器下不会自动提供关闭按钮。现在在 flags
中添加了 Qt.WindowCloseButtonHint,以明确请求窗口关闭按钮。这确保了弹窗
具有标准的关闭按钮,提升了用户体验并与其他对话框保持一致。

Log: 为时区选择弹窗窗口添加了关闭按钮

Influence:

  1. 打开日期时间设置,点击时区选择器
  2. 验证弹窗窗口的标题栏是否有关闭按钮
  3. 测试关闭按钮功能是否正常,可以关闭弹窗
  4. 确保弹窗仍然居中显示并保持模糊效果
  5. 验证弹窗在不同窗口管理器下的行为是否一致

PMS: BUG-345583

Summary by Sourcery

Bug Fixes:

  • Ensure the timezone selection popup always displays a standard close button in its title bar by updating its window flags.

The SearchableListViewPopup for timezone selection was missing a
close button. The window flags were set to Qt.Dialog, which on some
window managers does not automatically provide a close button. Added
Qt.WindowCloseButtonHint to the flags to explicitly request a window
close button. This ensures the popup has a standard close button for
better user experience and consistency with other dialogs.

Log: Added close button to timezone selection popup window

Influence:
1. Open the datetime settings and click on the timezone selector
2. Verify the popup window has a close button in the title bar
3. Test that the close button functions correctly to dismiss the popup
4. Ensure the popup still appears centered and maintains its blur effect
5. Verify the popup behavior is consistent across different window
managers

fix: 为时区选择弹窗添加关闭按钮提示

时区选择使用的 SearchableListViewPopup 弹窗缺少关闭按钮。窗口标志之前仅
设置为 Qt.Dialog,在某些窗口管理器下不会自动提供关闭按钮。现在在 flags
中添加了 Qt.WindowCloseButtonHint,以明确请求窗口关闭按钮。这确保了弹窗
具有标准的关闭按钮,提升了用户体验并与其他对话框保持一致。

Log: 为时区选择弹窗窗口添加了关闭按钮

Influence:
1. 打开日期时间设置,点击时区选择器
2. 验证弹窗窗口的标题栏是否有关闭按钮
3. 测试关闭按钮功能是否正常,可以关闭弹窗
4. 确保弹窗仍然居中显示并保持模糊效果
5. 验证弹窗在不同窗口管理器下的行为是否一致

PMS: BUG-345583
@sourcery-ai

sourcery-ai Bot commented Dec 26, 2025

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

Reviewer's Guide

Adds an explicit window close button to the timezone selection SearchableListViewPopup by adjusting its window flags, ensuring a standard close button appears across window managers while preserving existing appearance and behavior.

File-Level Changes

Change Details Files
Explicitly request a close button on the timezone selection popup window while keeping dialog behavior and visual style unchanged.
  • Update the popup window flags to combine Qt.Dialog with Qt.WindowCloseButtonHint so that a standard close button is shown by the window manager.
  • Document in code that adding Qt.WindowCloseButtonHint also sets the _NET_WM_WINDOW_TYPE to Dialog, preserving dialog semantics.
  • Retain existing blur, centering behavior, and palette configuration for the popup window without further modifications.
src/plugin-datetime/qml/SearchableListViewPopup.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

我来分析一下这个git diff的修改:

  1. 修改内容分析:
  • 在原有flags属性中增加了Qt.WindowCloseButtonHint标志
  • 添加了注释说明_NET_WM_WINDOW_TYPE会在添加Qt.WindowCloseButtonHint时自动添加Dialog类型
  1. 代码审查意见:

语法逻辑:

  • 语法正确,使用位或运算符(|)组合窗口标志是标准做法
  • 注释清晰说明了修改的原因

代码质量:

  • 建议将注释移到flags属性上方,这样更符合代码阅读习惯
  • 注释中的_NET_WM_WINDOW_TYPE是X11窗口系统的概念,建议说明这是针对X11系统的特性

代码性能:

  • 这个修改不会影响性能,只是窗口属性的设置

代码安全:

  • 修改安全,只是调整窗口标志

改进建议:

// For X11 system, _NET_WM_WINDOW_TYPE will add Dialog when added Qt.WindowCloseButtonHint
flags: Qt.Dialog | Qt.WindowCloseButtonHint

或者更详细的注释:

// Note: On X11 systems, the _NET_WM_WINDOW_TYPE property will automatically be set to Dialog
// when Qt.WindowCloseButtonHint is added. This ensures proper window type classification.
flags: Qt.Dialog | Qt.WindowCloseButtonHint

其他建议:

  1. 如果这个组件需要支持非X11系统(如Wayland),建议添加条件判断
  2. 可以考虑将窗口标志定义为一个单独的属性,便于维护

总体来说,这个修改是合理的,主要是为了确保窗口在X11系统下有正确的窗口类型和行为。

@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 left some high level feedback:

  • The new comment about _NET_WM_WINDOW_TYPE is a bit unclear; consider rephrasing or expanding it (or linking to the relevant Qt/WM behavior) so future readers understand why Qt.WindowCloseButtonHint is needed alongside Qt.Dialog.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new comment about `_NET_WM_WINDOW_TYPE` is a bit unclear; consider rephrasing or expanding it (or linking to the relevant Qt/WM behavior) so future readers understand why `Qt.WindowCloseButtonHint` is needed alongside `Qt.Dialog`.

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

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@18202781743

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Dec 26, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit c16d68d into linuxdeepin:master Dec 26, 2025
16 of 18 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