优化独立窗口弹窗 #5060
优化独立窗口弹窗 #5060
Conversation
|
maybe resolves #4942? |
|
请解决代码和主线的冲突。 |
There was a problem hiding this comment.
Pull request overview
This PR optimizes standalone window dialogs by replacing JavaFX Alert dialogs with custom JFX dialogs, improving consistency and adding stack traces to error messages in the game runtime export functionality.
- Centralized dialog management through a new
DialogUtilsutility class - Replaced JavaFX Alert with MessageDialogPane in LogWindow and GameCrashWindow
- Enhanced error messages by including stack traces for export failures
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
DialogUtils.java |
New utility class that centralizes dialog creation, display, and lifecycle management for both Decorator-based and StackPane-based dialogs |
DecoratorController.java |
Refactored to delegate dialog operations to DialogUtils, simplifying dialog handling code and removing duplicate logic |
LogWindow.java |
Replaced JavaFX Alert dialogs with MessageDialogPane; added StackPane container to support JFX dialogs; enhanced error messages with stack traces |
GameCrashWindow.java |
Replaced JavaFX Alert dialogs with MessageDialogPane; added StackPane container to support JFX dialogs; enhanced error messages with stack traces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| getSkinnable().stackPane.getChildren().setAll(vbox); | ||
| getChildren().setAll(getSkinnable().stackPane); | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line here. Only one blank line should be used for consistency with the rest of the codebase.
| StackPane container = dialog.getDialogContainer(); | ||
| if (container != null) { | ||
| container.getProperties().remove(PROPERTY_DIALOG_INSTANCE); | ||
| container.getProperties().remove(PROPERTY_DIALOG_PANE_INSTANCE); |
There was a problem hiding this comment.
这里没有清理 PROPERTY_PARENT_PANE_REF 和 PROPERTY_PARENT_DIALOG_REF。
HMCL/src/main/java/org/jackhuang/hmcl/ui/decorator/DecoratorController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| container.getProperties().remove(PROPERTY_PARENT_DIALOG_REF); | ||
| container.getProperties().remove(PROPERTY_PARENT_PANE_REF); | ||
| } |
There was a problem hiding this comment.
When closing the last dialog, the properties PROPERTY_PARENT_DIALOG_REF and PROPERTY_PARENT_PANE_REF should be removed from the content node's properties, not from the container's properties. These properties were set on the content node at lines 75-76. This could lead to memory leaks as references to the dialog and pane would remain in the content node's properties even after the dialog is closed.
| container.getProperties().remove(PROPERTY_PARENT_DIALOG_REF); | |
| container.getProperties().remove(PROPERTY_PARENT_PANE_REF); | |
| } | |
| } | |
| content.getProperties().remove(PROPERTY_PARENT_DIALOG_REF); | |
| content.getProperties().remove(PROPERTY_PARENT_PANE_REF); |
| public static void show(Decorator decorator, Node content) { | ||
| if (decorator.getDrawerWrapper() == null) { | ||
| Platform.runLater(() -> show(decorator, content)); | ||
| return; | ||
| } | ||
|
|
||
| show(decorator.getDrawerWrapper(), content, (dialog) -> { | ||
| JFXDialogPane pane = (JFXDialogPane) dialog.getContent(); | ||
| decorator.capableDraggingWindow(dialog); | ||
| decorator.forbidDraggingWindow(pane); | ||
| dialog.setDialogContainer(decorator.getDrawerWrapper()); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The original dialog implementation disabled the navigator when a dialog was shown and re-enabled it when all dialogs were closed. This behavior is missing in the new DialogUtils implementation. When showing dialogs in the Decorator context, the navigator should be disabled to prevent navigation while a dialog is open, and re-enabled when the last dialog is closed.
| @@ -0,0 +1,138 @@ | |||
| package org.jackhuang.hmcl.ui; | |||
There was a problem hiding this comment.
This file is missing the standard license header that all other files in the project have. The header should include the GPL-3.0 license notice, copyright information, and project name "Hello Minecraft! Launcher".
| } else { | ||
| pane.pop(content); | ||
| } |
There was a problem hiding this comment.
When popping a dialog that is not the last one, the content node's properties (PROPERTY_PARENT_PANE_REF and PROPERTY_PARENT_DIALOG_REF) should be cleaned up to prevent potential memory leaks. These properties were set on the content at lines 75-76.
3 participants
resolves #4942