Fix NamespaceSelectFilter (#4187)

src/renderer/components/+namespaces/namespace-select-filter.tsx

@@ -37,7 +37,7 @@ const Placeholder = observer((props: PlaceholderProps<any, boolean>) => {
   const getPlaceholder = (): React.ReactNode => {
     const namespaces = namespaceStore.contextNamespaces;
 
-    if (!namespaceStore.selectedNamespaces.length || !namespaces.length) {
+    if (namespaceStore.areAllSelectedImplicitly || !namespaces.length) {
       return <>All namespaces</>;
     }
 
@@ -60,6 +60,9 @@ export class NamespaceSelectFilter extends React.Component<SelectProps> {
   static isMultiSelection = observable.box(false);
   static isMenuOpen = observable.box(false);
 
+  /**
+   * Only updated on every open
+   */
   private selected = observable.set<string>();
   private didToggle = false;
 
@@ -88,7 +91,7 @@ export class NamespaceSelectFilter extends React.Component<SelectProps> {
     disposeOnUnmount(this, [
       reaction(() => this.isMenuOpen, newVal => {
         if (newVal) { // rising edge of selection
-          this.selected.replace(namespaceStore.selectedNamespaces);
+          this.selected.replace(namespaceStore.selectedNames);
           this.didToggle = false;
         }
       }),
@@ -116,9 +119,9 @@ export class NamespaceSelectFilter extends React.Component<SelectProps> {
     if (namespace) {
       if (this.isMultiSelection) {
         this.didToggle = true;
-        namespaceStore.toggleContext(namespace);
-      } else {
         namespaceStore.toggleSingle(namespace);
+      } else {
+        namespaceStore.selectSingle(namespace);
       }
     } else {
       namespaceStore.selectAll();

src/renderer/components/+namespaces/namespace.store.ts

@@ -20,7 +20,7 @@
  */
 
 import { action, comparer, computed, IReactionDisposer, IReactionOptions, makeObservable, reaction, } from "mobx";
-import { autoBind, createStorage, noop } from "../../utils";
+import { autoBind, createStorage, noop, ToggleSet } from "../../utils";
 import { KubeObjectStore, KubeObjectStoreLoadingParams } from "../../../common/k8s-api/kube-object.store";
 import { Namespace, namespacesApi } from "../../../common/k8s-api/endpoints/namespaces.api";
 import { apiManager } from "../../../common/k8s-api/api-manager";
@@ -78,7 +78,11 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
     return [];
   }
 
-  @computed get selectedNamespaces(): string[] {
+  /**
+   * @private
+   * The current value (list of namespaces names) in the storage layer
+   */
+  @computed private get selectedNamespaces(): string[] {
     return this.storage.get() ?? [];
   }
 
@@ -89,6 +93,9 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
     ].flat()));
   }
 
+  /**
+   * The list of selected namespace names (for filtering)
+   */
   @computed get contextNamespaces(): string[] {
     if (!this.selectedNamespaces.length) {
       return this.allowedNamespaces; // show all namespaces when nothing selected
@@ -97,6 +104,23 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
     return this.selectedNamespaces;
   }
 
+  /**
+   * The set of select namespace names (for filtering)
+   */
+  @computed get selectedNames(): Set<string> {
+    return new Set(this.contextNamespaces);
+  }
+
+  /**
+   * Is true when the the set of namespace names selected is implicitly all
+   *
+   * Namely, this will be true if the user has deselected all namespaces from
+   * the filter or if the user has clicked the "All Namespaces" option
+   */
+  @computed get areAllSelectedImplicitly(): boolean {
+    return this.selectedNamespaces.length === 0;
+  }
+
   subscribe() {
     /**
      * if user has given static list of namespaces let's not start watches
@@ -142,10 +166,15 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
     }
   }
 
+  /**
+   * Checks if namespace names are selected for filtering
+   * @param namespaces One or several names of namespaces to check if they are selected
+   * @returns `true` if all the provided names are selected
+   */
   hasContext(namespaces: string | string[]): boolean {
     return [namespaces]
       .flat()
-      .every(namespace => this.selectedNamespaces.includes(namespace));
+      .every(namespace => this.selectedNames.has(namespace));
   }
 
   /**
@@ -155,19 +184,39 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
     return this.contextNamespaces.length === this.allowedNamespaces.length;
   }
 
+  /**
+   * Acts like `toggleSingle` but can work on several at a time
+   * @param namespaces One or many names of namespaces to select
+   */
   @action
   toggleContext(namespaces: string | string[]) {
-    if (this.hasContext(namespaces)) {
+    const nextState = new ToggleSet(this.contextNamespaces);
-      this.clearSelected(namespaces);
+
-    } else {
+    for (const namespace of [namespaces].flat()) {
-      this.selectNamespaces([this.selectedNamespaces, namespaces].flat());
+      nextState.toggle(namespace);
-    }
     }
 
-  @action
+    this.storage.set([...nextState]);
+  }
+
+  /**
+   * Toggles the selection state of `namespace`. Namely, if it was previously
+   * specifically or implicitly selected then after this call it will be
+   * explicitly deselected.
+   * @param namespace The name of a namespace
+   */
   toggleSingle(namespace: string){
-    this.clearSelected();
+    const nextState = new ToggleSet(this.contextNamespaces);
-    this.selectNamespaces(namespace);
+
+    nextState.toggle(namespace);
+    this.storage.set([...nextState]);
+  }
+
+  /**
+   * Makes the given namespace the sole selected namespace
+   */
+  selectSingle(namespace: string) {
+    this.storage.set([namespace]);
   }
 
   /**
@@ -180,7 +229,11 @@ export class NamespaceStore extends KubeObjectStore<Namespace> {
   }
 
   /**
-   * @deprecated Use `NamespaceStore.selectAll` instead
+   * This function selects all namespaces implicitly.
+   *
+   * NOTE: does not toggle any namespaces
+   * @param selectAll NOT USED
+   * @deprecated Use `NamespaceStore.selectAll` instead.
    */
   toggleAll(selectAll?: boolean) {
     void selectAll;