fix: Resolved user confirmation dialog hierarchy anomaly#586
fix: Resolved user confirmation dialog hierarchy anomaly#586deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
log: Fixed integration and rollback handling pms: bug-352609
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSimplifies the dialog window behavior by removing custom transient parent handling and visibility-based activation logic, relying instead on default window hierarchy and activation behavior. Sequence diagram for simplified dialog window activation behaviorsequenceDiagram
actor User
participant Application
participant DialogWindow
participant QtWindowManager
User->>Application: triggerAction()
Application->>DialogWindow: create and show()
DialogWindow->>QtWindowManager: requestShowWindow()
QtWindowManager-->>DialogWindow: applyDefaultTransientParent()
QtWindowManager-->>DialogWindow: manageZOrderAndActivation()
DialogWindow-->>User: dialogVisibleAndInteractive
Updated class diagram for DialogWindow transient parent removalclassDiagram
class DialogWindow {
+real topPadding
+real bottomPadding
+real leftPadding
+real rightPadding
+onClosing(close)
}
class DialogWindowLegacy {
+var transientParentWindow
+updateTransientParent()
+Component_onCompleted()
+onVisibleChanged()
}
DialogWindowLegacy <|-- DialogWindow
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- By removing the
onVisibleChangedhandler (includingraise()andrequestActivate()), dialogs may now open without keyboard focus or appear behind other windows; if those behaviors are still required in some scenarios, consider moving the focus/stacking logic to the caller or reintroducing it in a more targeted way. - The removal of the
transientParentWindowhandling means modal dialogs will no longer automatically attach to the currentQt.application.activeWindow; verify that all dialog creation sites now explicitly settransientParentwhere needed, or consider providing a clearer, opt‑in helper for this behavior instead of relying on implicit application state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By removing the `onVisibleChanged` handler (including `raise()` and `requestActivate()`), dialogs may now open without keyboard focus or appear behind other windows; if those behaviors are still required in some scenarios, consider moving the focus/stacking logic to the caller or reintroducing it in a more targeted way.
- The removal of the `transientParentWindow` handling means modal dialogs will no longer automatically attach to the current `Qt.application.activeWindow`; verify that all dialog creation sites now explicitly set `transientParent` where needed, or consider providing a clearer, opt‑in helper for this behavior instead of relying on implicit application state.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码是一个对 Qt/QML 文件 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合改进意见
总结:这次修改在代码简洁性和性能上有微小提升,但在功能逻辑上存在重大变更风险。除非有充分的理由(如改用新的窗口管理架构),否则不应随意移除模态对话框的父窗口设置逻辑。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
log: Fixed integration and rollback handling
pms: bug-352609
Summary by Sourcery
Bug Fixes: