Remember last opened chat tab (#3205)

* remember last opened chat tab * replace from stream to foreach * remove unused annotations * redo ChatControllerTest * clear chat navigation when chat is disconnected * use `isCloseable` condition for remember chat tab
FAForever · Jun 20, 2024 · 7c903c8 · 7c903c8
1 parent 339162e
commit 7c903c8
Show file tree

Hide file tree

Showing 6 changed files with 224 additions and 124 deletions.
diff --git a/src/main/java/com/faforever/client/chat/ChatController.java b/src/main/java/com/faforever/client/chat/ChatController.java
@@ -2,13 +2,11 @@
 
 import com.faforever.client.exception.ProgrammingError;
 import com.faforever.client.fx.FxApplicationThreadExecutor;
-import com.faforever.client.fx.JavaFxUtil;
 import com.faforever.client.fx.NodeController;
 import com.faforever.client.net.ConnectionState;
 import com.faforever.client.theme.UiService;
-import javafx.collections.ListChangeListener;
 import javafx.collections.MapChangeListener;
-import javafx.collections.WeakListChangeListener;
+import javafx.collections.ObservableList;
 import javafx.collections.WeakMapChangeListener;
 import javafx.scene.control.Tab;
 import javafx.scene.control.TabPane;
@@ -22,9 +20,6 @@
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
 
-import java.util.HashMap;
-import java.util.Map;
-
 @Slf4j
 @Component
 @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)
@@ -34,13 +29,8 @@ public class ChatController extends NodeController<AnchorPane> {
   private final ChatService chatService;
   private final UiService uiService;
   private final FxApplicationThreadExecutor fxApplicationThreadExecutor;
+  private final ChatNavigation chatNavigation;
 
-  private final Map<ChatChannel, AbstractChatTabController> channelToChatTabController = new HashMap<>();
-  private final ListChangeListener<Tab> tabListChangeListener = change -> {
-    while (change.next()) {
-      change.getRemoved().forEach(tab -> channelToChatTabController.remove((ChatChannel) tab.getUserData()));
-    }
-  };
   private final MapChangeListener<String, ChatChannel> channelChangeListener = change -> {
     if (change.wasRemoved()) {
       onChannelLeft(change.getValueRemoved());
@@ -57,16 +47,22 @@ public class ChatController extends NodeController<AnchorPane> {
   public TextField channelNameTextField;
   public VBox disconnectedPane;
 
+  private ObservableList<Tab> openedTabs;
+
   @Override
   protected void onInitialize() {
     super.onInitialize();
+    openedTabs = tabPane.getTabs();
 
     chatService.addChannelsListener(new WeakMapChangeListener<>(channelChangeListener));
     chatService.getChannels().forEach(this::onChannelJoined);
 
     chatService.connectionStateProperty().when(showing).subscribe(this::onConnectionStateChange);
-
-    JavaFxUtil.addListener(tabPane.getTabs(), new WeakListChangeListener<>(tabListChangeListener));
+    tabPane.getSelectionModel().selectedItemProperty().subscribe(tab -> {
+      if (tab.isClosable()) {
+        chatNavigation.setLastOpenedTabId(tab.getId());
+      }
+    });
   }
 
   private void onChannelLeft(ChatChannel chatChannel) {
@@ -90,7 +86,8 @@ private void onDisconnected() {
       disconnectedPane.setVisible(true);
       connectingProgressPane.setVisible(false);
       tabPane.setVisible(false);
-      tabPane.getTabs().removeIf(Tab::isClosable);
+      openedTabs.removeIf(Tab::isClosable);
+      chatNavigation.clear();
     });
   }
 
@@ -112,42 +109,41 @@ private void onConnecting() {
   }
 
   private void removeTab(ChatChannel chatChannel) {
-    AbstractChatTabController controller = channelToChatTabController.remove(chatChannel);
-    if (controller != null) {
-      fxApplicationThreadExecutor.execute(() -> tabPane.getTabs().remove(controller.getRoot()));
-    }
+    fxApplicationThreadExecutor.execute(() -> {
+      String channelName = chatChannel.getName();
+      openedTabs.removeIf(tab -> channelName.equals(tab.getId()));
+      chatNavigation.removeTab(channelName);
+    });
   }
 
   private void addAndSelectTab(ChatChannel chatChannel) {
-    if (!channelToChatTabController.containsKey(chatChannel)) {
+    if (!containsTab(chatChannel)) {
       fxApplicationThreadExecutor.execute(() -> {
-        AbstractChatTabController tabController;
-        if (chatChannel.isPrivateChannel()) {
-          tabController = uiService.loadFxml("theme/chat/private_chat_tab.fxml");
-        } else {
-          tabController = uiService.loadFxml("theme/chat/channel_tab.fxml");
-        }
-        tabController.setChatChannel(chatChannel);
-        channelToChatTabController.put(chatChannel, tabController);
-        Tab tab = tabController.getRoot();
-        tab.setUserData(chatChannel);
+        Tab tab = createTab(chatChannel);
+        int index = chatService.isDefaultChannel(chatChannel) ? 0 : openedTabs.size() - 1; // Last index is `add channel` tab
+        openedTabs.add(index, tab);
 
-
-        if (chatService.isDefaultChannel(chatChannel)) {
-          tabPane.getTabs().addFirst(tab);
+        String channelName = chatChannel.getName();
+        if (chatNavigation.addTabIfMissing(channelName) || channelName.equals(chatNavigation.getLastOpenedTabId())) {
           tabPane.getSelectionModel().select(tab);
-        } else {
-          tabPane.getTabs().add(tabPane.getTabs().size() - 1, tab);
-
-          if (chatChannel.isPrivateChannel() || tabPane.getSelectionModel().getSelectedIndex() == tabPane.getTabs()
-                                                                                                         .size() - 1) {
-            tabPane.getSelectionModel().select(tab);
-          }
         }
       });
     }
   }
 
+  private boolean containsTab(ChatChannel chatChannel) {
+    return openedTabs.stream().anyMatch(tab -> chatChannel.getName().equals(tab.getId()));
+  }
+
+  private Tab createTab(ChatChannel chatChannel) {
+    String fxmlFile = chatChannel.isPrivateChannel() ? "theme/chat/private_chat_tab.fxml" : "theme/chat/channel_tab.fxml";
+    AbstractChatTabController tabController = uiService.loadFxml(fxmlFile);
+    tabController.setChatChannel(chatChannel);
+    Tab tab = tabController.getRoot();
+    tab.setId(chatChannel.getName());
+    return tab;
+  }
+
   private void onConnectionStateChange(ConnectionState newValue) {
     switch (newValue) {
       case DISCONNECTED -> onDisconnected();

diff --git a/src/main/java/com/faforever/client/chat/ChatMessageViewController.java b/src/main/java/com/faforever/client/chat/ChatMessageViewController.java
@@ -32,7 +32,6 @@
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Priority;
 import javafx.scene.layout.VBox;
-import javafx.scene.web.WebView;
 import javafx.stage.Popup;
 import javafx.stage.PopupWindow.AnchorLocation;
 import lombok.RequiredArgsConstructor;
@@ -55,11 +54,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.stream.Collectors;
 
-/**
- * A chat tab displays messages in a {@link WebView}. The WebView is used since text on a JavaFX canvas isn't
- * selectable, but text within a WebView is. This comes with some ugly implications; some logic has to be performed in
- * interaction with JavaScript, like when the user clicks a link.
- */
 @Slf4j
 @RequiredArgsConstructor
 @Component

diff --git a/src/main/java/com/faforever/client/chat/ChatNavigation.java b/src/main/java/com/faforever/client/chat/ChatNavigation.java
@@ -0,0 +1,35 @@
+package com.faforever.client.chat;
+
+import org.jetbrains.annotations.Nullable;
+import org.springframework.context.annotation.Lazy;
+import org.springframework.stereotype.Component;
+
+import java.util.LinkedHashSet;
+
+@Component
+@Lazy
+public class ChatNavigation {
+
+  private final LinkedHashSet<ChatTab> currentTabs = new LinkedHashSet<>();
+
+  public boolean addTabIfMissing(String tabId) {
+    return currentTabs.add(new ChatTab(tabId));
+  }
+
+  public void removeTab(String tabId) {
+    currentTabs.removeIf(chatTab -> chatTab.getId().equals(tabId));
+  }
+
+  @Nullable
+  public String getLastOpenedTabId() {
+    return currentTabs.stream().filter(ChatTab::isSelected).findFirst().map(ChatTab::getId).orElse(null);
+  }
+
+  public void setLastOpenedTabId(String tabId) {
+    currentTabs.forEach(tab -> tab.setSelected(tab.getId().equals(tabId)));
+  }
+
+  public void clear() {
+    currentTabs.clear();
+  }
+}
diff --git a/src/main/java/com/faforever/client/chat/ChatTab.java b/src/main/java/com/faforever/client/chat/ChatTab.java
@@ -0,0 +1,17 @@
+package com.faforever.client.chat;
+
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.Setter;
+
+@RequiredArgsConstructor
+@Getter
+@EqualsAndHashCode(onlyExplicitlyIncluded = true)
+public class ChatTab {
+
+  @EqualsAndHashCode.Include
+  private final String id;
+  @Setter
+  private boolean selected;
+}