Skip to content

[Enhancement] 账户列表优化 #6093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ToobLac wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Enhancement] 账户列表优化 #6093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdf1bff
account order
CalbootOnceMore May 1, 2026
586c16c
Merge remote-tracking branch 'upstream/main' into account-order
ToobLac May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion HMCL/src/main/java/org/jackhuang/hmcl/setting/Accounts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ private Accounts() {
public static final MicrosoftAccountFactory FACTORY_MICROSOFT = new MicrosoftAccountFactory(new MicrosoftService(OAUTH_CALLBACK));
public static final List<AccountFactory<?>> FACTORIES = immutableListOf(FACTORY_OFFLINE, FACTORY_MICROSOFT, FACTORY_AUTHLIB_INJECTOR);

// FX-Thread
public static boolean skipSelectionCheckFlag = false;

// ==== login type / account factory mapping ====
private static final Map<String, AccountFactory<?>> type2factory = new HashMap<>();
private static final Map<AccountFactory<?>, String> factory2type = new HashMap<>();
Expand Down Expand Up @@ -262,7 +265,7 @@ static void init() {
}

if (!globalConfig().isEnableOfflineAccount())
accounts.addListener(new ListChangeListener<Account>() {
accounts.addListener(new ListChangeListener<>() {
@Override
public void onChanged(Change<? extends Account> change) {
while (change.next()) {
Expand All @@ -280,6 +283,7 @@ public void onChanged(Change<? extends Account> change) {
selectedAccount.set(selected);

InvalidationListener listener = o -> {
if (skipSelectionCheckFlag) return;
// this method first checks whether the current selection is valid
// if it's valid, the underlying storage will be updated
// otherwise, the first account will be selected as an alternative(or null if accounts is empty)
Expand Down
12 changes: 8 additions & 4 deletions HMCL/src/main/java/org/jackhuang/hmcl/ui/FXUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,12 @@
import javafx.event.EventType;
import javafx.geometry.Bounds;
import javafx.geometry.Rectangle2D;
import javafx.scene.Cursor;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.*;
import javafx.scene.control.*;
import javafx.scene.control.skin.VirtualFlow;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.image.WritableImage;
import javafx.scene.input.*;
import javafx.scene.layout.ColumnConstraints;
import javafx.scene.layout.Priority;
Expand Down Expand Up @@ -1678,4 +1676,10 @@ public static void useJFXContextMenu(TextInputControl control) {
e.consume();
});
}

public static WritableImage takeSnapshot(Region node) {
SnapshotParameters snapShotParams = new SnapshotParameters();
snapShotParams.setFill(Color.TRANSPARENT);
return node.snapshot(snapShotParams, new WritableImage((int) node.getWidth() + 10, (int) node.getHeight() + 10));
}
}
9 changes: 8 additions & 1 deletion HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountListItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@
public class AccountListItem extends RadioButton {

private final Account account;
private final AccountListPage page;

private final StringProperty title = new SimpleStringProperty();
private final StringProperty subtitle = new SimpleStringProperty();

public AccountListItem(Account account) {
public AccountListItem(Account account, AccountListPage page) {
this.account = account;
this.page = page;
getStyleClass().clear();
setUserData(account);

Expand Down Expand Up @@ -188,6 +191,10 @@ public Account getAccount() {
return account;
}

public AccountListPage getPage() {
return page;
}

public String getTitle() {
return title.get();
}
Expand Down
36 changes: 18 additions & 18 deletions HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountListItemSkin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
import javafx.scene.control.Label;
import javafx.scene.control.SkinBase;
import javafx.scene.control.Tooltip;
import javafx.scene.input.ClipboardContent;
import javafx.scene.input.Dragboard;
import javafx.scene.input.TransferMode;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
Expand Down Expand Up @@ -95,24 +98,10 @@ public AccountListItemSkin(AccountListItem skinnable) {
spinnerMove.getStyleClass().add("small-spinner-pane");
btnMove.setOnAction(e -> {
Account account = skinnable.getAccount();
Accounts.getAccounts().remove(account);
if (account.isPortable()) {
account.setPortable(false);
if (!Accounts.getAccounts().contains(account))
Accounts.getAccounts().add(account);
} else {
account.setPortable(true);
if (!Accounts.getAccounts().contains(account)) {
int idx = 0;
for (int i = Accounts.getAccounts().size() - 1; i >= 0; i--) {
if (Accounts.getAccounts().get(i).isPortable()) {
idx = i + 1;
break;
}
}
Accounts.getAccounts().add(idx, account);
}
}
int index = Accounts.getAccounts().indexOf(account);
Accounts.getAccounts().removeAll(account);
account.setPortable(!account.isPortable());
Accounts.getAccounts().add(index, account);
Comment on lines +101 to +104
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using removeAll(account) is inefficient because it scans the entire list. Since we already have the index, we can use remove(index) which is faster. Additionally, if index is -1 (e.g., if the account was removed concurrently), calling add(-1, account) will throw an IndexOutOfBoundsException. We should guard against this by checking if index >= 0.

            int index = Accounts.getAccounts().indexOf(account);
            if (index >= 0) {
                Accounts.getAccounts().remove(index);
                account.setPortable(!account.isPortable());
                Accounts.getAccounts().add(index, account);
            }

});
btnMove.getStyleClass().add("toggle-icon4");
if (skinnable.getAccount().isPortable()) {
Expand Down Expand Up @@ -184,6 +173,17 @@ public AccountListItemSkin(AccountListItem skinnable) {
root.setStyle("-fx-padding: 8 8 8 0;");
JFXDepthManager.setDepth(root, 1);

// Enable drag detection for reordering
root.setOnDragDetected(event -> {
if (skinnable.getPage().isSearching().get()) return;
Dragboard db = root.startDragAndDrop(TransferMode.MOVE);
ClipboardContent content = new ClipboardContent();
content.putString(skinnable.getAccount().getIdentifier());
db.setContent(content);
db.setDragView(FXUtils.takeSnapshot(root));
event.consume();
});

getChildren().setAll(root);
}
}
141 changes: 130 additions & 11 deletions HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountListPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
*/
package org.jackhuang.hmcl.ui.account;

import com.jfoenix.controls.JFXButton;
import com.jfoenix.controls.JFXTextField;
import com.jfoenix.effects.JFXDepthManager;
import javafx.animation.PauseTransition;
import javafx.beans.InvalidationListener;
import javafx.beans.binding.Bindings;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ListProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.beans.property.ReadOnlyObjectWrapper;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.beans.binding.BooleanBinding;
import javafx.beans.property.*;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.collections.FXCollections;
Expand All @@ -33,10 +33,17 @@
import javafx.scene.control.ScrollPane;
import javafx.scene.control.Skin;
import javafx.scene.control.Tooltip;
import javafx.scene.input.Dragboard;
import javafx.scene.input.TransferMode;
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.util.Duration;
import org.jackhuang.hmcl.auth.Account;
import org.jackhuang.hmcl.auth.authlibinjector.AuthlibInjectorAccount;
import org.jackhuang.hmcl.auth.authlibinjector.AuthlibInjectorServer;
import org.jackhuang.hmcl.auth.microsoft.MicrosoftAccount;
import org.jackhuang.hmcl.auth.offline.OfflineAccount;
import org.jackhuang.hmcl.setting.Accounts;
import org.jackhuang.hmcl.ui.Controllers;
import org.jackhuang.hmcl.ui.FXUtils;
Expand All @@ -45,6 +52,7 @@
import org.jackhuang.hmcl.ui.construct.ClassTitle;
import org.jackhuang.hmcl.ui.decorator.DecoratorAnimatedPage;
import org.jackhuang.hmcl.ui.decorator.DecoratorPage;
import org.jackhuang.hmcl.util.StringUtils;
import org.jackhuang.hmcl.util.i18n.LocaleUtils;
import org.jackhuang.hmcl.util.io.NetworkUtils;
import org.jackhuang.hmcl.util.javafx.BindingMapping;
Expand All @@ -53,6 +61,8 @@
import java.util.Locale;

import static org.jackhuang.hmcl.setting.ConfigHolder.globalConfig;
import static org.jackhuang.hmcl.ui.FXUtils.onEscPressed;
import static org.jackhuang.hmcl.ui.ToolbarListPageSkin.createToolbarButton2;
import static org.jackhuang.hmcl.util.i18n.I18n.i18n;
import static org.jackhuang.hmcl.util.javafx.ExtendedProperties.createSelectedItemPropertyFor;
import static org.jackhuang.hmcl.util.logging.Logger.LOG;
Expand All @@ -68,7 +78,7 @@ public final class AccountListPage extends DecoratorAnimatedPage implements Deco
|| globalConfig().isEnableOfflineAccount())
RESTRICTED.set(false);
else
globalConfig().enableOfflineAccountProperty().addListener(new ChangeListener<Boolean>() {
globalConfig().enableOfflineAccountProperty().addListener(new ChangeListener<>() {
@Override
public void changed(ObservableValue<? extends Boolean> o, Boolean oldValue, Boolean newValue) {
if (newValue) {
Expand All @@ -80,14 +90,42 @@ public void changed(ObservableValue<? extends Boolean> o, Boolean oldValue, Bool
}

private final ObservableList<AccountListItem> items;
private final ObservableList<AccountListItem> displayedItems;
private final ReadOnlyObjectWrapper<State> state = new ReadOnlyObjectWrapper<>(State.fromTitle(i18n("account.manage")));
private final ListProperty<Account> accounts = new SimpleListProperty<>(this, "accounts", FXCollections.observableArrayList());
private final ListProperty<AuthlibInjectorServer> authServers = new SimpleListProperty<>(this, "authServers", FXCollections.observableArrayList());
private final ObjectProperty<Account> selectedAccount;

private final StringProperty searchingText = new SimpleStringProperty(this, "searchingText", "");
private final BooleanBinding isSearching = Bindings.createBooleanBinding(() -> StringUtils.isNotBlank(searchingText.get()), searchingText);

public AccountListPage() {
items = MappedObservableList.create(accounts, AccountListItem::new);
items = MappedObservableList.create(accounts, (account) -> new AccountListItem(account, this));
displayedItems = FXCollections.observableArrayList(items);
selectedAccount = createSelectedItemPropertyFor(items, Account.class);

InvalidationListener listener = (observable) -> {
String text = searchingText.get().toLowerCase(Locale.ROOT);
if (StringUtils.isBlank(text)) {
displayedItems.setAll(items);
return;
}
displayedItems.setAll(
items.stream().filter(item -> {
Account account = item.getAccount();
String type = "";
if (account instanceof MicrosoftAccount) type = "microsoft";
else if (account instanceof OfflineAccount) type = "offline";
else if (account instanceof AuthlibInjectorAccount) type = ((AuthlibInjectorAccount) account).getServer().getUrl().toLowerCase(Locale.ROOT);
return account.getCharacter().toLowerCase(Locale.ROOT).contains(text)
|| account.getUsername().toLowerCase(Locale.ROOT).contains(text)
|| account.getUUID().toString().contains(text)
|| type.contains(text);
}).toList()
);
};
Comment on lines +107 to +126
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If searchingText.get() is null, calling toLowerCase(Locale.ROOT) on it will throw a NullPointerException. It is safer to check for blank/null values first before performing string operations.

        InvalidationListener listener = (observable) -> {
            String query = searchingText.get();
            if (StringUtils.isBlank(query)) {
                displayedItems.setAll(items);
                return;
            }
            String text = query.toLowerCase(Locale.ROOT);
            displayedItems.setAll(
                    items.stream().filter(item -> {
                        Account account = item.getAccount();
                        String type = "";
                        if (account instanceof MicrosoftAccount) type = "microsoft";
                        else if (account instanceof OfflineAccount) type = "offline";
                        else if (account instanceof AuthlibInjectorAccount) type = ((AuthlibInjectorAccount) account).getServer().getUrl().toLowerCase(Locale.ROOT);
                        return account.getCharacter().toLowerCase(Locale.ROOT).contains(text)
                                || account.getUsername().toLowerCase(Locale.ROOT).contains(text)
                                || account.getUUID().toString().contains(text)
                                || type.contains(text);
                    }).toList()
            );
        };

items.addListener(listener);
searchingText.addListener(listener);
}

public ObjectProperty<Account> selectedAccountProperty() {
Expand All @@ -107,6 +145,10 @@ public ListProperty<AuthlibInjectorServer> authServersProperty() {
return authServers;
}

public BooleanBinding isSearching() {
return isSearching;
}

@Override
protected Skin<?> createDefaultSkin() {
return new AccountListPageSkin(this);
Expand Down Expand Up @@ -204,6 +246,27 @@ public AccountListPageSkin(AccountListPage skinnable) {
setLeft(scrollPane, addAuthServerItem);
}

HBox searchBar = new HBox();
{
JFXTextField searchField = new JFXTextField();
searchField.setPromptText(i18n("search"));
HBox.setHgrow(searchField, Priority.ALWAYS);
PauseTransition pause = new PauseTransition(Duration.millis(100));
pause.setOnFinished(e -> skinnable.searchingText.set(searchField.getText()));
searchField.textProperty().addListener((observable, oldValue, newValue) -> {
pause.setRate(1);
pause.playFromStart();
});
JFXButton btnClearSearch = createToolbarButton2(null, SVG.CLOSE, searchField::clear);
onEscPressed(searchField, btnClearSearch::fire);

searchBar.getChildren().setAll(searchField, btnClearSearch);
searchBar.getStyleClass().add("card");
searchBar.setSpacing(1);
VBox.setMargin(searchBar, new Insets(10, 10, 5, 10));
JFXDepthManager.setDepth(searchBar, 1);
}

ScrollPane scrollPane = new ScrollPane();
VBox list = new VBox();
{
Expand All @@ -213,13 +276,69 @@ public AccountListPageSkin(AccountListPage skinnable) {
list.setSpacing(10);
list.getStyleClass().add("card-list");

Bindings.bindContent(list.getChildren(), skinnable.items);
list.setOnDragOver((event) -> {
if (event.getGestureSource() != list && event.getDragboard().hasString()) {
event.acceptTransferModes(TransferMode.MOVE);
}
event.consume();
});

list.setOnDragDropped((event) -> {
Dragboard db = event.getDragboard();
boolean success = false;
if (db.hasString()) {
String accountId = db.getString();
int targetIndex = getTargetIndex(list, event.getY());

// Find the account in the original list
Account draggedAccount = null;
int sourceIndex = -1;
for (int i = 0; i < Accounts.getAccounts().size(); i++) {
if (Accounts.getAccounts().get(i).getIdentifier().equals(accountId)) {
draggedAccount = Accounts.getAccounts().get(i);
sourceIndex = i;
break;
}
}

boolean selected = skinnable.selectedAccountProperty().get() == draggedAccount;
if (draggedAccount != null && sourceIndex != targetIndex) {
// Remove from old position
Accounts.skipSelectionCheckFlag = true;
Accounts.getAccounts().remove(sourceIndex);
// Insert at new position
int newIndex = targetIndex > sourceIndex ? targetIndex - 1 : targetIndex;
if (newIndex < 0) newIndex = 0;
if (newIndex > Accounts.getAccounts().size()) newIndex = Accounts.getAccounts().size();
Accounts.getAccounts().add(newIndex, draggedAccount);
if (selected) skinnable.selectedAccountProperty().set(draggedAccount);
Accounts.skipSelectionCheckFlag = false;
success = true;
}
Comment on lines +304 to +317
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If an exception occurs during list modification, Accounts.skipSelectionCheckFlag will remain true permanently, which disables selection validation globally. Wrapping the list operations in a try-finally block ensures the flag is always reset. Additionally, if newIndex == sourceIndex, we can completely avoid removing and re-adding the item, preventing unnecessary list change events and potential UI flickers.

                        if (draggedAccount != null) {
                            int newIndex = targetIndex > sourceIndex ? targetIndex - 1 : targetIndex;
                            if (newIndex < 0) {
                                newIndex = 0;
                            }
                            int maxIndex = Accounts.getAccounts().size() - 1;
                            if (newIndex > maxIndex) {
                                newIndex = maxIndex;
                            }

                            if (sourceIndex != newIndex) {
                                boolean selected = skinnable.selectedAccountProperty().get() == draggedAccount;
                                Accounts.skipSelectionCheckFlag = true;
                                try {
                                    Accounts.getAccounts().remove(sourceIndex);
                                    Accounts.getAccounts().add(newIndex, draggedAccount);
                                    if (selected) {
                                        skinnable.selectedAccountProperty().set(draggedAccount);
                                    }
                                } finally {
                                    Accounts.skipSelectionCheckFlag = false;
                                }
                                success = true;
                            }
                        }

}
event.setDropCompleted(success);
event.consume();
});

Bindings.bindContent(list.getChildren(), skinnable.displayedItems);

scrollPane.setContent(list);
FXUtils.smoothScrolling(scrollPane);
}

setCenter(new VBox(searchBar, scrollPane));
}

setCenter(scrollPane);
private int getTargetIndex(VBox list, double y) {
int index = 0;
for (int i = 0; i < list.getChildren().size(); i++) {
javafx.scene.Node child = list.getChildren().get(i);
if (child.getLayoutY() + child.getBoundsInParent().getHeight() / 2 > y) {
return i;
}
index = i + 1;
}
return index;
}
}
}