review feedback

Author: dsmmcken

packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx

@@ -435,18 +435,14 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
 
             actions: this.emptyStringFilterActions(
               column,
-              valueText,
-              value,
               quickFilters.get(modelColumn),
               '&&'
             ),
             order: 2,
             group: ContextActions.groups.high,
           });
         }
-        filterMenu.actions.push(
-          ...this.emptyStringFilterActions(column, valueText, value)
-        );
+        filterMenu.actions.push(...this.emptyStringFilterActions(column));
       } else if (TableUtils.isBooleanType(column.type)) {
         // boolean should have OR condition, and handles it's own null menu options
         if (quickFilters.get(modelColumn)) {
@@ -1561,13 +1557,11 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
 
   emptyStringFilterActions(
     column: Column,
-    valueText: string,
-    value: unknown,
     quickFilter?: QuickFilter,
     operator?: '&&' | '||' | null
   ): ContextAction[] {
     const { dh } = this;
-    const filterValue = dh.FilterValue.ofString(value);
+    const filterValue = dh.FilterValue.ofString('');
     let newQuickFilter:
       | {
           filter: null | FilterCondition | undefined;
@@ -1610,7 +1604,7 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
           ),
           IrisGridContextMenuHandler.getQuickFilterText(
             filterText,
-            `=${valueText}`,
+            `=`,
             operator
           )
         );
@@ -1631,7 +1625,7 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
           ),
           IrisGridContextMenuHandler.getQuickFilterText(
             filterText,
-            `!=${valueText}`,
+            `!=`,
             operator
           )
         );

packages/jsapi-utils/src/TableUtils.test.ts

@@ -523,18 +523,16 @@ describe('quick filter tests', () => {
       (Date.parse(dateString) as unknown) as DateWrapper;
   });
 
-  describe('escapeQuickTextFilter', () => {
-    // test case, expected result
+  describe('Escape/UnescapeQuickTextFilter', () => {
+    // unescaped, escaped
     test.each([
       ['', ''],
       [' ', ' '],
-      [null, null],
       ['test', 'test'],
       ['test\\test', 'test\\test'],
       ['null\\null', 'null\\null'],
       ['\\null\\null', '\\null\\null'],
       ['=test', '\\=test'],
-      ['\\=test', '\\=test'],
       ['\\test', '\\test'],
       ['null', '\\null'],
       ['NULL', '\\NULL'],
@@ -546,58 +544,44 @@ describe('quick filter tests', () => {
       ['!null', '\\!null'],
       ['!=null', '\\!=null'],
       ['*fish', '\\*fish'],
-      ['\\*fish', '\\*fish'],
       ['=*fish', '\\=*fish'],
       ['shooting*', 'shooting\\*'],
-      ['shooting\\*', 'shooting\\*'],
       ['!=shooting*', '\\!=shooting*'],
       ['==', '\\=='],
       ['=', '\\='],
       ['!=', '\\!='],
       ['!~', '\\!~'],
       ['~', '\\~'],
       ['!', '\\!'],
+    ])(
+      'given %p, escape quick filter should return %p and vice versa',
+      (unescaped, escaped) => {
+        expect(TableUtils.escapeQuickTextFilter(unescaped)).toBe(escaped);
+        expect(TableUtils.unescapeQuickTextFilter(escaped)).toBe(unescaped);
+      }
+    );
+
+    // escape specific test cases
+    test.each([
+      [null, null],
+      ['\\=test', '\\=test'],
+      ['\\*fish', '\\*fish'],
+      ['shooting\\*', 'shooting\\*'],
     ])(
       'given %p, escape quick filter should return %p',
       (testCase, expectedResult) => {
         expect(TableUtils.escapeQuickTextFilter(testCase)).toBe(expectedResult);
       }
     );
-  });
 
-  describe('unescapeQuickTextFilter', () => {
-    // test case, expected result
+    // unescape specific test cases
     test.each([
-      ['', ''],
-      [' ', ' '],
-      ['test', 'test'],
-      ['test\\test', 'test\\test'],
-      ['null\\null', 'null\\null'],
-      ['\\null\\null', '\\null\\null'],
-      ['\\=test', '=test'],
       ['=test', '=test'],
-      ['\\test', '\\test'],
       ['null', 'null'],
-      ['\\null', 'null'],
-      ['\\\\null', '\\null'],
-      ['\\NULL', 'NULL'],
-      ['\\=null', '=null'],
-      ['\\!null', '!null'],
-      ['\\!=null', '!=null'],
-      ['\\*fish', '*fish'],
       ['*fish', '*fish'],
-      ['\\=*fish', '=*fish'],
-      ['shooting\\*', 'shooting*'],
       ['shooting*', 'shooting*'],
-      ['\\!=shooting*', '!=shooting*'],
       ['=', '='],
       ['==', '=='],
-      ['\\==', '=='],
-      ['\\=', '='],
-      ['\\!=', '!='],
-      ['\\!~', '!~'],
-      ['\\~', '~'],
-      ['\\!', '!'],
     ])(
       'given %p, unescape quick filter should return %p',
       (testCase, expectedResult) => {