Conversation
|
要不要添加一个 tooltip 来提示一下？ |
There was a problem hiding this comment.
Pull Request Overview
This PR implements scroll wheel support for switching game instances on the "实例管理" (Instance Management) button to maintain UI consistency with the existing account management button that already supports this feature.
- Extracts scroll wheel functionality into a reusable
FXUtils.onScroll()utility method - Refactors existing account switching code to use the new utility method
- Adds scroll wheel support to the game instance list item in the sidebar
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| FXUtils.java | Adds generic onScroll() utility method for scroll wheel navigation through lists |
| AccountAdvancedListItem.java | Refactors to use new utility method, removes duplicate scroll handling code |
| MainPage.java | Refactors launch pane scroll handling to use utility method, adds getter methods |
| RootPage.java | Adds scroll wheel support to game list item using new utility method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public static <T> void onScroll(Node node, List<T> list, | ||
| ToIntFunction<List<T>> finder, | ||
| Consumer<T> updater | ||
| ) { |
There was a problem hiding this comment.
The onScroll method lacks documentation. Add a JavaDoc comment explaining the purpose, parameters, and behavior of this utility method, especially the finder function's expected return value (-1 for not found) and how the circular navigation works.
| } | ||
|
|
||
| public Profile getProfile() { | ||
| return profile; |
There was a problem hiding this comment.
[nitpick] The newly added getProfile() method exposes the internal profile object without any access control or immutability guarantees. Consider whether this direct exposure is necessary or if a more restricted API would be safer.
| return profile; | |
| // Return a defensive copy to prevent external modification | |
| return profile == null ? null : new Profile(profile); |
| } | ||
|
|
||
| public ObservableList<Version> getVersions() { | ||
| return versions; |
There was a problem hiding this comment.
The getVersions() method returns the internal ObservableList directly, allowing external code to modify it. Consider returning an unmodifiable view using FXCollections.unmodifiableObservableList(versions) to prevent unintended modifications.
| return versions; | |
| return FXCollections.unmodifiableObservableList(versions); |
考虑到账户管理按钮已经支持通过这种方式快速切换账户，让实例管理按钮支持同样的功能能够增强界面的一致性。