将世界管理界面“生成启动脚本”按钮折叠到二级菜单 #5198
将世界管理界面“生成启动脚本”按钮折叠到二级菜单 #5198
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request aims to reorganize the World Management interface by moving the "Generate Launch Script" button from the toolbar to a secondary popup menu to reduce UI clutter.
Changes:
- Removed "Generate Launch Script" button from the toolbar
- Added both "Launch" and "Generate Launch Script" options to the existing "Manage" popup menu
- Added a menu separator to organize the popup menu items
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| managePopupMenu.getContent().addAll( | ||
| new IconedMenuItem(SVG.ROCKET_LAUNCH, i18n("version.launch"), this::launch, managePopup), |
There was a problem hiding this comment.
The "Launch" button is duplicated. Line 114 adds a "Launch" button to the toolbar for Minecraft 1.20+ worlds, and this line adds the same "Launch" option to the popup menu. Based on the PR title's intent to collapse buttons into a secondary menu, the toolbar "Launch" button at line 114 should likely be removed as well, leaving only the popup menu items.
| new IconedMenuItem(SVG.ROCKET_LAUNCH, i18n("version.launch"), this::launch, managePopup), |
| new IconedMenuItem(SVG.ROCKET_LAUNCH, i18n("version.launch"), this::launch, managePopup), | ||
| new IconedMenuItem(SVG.SCRIPT, i18n("version.launch_script"), this::generateLaunchScript, managePopup), |
There was a problem hiding this comment.
The version condition check for Minecraft 1.20+ is missing for these menu items. The original toolbar items (lines 113-115) were only added for worlds with game version 1.20 or later (23w14a snapshot or later). These menu items should be conditionally added based on the same version check, or the entire popup menu block should be wrapped in the version condition to maintain consistency with the original behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PopupMenu managePopupMenu = new PopupMenu(); | ||
| JFXPopup managePopup = new JFXPopup(managePopupMenu); | ||
|
|
||
| if (world.getGameVersion() != null && world.getGameVersion().isAtLeast("1.20", "23w14a")) { |
There was a problem hiding this comment.
The version check condition is duplicated on lines 113 and 146. Consider extracting this to a local variable or method to avoid code duplication and improve maintainability. For example, you could add a boolean variable before the toolbar creation to store this condition result.
| new IconedMenuItem(SVG.SCRIPT, i18n("version.launch_script"), this::generateLaunchScript, managePopup), | ||
| new MenuSeparator() | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
The MenuSeparator is added unconditionally when the version check passes. If only the "Generate Launch Script" button should be moved to the popup menu (not the "Launch" button), then this separator might create an unnecessary visual gap when there's only one item before the export/delete/duplicate items. Consider whether the separator is still needed if the "Launch" button is removed from the popup menu.
| new IconedMenuItem(SVG.SCRIPT, i18n("version.launch_script"), this::generateLaunchScript, managePopup), | |
| new MenuSeparator() | |
| ); | |
| } | |
| new IconedMenuItem(SVG.SCRIPT, i18n("version.launch_script"), this::generateLaunchScript, managePopup) | |
| ); | |
| } | |
| if (managePopupMenu.getContent().size() > 1) { | |
| managePopupMenu.getContent().add(new MenuSeparator()); | |
| } |
No description provided.