Conversation
CiiLu
commented
Mar 28, 2026
CiiLu
commented
- 修复模组管理点击全选会工具栏会闪烁的问题
- 在全选时禁用全选按钮
There was a problem hiding this comment.
Pull request overview
该 PR 针对版本管理页（模组/数据包列表）工具栏的“全选”交互进行优化：避免“全选”触发选择状态短暂清空导致的工具栏闪烁，并在已全选状态下禁用“全选”按钮以减少无效操作。
Changes:
- 将“全选”从
selectAll()改为selectRange(0, items.size())，减少选择状态抖动引起的工具栏闪烁 - 监听选中项变化，在已全选时禁用“全选”按钮
-（ModListPageSkin）将org.jackhuang.hmcl.util.*通配导入改为显式导入
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java | 调整模组列表“全选”实现并新增“全选按钮在全选时禁用”的逻辑 |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DataPackListPageSkin.java | 调整数据包列表“全选”实现并新增“全选按钮在全选时禁用”的逻辑 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java
Outdated
Show resolved
Hide resolved
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java
Outdated
Show resolved
Hide resolved
| // reason for not using selectAll() is that selectAll() first clears all selected then selects all, causing the toolbar to flicker | ||
| var selectAllButton = createToolbarButton2(i18n("button.select_all"), SVG.SELECT_ALL, () -> listView.getSelectionModel().selectRange(0, listView.getItems().size())); | ||
|
|
||
| listView.getSelectionModel().getSelectedItems().addListener( | ||
| (ListChangeListener<Object>) change -> { | ||
| selectAllButton.setDisable(!listView.getItems().isEmpty() | ||
| && listView.getSelectionModel().getSelectedItems().size() == listView.getItems().size()); | ||
| } | ||
| ); |
There was a problem hiding this comment.
The select-all button disable state is updated only when selectedItems changes and isn’t initialized. If listView.getItems() size changes (e.g., filtering/search updates) without a selection change, the button can remain disabled/enabled incorrectly. Consider binding disableProperty to a condition that observes both items size and selectedItems size, and ensure it’s evaluated once initially.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DataPackListPageSkin.java
Outdated
Show resolved
Hide resolved
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d570b7a231
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| listView.getSelectionModel().getSelectedItems().addListener(listener); | ||
| listView.getItems().addListener(listener); |
There was a problem hiding this comment.
listView.getItems().addListener(listener) is registered before setItems(filteredList) is called, so this listener stays attached to the initial default list instead of the actual filteredList backing the view. As a result, the new select-all disable state is not recomputed when the displayed datapack list changes without a selection change (for example when items are refreshed/updated), and the button can remain incorrectly disabled after the list size changes. Register the items listener after setItems(...) (or observe itemsProperty) so it tracks the active list.
Useful? React with 👍 / 👎.
