Fix file browser search highlighting bug (jupyterlab#12538)

* Fix file browser search highlighting bug * Move to use IScore | null to prevent API break * Fix failing test * Revert type of IScore to original * Update packages/ui-components/src/components/search.tsx Co-authored-by: Frédéric Collonval <[email protected]>
gabalafou · May 13, 2022 · 222b472 · 222b472
1 parent d5d5fb9
commit 222b472
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 32 deletions.
diff --git a/packages/filebrowser/src/browser.ts b/packages/filebrowser/src/browser.ts
@@ -7,6 +7,7 @@ import { Contents, ServerConnection } from '@jupyterlab/services';
 import { ITranslator, nullTranslator } from '@jupyterlab/translation';
 import {
   FilenameSearcher,
+  IScore,
   ReactWidget,
   SidePanel
 } from '@jupyterlab/ui-components';
@@ -97,7 +98,10 @@ export class FileBrowser extends SidePanel {
     this.listing.addClass(LISTING_CLASS);
 
     this._filenameSearcher = FilenameSearcher({
-      updateFilter: (filterFn: (item: string) => boolean) => {
+      updateFilter: (
+        filterFn: (item: string) => Partial<IScore> | null,
+        query?: string
+      ) => {
         this.listing.model.setFilter(value => {
           return filterFn(value.name.toLowerCase());
         });
@@ -161,7 +165,10 @@ export class FileBrowser extends SidePanel {
     this._filenameSearcher.dispose();
 
     this._filenameSearcher = FilenameSearcher({
-      updateFilter: (filterFn: (item: string) => boolean) => {
+      updateFilter: (
+        filterFn: (item: string) => Partial<IScore> | null,
+        query?: string
+      ) => {
         this.listing.model.setFilter(value => {
           return filterFn(value.name.toLowerCase());
         });

diff --git a/packages/filebrowser/src/model.ts b/packages/filebrowser/src/model.ts
@@ -11,6 +11,7 @@ import {
   nullTranslator,
   TranslationBundle
 } from '@jupyterlab/translation';
+import { IScore } from '@jupyterlab/ui-components';
 import {
   ArrayExt,
   ArrayIterator,
@@ -773,7 +774,11 @@ export namespace TogglableHiddenFileBrowserModel {
 export class FilterFileBrowserModel extends TogglableHiddenFileBrowserModel {
   constructor(options: FilterFileBrowserModel.IOptions) {
     super(options);
-    this._filter = options.filter ? options.filter : model => true;
+    this._filter =
+      options.filter ??
+      (model => {
+        return {};
+      });
     this._filterDirectories = options.filterDirectories ?? true;
   }
 
@@ -797,17 +802,19 @@ export class FilterFileBrowserModel extends TogglableHiddenFileBrowserModel {
       if (!this._filterDirectories && value.type === 'directory') {
         return true;
       } else {
-        return this._filter(value);
+        const filtered = this._filter(value);
+        value.indices = filtered?.indices;
+        return !!filtered;
       }
     });
   }
 
-  setFilter(filter: (value: Contents.IModel) => boolean): void {
+  setFilter(filter: (value: Contents.IModel) => Partial<IScore> | null): void {
     this._filter = filter;
     void this.refresh();
   }
 
-  private _filter: (value: Contents.IModel) => boolean;
+  private _filter: (value: Contents.IModel) => Partial<IScore> | null;
   private _filterDirectories: boolean;
 }
 
@@ -822,7 +829,7 @@ export namespace FilterFileBrowserModel {
     /**
      * Filter function on file browser item model
      */
-    filter?: (value: Contents.IModel) => boolean;
+    filter?: (value: Contents.IModel) => Partial<IScore> | null;
 
     /**
      * Filter directories

diff --git a/packages/filebrowser/src/opendialog.ts b/packages/filebrowser/src/opendialog.ts
@@ -6,7 +6,7 @@ import { PathExt } from '@jupyterlab/coreutils';
 import { IDocumentManager } from '@jupyterlab/docmanager';
 import { Contents } from '@jupyterlab/services';
 import { ITranslator, nullTranslator } from '@jupyterlab/translation';
-import { newFolderIcon, refreshIcon } from '@jupyterlab/ui-components';
+import { IScore, newFolderIcon, refreshIcon } from '@jupyterlab/ui-components';
 import { toArray } from '@lumino/algorithm';
 import { PanelLayout, Widget } from '@lumino/widgets';
 import { FileBrowser } from './browser';
@@ -53,7 +53,7 @@ export namespace FileDialog {
     /**
      * Filter function on file browser item model
      */
-    filter?: (value: Contents.IModel) => boolean;
+    filter?: (value: Contents.IModel) => Partial<IScore> | null;
 
     /**
      * The application language translator.
@@ -108,7 +108,9 @@ export namespace FileDialog {
   ): Promise<Dialog.IResult<Contents.IModel[]>> {
     return getOpenFiles({
       ...options,
-      filter: model => model.type === 'directory'
+      filter: model => {
+        return model.type === 'directory' ? {} : null;
+      }
     });
   }
 }
@@ -121,7 +123,7 @@ class OpenDialog
   implements Dialog.IBodyWidget<Contents.IModel[]> {
   constructor(
     manager: IDocumentManager,
-    filter?: (value: Contents.IModel) => boolean,
+    filter?: (value: Contents.IModel) => Partial<IScore> | null,
     translator?: ITranslator,
     filterDirectories?: boolean
   ) {
@@ -230,7 +232,7 @@ namespace Private {
   export const createFilteredFileBrowser = (
     id: string,
     manager: IDocumentManager,
-    filter?: (value: Contents.IModel) => boolean,
+    filter?: (value: Contents.IModel) => Partial<IScore> | null,
     options: IFileBrowserFactory.IOptions = {},
     translator?: ITranslator,
     filterDirectories?: boolean

diff --git a/packages/filebrowser/test/openfiledialog.spec.ts b/packages/filebrowser/test/openfiledialog.spec.ts
@@ -55,7 +55,7 @@ describe('@jupyterlab/filebrowser', () => {
       it('should accept filter option', () => {
         const model = new FilterFileBrowserModel({
           manager,
-          filter: (model: Contents.IModel) => false
+          filter: (model: Contents.IModel) => null
         });
         expect(model).toBeInstanceOf(FilterFileBrowserModel);
       });
@@ -78,7 +78,7 @@ describe('@jupyterlab/filebrowser', () => {
       it('should list all directories if filterDirectories is false', async () => {
         const filteredModel = new FilterFileBrowserModel({
           manager,
-          filter: (model: Contents.IModel) => false,
+          filter: (model: Contents.IModel) => null,
           filterDirectories: false
         });
         await filteredModel.cd();
@@ -94,7 +94,7 @@ describe('@jupyterlab/filebrowser', () => {
       it('should filter files and directories if filterDirectories is true', async () => {
         const filteredModel = new FilterFileBrowserModel({
           manager,
-          filter: (model: Contents.IModel) => false,
+          filter: (model: Contents.IModel) => null,
           filterDirectories: true
         });
         await filteredModel.cd();
@@ -108,7 +108,8 @@ describe('@jupyterlab/filebrowser', () => {
       it('should respect the filter', async () => {
         const filteredModel = new FilterFileBrowserModel({
           manager,
-          filter: (model: Contents.IModel) => model.type === 'notebook'
+          filter: (model: Contents.IModel) =>
+            model.type === 'notebook' ? {} : null
         });
         await filteredModel.cd();
         const model = new FileBrowserModel({ manager });
@@ -151,7 +152,8 @@ describe('@jupyterlab/filebrowser', () => {
         manager,
         title: 'Select a notebook',
         host: node,
-        filter: (value: Contents.IModel) => value.type === 'notebook'
+        filter: (value: Contents.IModel) =>
+          value.type === 'notebook' ? {} : null
       });
 
       await acceptDialog();
@@ -174,7 +176,8 @@ describe('@jupyterlab/filebrowser', () => {
         manager,
         title: 'Select a notebook',
         host: node,
-        filter: (value: Contents.IModel) => value.type === 'notebook'
+        filter: (value: Contents.IModel) =>
+          value.type === 'notebook' ? {} : null
       });
 
       await waitForDialog();

diff --git a/packages/settingeditor/src/pluginlist.tsx b/packages/settingeditor/src/pluginlist.tsx
@@ -9,6 +9,7 @@ import { ITranslator, nullTranslator } from '@jupyterlab/translation';
 import {
   classes,
   FilterBox,
+  IScore,
   LabIcon,
   settingsIcon,
   updateFilterFunction
@@ -241,7 +242,7 @@ export class PluginList extends ReactWidget {
    * @returns - String array of properties that match the search results.
    */
   getFilterString(
-    filter: (item: string) => boolean,
+    filter: (item: string) => Partial<IScore> | null,
     props: ISettingRegistry.IProperty,
     definitions?: any,
     ref?: string
@@ -319,7 +320,10 @@ export class PluginList extends ReactWidget {
    * Updates the filter when the search bar value changes.
    * @param filter Filter function passed by search bar based on search value.
    */
-  setFilter(filter: (item: string) => boolean, query?: string): void {
+  setFilter(
+    filter: (item: string) => Partial<IScore> | null,
+    query?: string
+  ): void {
     this._filter = (plugin: ISettingRegistry.IPlugin): string[] | null => {
       if (filter(plugin.schema.title ?? '')) {
         return null;

diff --git a/packages/ui-components/src/components/search.tsx b/packages/ui-components/src/components/search.tsx
@@ -13,7 +13,10 @@ export interface IFilterBoxProps {
   /**
    * A function to callback when filter is updated.
    */
-  updateFilter: (filterFn: (item: string) => boolean, query?: string) => void;
+  updateFilter: (
+    filterFn: (item: string) => Partial<IScore> | null,
+    query?: string
+  ) => void;
 
   /**
    * Whether to use the fuzzy filter.
@@ -44,7 +47,7 @@ export interface IFilterBoxProps {
 /**
  * A text match score with associated content item.
  */
-interface IScore {
+export interface IScore {
   /**
    * The numerical score for the text match.
    */
@@ -59,7 +62,7 @@ interface IScore {
 /**
  * Perform a fuzzy search on a single item.
  */
-function fuzzySearch(source: string, query: string): IScore | null {
+export function fuzzySearch(source: string, query: string): IScore | null {
   // Set up the match score and indices array.
   let score = Infinity;
   let indices: number[] | null = null;
@@ -111,26 +114,24 @@ export const updateFilterFunction = (
   useFuzzyFilter: boolean,
   caseSensitive?: boolean
 ) => {
-  return (item: string) => {
+  return (item: string): Partial<IScore> | null => {
     if (useFuzzyFilter) {
       // Run the fuzzy search for the item and query.
       const query = value.toLowerCase();
-      let score = fuzzySearch(item, query);
       // Ignore the item if it is not a match.
-      if (!score) {
-        return false;
-      }
-      return true;
+      return fuzzySearch(item, query);
     }
     if (!caseSensitive) {
       item = item.toLocaleLowerCase();
       value = value.toLocaleLowerCase();
     }
     const i = item.indexOf(value);
     if (i === -1) {
-      return false;
+      return null;
     }
-    return true;
+    return {
+      indices: [...Array(item.length).keys()].map(x => x + 1)
+    };
   };
 };
 
@@ -140,7 +141,7 @@ export const FilterBox = (props: IFilterBoxProps) => {
   if (props.forceRefresh) {
     useEffect(() => {
       props.updateFilter((item: string) => {
-        return true;
+        return {};
       });
     }, []);
   }