Fix menu filtering && refactor to move filtration back to the model

- Sigh... this actually needs to be in the model because the model has
  to be aware of selection, and selection has to be aware of filtering.

- Fix an infinite-recursion bug (wat) with the selection menu

src/globals-ui.ts

@@ -1,6 +1,5 @@
 // istanbul ignore file -- global state for the UI
 
-import {ref, type Ref} from "vue";
 import browser from "webextension-polyfill";
 
 import {KVSCache} from "@/datastore/kvs";
@@ -9,11 +8,6 @@ import * as M from "@/model";
 import type {BookmarkMetadata} from "@/model/bookmark-metadata";
 import type {Favicon} from "@/model/favicons";
 import {resolveNamed} from "@/util";
-import {isFolder, type Folder, type Node} from "./model/bookmarks";
-import type {Tab, Window} from "./model/tabs";
-import {TreeFilter} from "./model/tree-filter";
-
-const filter_fn = ref((_: Window | Tab | Node) => true as boolean);
 
 /** Global variables.  The core conceit here is these are all initialized as
  * `undefined!`, and then initialized properly in the async `init()` function
@@ -25,18 +19,6 @@ const the = {
   /** The main model, describing open windows, saved bookmarks, and any other
    * persistent data kept by Tab Stash. */
   model: undefined! as M.Model,
-
-  /** A predicate function that implements the currently-active search in the UI
-   * (e.g. if the search bar at the top of the page has something in it).  If
-   * there is no active search, the predicate should always return `true`. */
-  filter_fn: undefined! as Ref<(_: Window | Tab | Node) => boolean>,
-
-  /** The filtered state of all open windows/tabs, according to `the.filter_fn`.
-   * */
-  tab_filter: undefined! as TreeFilter<Window, Tab>,
-
-  /** The filtered state of all bookmarks, according to `the.filter_fn`. */
-  bookmark_filter: undefined! as TreeFilter<Folder, Node>,
 };
 
 (<any>globalThis).the = the;
@@ -67,24 +49,4 @@ export async function init() {
       ),
     ),
   });
-
-  the.filter_fn = ref(_ => true);
-
-  the.tab_filter = new TreeFilter<Window, Tab>({
-    isParent(node): node is Window {
-      return "children" in node;
-    },
-    predicate(node) {
-      return filter_fn.value(node);
-    },
-  });
-
-  the.bookmark_filter = new TreeFilter<Folder, Node>({
-    isParent(node): node is Folder {
-      return isFolder(node);
-    },
-    predicate(node) {
-      return filter_fn.value(node);
-    },
-  });
 }

src/model/index.ts

@@ -30,6 +30,7 @@
 //   mutating and accessing the state in various ways that a user might want to
 //   perform.  All the business logic resides here.
 
+import {computed, ref} from "vue";
 import browser from "webextension-polyfill";
 
 import {
@@ -38,6 +39,7 @@ import {
   filterMap,
   shortPoll,
   TaskMonitor,
+  textMatcher,
   tryAgain,
   urlToOpen,
 } from "../util";
@@ -54,6 +56,7 @@ import * as Options from "./options";
 import * as Selection from "./selection";
 import * as Tabs from "./tabs";
 import {pathTo} from "./tree";
+import {TreeFilter} from "./tree-filter";
 
 export {
   BookmarkMetadata,
@@ -82,6 +85,10 @@ export type ModelItem = Bookmarks.Node | Tabs.Tab;
 export type NewTab = {title?: string; url: string};
 export type NewFolder = {title: string; children: (NewTab | NewFolder)[]};
 
+export const isModelParent = (
+  item: ModelItem | Tabs.Window,
+): item is Bookmarks.Folder | Tabs.Window => "children" in item;
+
 export const isModelItem = (item: StashItem): item is ModelItem => "id" in item;
 
 export const isTab = (item: StashItem): item is Tabs.Tab =>
@@ -139,6 +146,12 @@ export class Model {
   readonly bookmark_metadata: BookmarkMetadata.Model;
   readonly selection: Selection.Model;
 
+  readonly searchText = ref("");
+  readonly filter: TreeFilter<
+    Tabs.Window | Bookmarks.Folder,
+    Bookmarks.Node | Tabs.Tab
+  >;
+
   constructor(src: Source) {
     this.browser_settings = src.browser_settings;
     this.options = src.options;
@@ -151,6 +164,19 @@ export class Model {
     this.favicons = src.favicons;
     this.bookmark_metadata = src.bookmark_metadata;
     this.selection = new Selection.Model([this.tabs, this.bookmarks]);
+
+    this.filter = new TreeFilter(
+      isModelParent,
+      computed(() => {
+        const searchText = this.searchText.value;
+        if (!searchText) return _ => true;
+
+        const matcher = textMatcher(searchText);
+        return node =>
+          ("title" in node && matcher(node.title)) ||
+          ("url" in node && matcher(node.url));
+      }),
+    );
   }
 
   /** Reload model data (where possible) in the event of an unexpected issue.

src/model/tree-filter.test.ts

@@ -1,9 +1,14 @@
 import {expect} from "chai";
 import {nextTick, ref, type Ref} from "vue";
 
-import {TreeFilter, type FilteredTreeAccessors} from "./tree-filter";
+import {TreeFilter} from "./tree-filter";
 
-import {makeDefaultTree, type TestNode, type TestParent} from "./tree.test";
+import {
+  isTestParent,
+  makeDefaultTree,
+  type TestNode,
+  type TestParent,
+} from "./tree.test";
 
 type Parent = TestParent;
 type Child = TestNode;
@@ -15,15 +20,6 @@ describe("model/tree-filter", () => {
   // istanbul ignore next
   const predicate: Ref<(n: Parent | Child) => boolean> = ref(_ => false);
 
-  const accessors: FilteredTreeAccessors<TestParent, TestNode> = {
-    isParent(node): node is TestParent {
-      return "children" in node;
-    },
-    predicate(node): boolean {
-      return predicate.value(node);
-    },
-  };
-
   function checkFilterInvariants() {
     const visit = (n: Parent | Child) => {
       const i = treeFilter.info(n);
@@ -56,7 +52,7 @@ describe("model/tree-filter", () => {
   beforeEach(() => {
     // istanbul ignore next
     predicate.value = _ => false;
-    treeFilter = new TreeFilter(accessors);
+    treeFilter = new TreeFilter(isTestParent, predicate);
   });
 
   it("reports when nothing matches the filter", () => {

src/model/tree-filter.ts

@@ -1,4 +1,4 @@
-import {computed, reactive} from "vue";
+import {computed, reactive, type Ref} from "vue";
 
 import {computedLazyEq} from "../util";
 import type {IsParentFn, TreeNode, TreeParent} from "./tree";
@@ -17,32 +17,24 @@ export interface FilterInfo {
   readonly nonMatchingCount: number;
 }
 
-export interface FilteredTreeAccessors<
-  P extends TreeParent<P, N>,
-  N extends TreeNode<P, N>,
-> {
-  /** Check if this node is a parent node or not. */
-  isParent(node: P | N): node is P;
-
-  /** The predicate function.  This function is called from within a Vue
-   * computed() context, so if the predicate function relies on any reactive
-   * values to compute the result, it will automatically be re-run when those
-   * values change. */
-  predicate(node: P | N): boolean;
-}
-
 /** A Tree whose nodes have been filtered by a predicate function. */
-export class TreeFilter<P extends TreeParent<P, N>, N extends TreeNode<P, N>>
-  implements FilteredTreeAccessors<P, N>
-{
+export class TreeFilter<P extends TreeParent<P, N>, N extends TreeNode<P, N>> {
+  /** Check if a particular node is a parent node or not. */
   readonly isParent: IsParentFn<P, N>;
-  readonly predicate: (node: P | N) => boolean;
+
+  /** The predicate function used to determine whether a node `isMatching` or
+   * not.  Updating this ref will update the `.isMatching` property on every
+   * node. */
+  readonly predicate: Ref<(node: P | N) => boolean>;
 
   private readonly nodes = new WeakMap<P | N, FilterInfo>();
 
-  constructor(accessors: FilteredTreeAccessors<P, N>) {
-    this.isParent = accessors.isParent;
-    this.predicate = accessors.predicate;
+  constructor(
+    isParent: IsParentFn<P, N>,
+    predicate: Ref<(node: P | N) => boolean>,
+  ) {
+    this.isParent = isParent;
+    this.predicate = predicate;
   }
 
   /** Returns a FilterInfo object describing whether this node (and/or its
@@ -53,7 +45,7 @@ export class TreeFilter<P extends TreeParent<P, N>, N extends TreeNode<P, N>>
 
     const isParent = this.isParent(node);
 
-    const isMatching = computed(() => this.predicate(node));
+    const isMatching = computed(() => this.predicate.value(node));
 
     const hasMatchInSubtree = isParent
       ? computedLazyEq(() => {

src/model/tree.test.ts

@@ -23,6 +23,10 @@ export type TestPosition = TreePosition<TestParent, TestNode>;
 export type TestNodeDef = string | TestParentDef;
 export type TestParentDef = {name: string; children: TestNodeDef[]};
 
+export function isTestParent(n: TestNode): n is TestParent {
+  return "children" in n;
+}
+
 export function makeTree(
   def: TestParentDef,
 ): [TestParent, Record<string, TestParent>, Record<string, TestNode>] {

src/stash-list/bookmark.vue

@@ -116,7 +116,7 @@ export default defineComponent({
     bgKey: bgKeyName,
 
     filterInfo(): FilterInfo {
-      return the.bookmark_filter.info(this.bookmark);
+      return the.model.filter.info(this.bookmark);
     },
 
     relatedTabs(): readonly Tab[] {

src/stash-list/folder-list.vue

@@ -52,7 +52,7 @@ export default defineComponent({
     isFolder,
 
     itemClasses(f: Node): Record<string, boolean> {
-      const fi = the.bookmark_filter.info(f);
+      const fi = the.model.filter.info(f);
       return {
         hidden:
           !isFolder(f) ||

src/stash-list/folder.vue

@@ -291,7 +291,7 @@ export default defineComponent({
     },
 
     filterInfo() {
-      return the.bookmark_filter.info(this.folder);
+      return the.model.filter.info(this.folder);
     },
 
     metadata(): BookmarkMetadataEntry {
@@ -469,7 +469,7 @@ export default defineComponent({
     },
 
     childClasses(node: Node): Record<string, boolean> {
-      const fi = the.bookmark_filter.info(node);
+      const fi = the.model.filter.info(node);
       return {
         hidden: !(
           this.isValidChild(node) &&

src/stash-list/index.vue

@@ -149,11 +149,10 @@ import {
   friendlyFolderName,
   type Folder,
   type FolderStats,
-  type Node,
 } from "../model/bookmarks";
-import type {Tab, Window} from "../model/tabs";
+import type {Tab} from "../model/tabs";
 import {fetchInfoForSites} from "../tasks/siteinfo";
-import {TaskMonitor, parseVersion, textMatcher} from "../util";
+import {TaskMonitor, parseVersion} from "../util";
 
 import Menu from "../components/menu.vue";
 import Notification from "../components/notification.vue";
@@ -197,14 +196,6 @@ export default defineComponent({
       return document.documentElement.dataset!.view ?? "tab";
     },
 
-    filterFn(): (node: Window | Tab | Node) => boolean {
-      if (!this.searchText) return _ => true;
-      const matcher = textMatcher(this.searchText);
-      return node =>
-        ("title" in node && matcher(node.title)) ||
-        ("url" in node && matcher(node.url));
-    },
-
     stash_root_warning(): {text: string; help: () => void} | undefined {
       return the.model.bookmarks.stash_root_warning.value;
     },
@@ -337,8 +328,8 @@ export default defineComponent({
   },
 
   watch: {
-    filterFn() {
-      the.filter_fn.value = this.filterFn;
+    searchText() {
+      the.model.searchText.value = this.searchText;
     },
   },
 

src/stash-list/select-folder.vue

@@ -2,14 +2,14 @@
   <ul>
     <li v-for="folder of visibleChildFolders">
       <button
-        :class="props.buttonClasses(props.folder)"
-        :title="props.tooltips(props.folder)"
-        @click.prevent="emit('select', $event, props.folder)"
+        :class="props.buttonClasses(folder)"
+        :title="props.tooltips(folder)"
+        @click.prevent="emit('select', $event, folder)"
       >
         <span>{{ friendlyFolderName(folder.title) }}</span>
       </button>
       <Self
-        :folder="props.folder"
+        :folder="folder"
         :tooltips="props.tooltips"
         :button-classes="props.buttonClasses"
         @select="(ev, folder) => emit('select', ev, folder)"

src/stash-list/selection-menu.vue

@@ -74,7 +74,7 @@
 </template>
 
 <script lang="ts">
-import {defineComponent} from "vue";
+import {computed, defineComponent} from "vue";
 
 import {altKeyName, textMatcher} from "../util";
 
@@ -105,16 +105,11 @@ export default defineComponent({
   }),
 
   provide() {
-    const self = this;
     return {
-      bookmark_filter: new TreeFilter<Folder, Node>({
-        isParent(node): node is Folder {
-          return isFolder(node);
-        },
-        predicate(node) {
-          return isFolder(node) && self.filter(friendlyFolderName(node.title));
-        },
-      }),
+      bookmark_filter: new TreeFilter<Folder, Node>(
+        isFolder,
+        computed(() => this.filter),
+      ),
     };
   },
 
@@ -129,8 +124,9 @@ export default defineComponent({
       return the.model.selection.selectedCount.value;
     },
 
-    filter(): (text: string) => boolean {
-      return textMatcher(this.searchText);
+    filter(): (node: Folder | Node) => boolean {
+      const matcher = textMatcher(this.searchText);
+      return node => isFolder(node) && matcher(friendlyFolderName(node.title));
     },
 
     createTitle(): string {

src/stash-list/tab.vue

@@ -83,7 +83,7 @@ export default defineComponent({
 
   computed: {
     filterInfo() {
-      return the.tab_filter.info(this.tab);
+      return the.model.filter.info(this.tab);
     },
 
     altKey: altKeyName,