重构 SchematicsPage 以使用 ListCell 展示元素#5276
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the SchematicsPage to use a custom ListCell for displaying items instead of having each Item extend Control with its own Skin. This is a cleaner architectural approach that follows JavaFX best practices.
Changes:
- Converted Item from a Control class to a sealed class implementing Comparable
- Removed the ItemSkin class and replaced it with a ListCell-based Cell class
- Added getImage() method to LitematicFileItem to support preview images
- Updated SchematicsPageSkin to provide the custom Cell via createListCell override
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
| } | ||
|
|
||
| abstract class Item extends Control implements Comparable<Item> { | ||
| abstract sealed class Item implements Comparable<Item> { |
There was a problem hiding this comment.
The sealed class Item is missing a permits clause. Sealed classes in Java must explicitly list the classes that are allowed to extend them using the permits keyword. Add permits BackItem, DirItem, LitematicFileItem to specify which inner classes can extend this sealed class.
| abstract sealed class Item implements Comparable<Item> { | |
| abstract sealed class Item implements Comparable<Item> permits BackItem, DirItem, LitematicFileItem { |
| if (empty || item == null) { | ||
| setGraphic(null); | ||
| center.setTitle(""); | ||
| center.setSubtitle(""); | ||
| } else { | ||
| if (item instanceof LitematicFileItem fileItem && fileItem.getImage() != null) { | ||
| iconImageView.setImage(fileItem.getImage()); | ||
| left.getChildren().setAll(iconImageView); | ||
| } else { | ||
| iconSVG.setContent(item.getIcon().getPath()); | ||
| left.getChildren().setAll(iconSVGWrapper); | ||
| } | ||
|
|
||
| center.setTitle(item.getName()); | ||
| center.setSubtitle(item.getDescription()); | ||
|
|
||
| Path path = item.getPath(); | ||
| if (path != null) { | ||
| tooltip.setText(FileUtils.getAbsolutePath(path)); | ||
| FXUtils.installSlowTooltip(left, tooltip); | ||
| } else { | ||
| tooltip.setText(""); | ||
| Tooltip.uninstall(left, tooltip); | ||
| } |
There was a problem hiding this comment.
The tooltip is being installed and uninstalled repeatedly in updateItem, which can cause memory leaks and performance issues. The tooltip should be installed once in the constructor (similar to the WorldListCell pattern at line 254 in WorldListPage.java), and only the text should be updated in updateItem. When the item is empty or has no path, set the tooltip text to an empty string instead of uninstalling it. Additionally, ensure that tooltip.setText is called in the empty branch at line 635 to clear the tooltip text when the cell is reused.
No description provided.