Skip to content

Conversation

@tsic404
Copy link
Contributor

@tsic404 tsic404 commented Nov 29, 2024

Qt popup get event from other window, so it's necessary to make dock can get focus. And add a PopupWindow to handle Popup auto hide.

log: as title

yixinshark
yixinshark previously approved these changes Nov 29, 2024
@github-actions
Copy link

TAG Bot

TAG: 1.99.8
EXISTED: no
DISTRIBUTION: UNRELEASED

yixinshark
yixinshark previously approved these changes Nov 29, 2024
Qt popup get event from other window, so it's necessary
to make dock can get focus. And add a PopupWindow to
handle Popup auto hide.

log: as title
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. popupwindow.cpp文件中,mouseReleaseEvent函数中使用了geometry()globalPosition(),但没有检查event是否为nullptr,可能会导致空指针异常。
  2. popupwindow.h文件中,PopupWindow类继承了QQuickWindowQmlImpl,但没有在类声明中添加相应的注释说明其用途和功能。
  3. qmlplugin.cpp文件中,qmlRegisterType<PopupWindow>(uri, 1, 0, "PopupWindow");这行代码注册了PopupWindow类型,但没有在注释中说明为什么需要注册这个类型。
  4. PanelPopupWindow.qml文件中,PopupWindowflags属性被设置为(Qt.Tool | Qt.WindowStaysOnTopHint) | Qt.Popup,但是Qt.ToolQt.WindowStaysOnTopHint标志在PopupWindow中可能不适用,应该检查是否需要这些标志。
  5. main.qml文件中,DLayerShellWindow.keyboardInteractivity属性被设置为DLayerShellWindow.KeyboardInteractivityOnDemand,但没有在注释中说明为什么需要这个设置。

总体来说,代码的修改看起来是为了修复一个弹出窗口无法隐藏的问题,但是需要确保所有的修改都是必要的,并且没有引入新的问题。同时,应该添加更多的注释来解释代码的意图和功能,以提高代码的可读性和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tsic404, yixinshark

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

@yixinshark
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Nov 29, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 3126e21 into linuxdeepin:master Nov 29, 2024
8 of 11 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.

4 participants