Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: ajust window flags #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: ajust window flags #380

wants to merge 1 commit into from

Conversation

kegechen
Copy link
Contributor

fix window flags is always Qt::Window

deepin-ci-robot added a commit to linuxdeepin/dtk6declarative that referenced this pull request Aug 19, 2024
@deepin-ci-robot
sync: from linuxdeepin/dtkdeclarative
c2883a1 
Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#380
@deepin-ci-robot deepin-ci-robot mentioned this pull request Aug 19, 2024
Closed
@deepin-bot
Copy link
Contributor

deepin-bot bot commented Aug 19, 2024

Doc Check bot
🟢 Document Coverage Check Passed!

18202781743
18202781743 reviewed Aug 20, 2024
View reviewed changes
src/dquickwindow.cpp
@@ -319,6 +340,12 @@ DQuickWindowAttached::DQuickWindowAttached(QWindow *window)
: QObject(window)
, DObject(*new DQuickWindowAttachedPrivate(window, this))
{
auto windowFlags = window->flags();
if (window->isTopLevel()) {
fixTopLevelWindowFlags(windowFlags);
Copy link
Contributor

Choose a reason for hiding this comment

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

这个行为,应该让window的创建者自己设置吧?

Copy link
Contributor Author

@kegechen kegechen Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

widget 里面是 qwidgetwindow 主动做了这个事，默认窗口只有 Qt::Window 这个 flag
如果让创建者自己设置，那么所有的窗口都需要自行处理一次，包括我的example也没有最大化、最小化、关闭按钮

@kegechen kegechen requested a review from zccrs August 20, 2024 03:36
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: FeiWang1119, kegechen

The full list of commands accepted by this bot can be found here.

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

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Aug 21, 2024

TAG Bot

New tag: 5.6.34
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #382

@kegechen
chore: ajust window flags
0f0192a 
fix window flags is always Qt::Window
deepin-ci-robot added a commit to linuxdeepin/dtk6declarative that referenced this pull request Aug 22, 2024
@deepin-ci-robot
sync: from linuxdeepin/dtkdeclarative
5959054 
Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#380
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

关键摘要:

  • 在fixTopLevelWindowFlags函数中，对于Qt::WindowFullscreenButtonHint的移除操作可能不必要，因为该属性在Qt的文档中没有说明是否支持在顶层窗口上设置。
  • fixTopLevelWindowFlags函数中的switch语句可以简化，使用case和default直接关联所有可能的窗口类型，而不是使用Qt::Window和Qt::Dialog等。
  • fixTopLevelWindowFlags函数中的if语句可以简化，直接在switch语句中设置所有所需的标志，而不是检查Qt::WindowType_Mask。
  • fixTopLevelWindowFlags函数中的if语句检查(flags & Qt::WindowType_Mask) == Qt::SplashScreen，但没有考虑到Qt::SplashScreen窗口类型可能不会出现在flags中，这可能会导致未定义行为。
  • fixTopLevelWindowFlags函数中的if语句检查window->isTopLevel()，但没有考虑到window可能不是顶级窗口的情况，这可能会导致未定义行为。
  • fixTopLevelWindowFlags函数中的window->setFlags(windowFlags);可能会覆盖窗口的现有属性，应该先获取当前的窗口属性，然后与需要设置的属性进行合并。

是否建议立即修改:
是

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 23, 2024

TAG Bot

New tag: 5.0.35
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@18202781743 18202781743 18202781743 left review comments

@FeiWang1119 FeiWang1119 FeiWang1119 approved these changes

@zccrs zccrs Awaiting requested review from zccrs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kegechen @deepin-ci-robot @18202781743 @FeiWang1119