From 44817b0a6d46aaf059b254c697a62331ce66cbc5 Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Thu, 11 Jan 2024 09:40:04 -0500
Subject: [PATCH 1/6] fix(GoToRow): changing column keeps manually entered
 value

---
 packages/iris-grid/src/IrisGrid.tsx | 33 +++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx
index 3d387c75d2..146f6938d0 100644
--- a/packages/iris-grid/src/IrisGrid.tsx
+++ b/packages/iris-grid/src/IrisGrid.tsx
@@ -437,6 +437,7 @@ export interface IrisGridState {
 
   gotoValueSelectedColumnName: ColumnName;
   gotoValueSelectedFilter: FilterTypeValue;
+  goToValueManuallyChanged: boolean;
   gotoValue: string;
 
   columnHeaderGroups: readonly ColumnHeaderGroup[];
@@ -844,6 +845,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       gotoValueSelectedColumnName: model.columns[0]?.name ?? '',
       gotoValueSelectedFilter: FilterType.eqIgnoreCase,
       gotoValue: '',
+      goToValueManuallyChanged: false,
       columnHeaderGroups: columnHeaderGroups ?? model.initialColumnHeaderGroups,
     };
   }
@@ -3943,20 +3945,39 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
   handleGotoValueSelectedColumnNameChanged(columnName: ColumnName): void {
     const { model } = this.props;
     const cursorRow = this.grid?.state.cursorRow;
+    const {
+      gotoValueSelectedColumnName: prevColumnName,
+      goToValueManuallyChanged,
+    } = this.state;
 
     if (cursorRow != null) {
       const index = model.getColumnIndexByName(columnName);
       const column = IrisGridUtils.getColumnByName(model.columns, columnName);
+      const prevColumn = IrisGridUtils.getColumnByName(
+        model.columns,
+        prevColumnName
+      );
       if (index == null || column == null) {
         return;
       }
       const value = model.valueForCell(index, cursorRow);
       const text = IrisGridUtils.convertValueToText(value, column.type);
-      this.setState({
-        gotoValueSelectedColumnName: columnName,
-        gotoValue: text,
-        gotoValueError: '',
-      });
+
+      // do NOT update value if user manually changed value AND column type remains the same
+      if (goToValueManuallyChanged && column.type === prevColumn?.type) {
+        this.setState({
+          gotoValueSelectedColumnName: columnName,
+          gotoValueError: '',
+        });
+      } else {
+        // do update, and set goToValueManuallyChanged to false because value was automatically changed
+        this.setState({
+          gotoValueSelectedColumnName: columnName,
+          gotoValue: text,
+          gotoValueError: '',
+          goToValueManuallyChanged: false,
+        });
+      }
     }
     this.setState({
       gotoValueSelectedColumnName: columnName,
@@ -3969,7 +3990,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
   }
 
   handleGotoValueChanged = (input: string): void => {
-    this.setState({ gotoValue: input });
+    this.setState({ gotoValue: input, goToValueManuallyChanged: true });
     this.debouncedSeekRow(input);
   };
 

From f33dbe5dc1208689f08c11404dcbbfa9af55eadb Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Mon, 15 Jan 2024 11:04:37 -0500
Subject: [PATCH 2/6] test(GoToRow): add changing column tests

---
 packages/iris-grid/src/GotoRow.tsx |   1 +
 tests/table-gotorow.spec.ts        | 145 +++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 tests/table-gotorow.spec.ts

diff --git a/packages/iris-grid/src/GotoRow.tsx b/packages/iris-grid/src/GotoRow.tsx
index 6848d2b09f..6cd5ded240 100644
--- a/packages/iris-grid/src/GotoRow.tsx
+++ b/packages/iris-grid/src/GotoRow.tsx
@@ -372,6 +372,7 @@ const GotoRow = forwardRef<GotoRowElement, GotoRowProps>(
                     }}
                     value={gotoValueSelectedColumnName}
                     aria-label="column-name-select"
+                    id="column-name-select"
                   >
                     {columns.map(column => (
                       <option key={column.name} value={column.name}>
diff --git a/tests/table-gotorow.spec.ts b/tests/table-gotorow.spec.ts
new file mode 100644
index 0000000000..4fbbe7fd26
--- /dev/null
+++ b/tests/table-gotorow.spec.ts
@@ -0,0 +1,145 @@
+import { test, expect, Page } from '@playwright/test';
+import { pasteInMonaco } from './utils';
+
+const customTableCommand = `
+from deephaven import empty_table
+
+size = 20
+
+ordered_int_and_offset = empty_table(20).update([
+    "MyString=(\`str\`+i)",
+    "MyInt1=(i+100)",
+    "MyInt2=(i+200)",
+])`;
+
+async function setColumnAndExpectInputValue({
+  page,
+  setInputValueTo,
+  setColumnNameTo,
+  expectInputValueToBe,
+}: {
+  page: Page;
+  setInputValueTo?: string;
+  setColumnNameTo: string;
+  expectInputValueToBe: string;
+}) {
+  if (setInputValueTo !== undefined) {
+    const inputValue = await page.locator('input[aria-label="Value Input"]');
+    await expect(inputValue).toHaveCount(1);
+    await inputValue.fill(setInputValueTo);
+  }
+
+  const columnSelect = await page.locator('#column-name-select');
+  await expect(columnSelect).toHaveCount(1);
+  await columnSelect.selectOption(setColumnNameTo);
+
+  const inputValue = await page.locator('input[aria-label="Value Input"]');
+  await expect(inputValue).toHaveCount(1);
+  await expect(inputValue).toHaveValue(expectInputValueToBe);
+}
+
+test.describe.configure({ mode: 'serial' });
+
+test.describe('GoToRow change column', () => {
+  let page: Page;
+
+  test.beforeAll(async ({ browser }) => {
+    page = await browser.newPage();
+    await page.goto('');
+    await expect(
+      page.locator('.iris-grid .iris-grid-loading-status')
+    ).toHaveCount(0);
+
+    // create the table
+    const consoleInput = page.locator('.console-input');
+    await pasteInMonaco(consoleInput, customTableCommand);
+    await page.keyboard.press('Enter');
+
+    // wait for panel to show and finish loading
+    await expect(page.locator('.iris-grid-panel')).toHaveCount(1);
+    await expect(page.locator('.iris-grid-panel .loading-spinner')).toHaveCount(
+      0
+    );
+
+    // get the grid
+    const grid = await page.locator('.iris-grid-panel .iris-grid');
+    await expect(grid).toHaveCount(1);
+    const gridLocation = await grid.boundingBox();
+    expect(gridLocation).not.toBeNull();
+    if (gridLocation === null) return;
+    // activate GoToRow
+    const columnHeaderHeight = 30;
+    await page.mouse.click(
+      gridLocation.x + 1,
+      gridLocation.y + 1 + columnHeaderHeight
+    );
+    await page.keyboard.down('Control');
+    await page.keyboard.press('g');
+    await page.keyboard.up('Control');
+
+    // wait for GoToRow bar to show
+    await expect(page.locator('.iris-grid-bottom-bar')).toHaveCount(1);
+  });
+
+  test('unmodified value, different column type', async () => {
+    await setColumnAndExpectInputValue({
+      page,
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '100',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setColumnNameTo: 'MyString',
+      expectInputValueToBe: 'str0',
+    });
+  });
+
+  test('modified set value, different column type', async () => {
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: 'str5',
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '105',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: '110',
+      setColumnNameTo: 'MyString',
+      expectInputValueToBe: 'str10',
+    });
+  });
+
+  test('unmodified set value, same column type', async () => {
+    // set to int1 first (from string)
+    await setColumnAndExpectInputValue({
+      page,
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '110',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setColumnNameTo: 'MyInt2',
+      expectInputValueToBe: '210',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '110',
+    });
+  });
+
+  test('modified set value, same column type', async () => {
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: '115',
+      setColumnNameTo: 'MyInt2',
+      expectInputValueToBe: '115',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: '216',
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '216',
+    });
+  });
+});

From 05ea5a9aa202d7876657e8a634e00bcf060abc54 Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Wed, 17 Jan 2024 14:32:16 -0500
Subject: [PATCH 3/6] test(GoToRow): add a wait for loading

---
 tests/table-gotorow.spec.ts | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/table-gotorow.spec.ts b/tests/table-gotorow.spec.ts
index 4fbbe7fd26..5ea867f235 100644
--- a/tests/table-gotorow.spec.ts
+++ b/tests/table-gotorow.spec.ts
@@ -23,6 +23,7 @@ async function setColumnAndExpectInputValue({
   setColumnNameTo: string;
   expectInputValueToBe: string;
 }) {
+  console.log({ setInputValueTo, setColumnNameTo, expectInputValueToBe });
   if (setInputValueTo !== undefined) {
     const inputValue = await page.locator('input[aria-label="Value Input"]');
     await expect(inputValue).toHaveCount(1);
@@ -38,6 +39,12 @@ async function setColumnAndExpectInputValue({
   await expect(inputValue).toHaveValue(expectInputValueToBe);
 }
 
+async function waitForLoadingDone(page: Page) {
+  await expect(
+    page.locator('.iris-grid .iris-grid-loading-status')
+  ).toHaveCount(0);
+}
+
 test.describe.configure({ mode: 'serial' });
 
 test.describe('GoToRow change column', () => {
@@ -46,9 +53,7 @@ test.describe('GoToRow change column', () => {
   test.beforeAll(async ({ browser }) => {
     page = await browser.newPage();
     await page.goto('');
-    await expect(
-      page.locator('.iris-grid .iris-grid-loading-status')
-    ).toHaveCount(0);
+    await waitForLoadingDone(page);
 
     // create the table
     const consoleInput = page.locator('.console-input');
@@ -63,7 +68,7 @@ test.describe('GoToRow change column', () => {
 
     // get the grid
     const grid = await page.locator('.iris-grid-panel .iris-grid');
-    await expect(grid).toHaveCount(1);
+    await waitForLoadingDone(page);
     const gridLocation = await grid.boundingBox();
     expect(gridLocation).not.toBeNull();
     if (gridLocation === null) return;

From 4507812a281fb6b2c35e999183f8b44b9a5e1172 Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Mon, 22 Jan 2024 11:57:50 -0500
Subject: [PATCH 4/6] fix: add a wait to account for debounce

---
 tests/table-gotorow.spec.ts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/table-gotorow.spec.ts b/tests/table-gotorow.spec.ts
index 5ea867f235..4984719b10 100644
--- a/tests/table-gotorow.spec.ts
+++ b/tests/table-gotorow.spec.ts
@@ -23,18 +23,18 @@ async function setColumnAndExpectInputValue({
   setColumnNameTo: string;
   expectInputValueToBe: string;
 }) {
-  console.log({ setInputValueTo, setColumnNameTo, expectInputValueToBe });
   if (setInputValueTo !== undefined) {
-    const inputValue = await page.locator('input[aria-label="Value Input"]');
+    const inputValue = page.locator('input[aria-label="Value Input"]');
     await expect(inputValue).toHaveCount(1);
     await inputValue.fill(setInputValueTo);
+    await page.waitForTimeout(300);
   }
 
-  const columnSelect = await page.locator('#column-name-select');
+  const columnSelect = page.locator('#column-name-select');
   await expect(columnSelect).toHaveCount(1);
   await columnSelect.selectOption(setColumnNameTo);
 
-  const inputValue = await page.locator('input[aria-label="Value Input"]');
+  const inputValue = page.locator('input[aria-label="Value Input"]');
   await expect(inputValue).toHaveCount(1);
   await expect(inputValue).toHaveValue(expectInputValueToBe);
 }

From 6badd0524acc34f201433403818d15302602799e Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Tue, 23 Jan 2024 15:02:08 -0500
Subject: [PATCH 5/6] feat: reset manually changed state, rename var

---
 packages/iris-grid/src/IrisGrid.tsx | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx
index 6c64fab07e..87ddab5bc0 100644
--- a/packages/iris-grid/src/IrisGrid.tsx
+++ b/packages/iris-grid/src/IrisGrid.tsx
@@ -444,7 +444,7 @@ export interface IrisGridState {
 
   gotoValueSelectedColumnName: ColumnName;
   gotoValueSelectedFilter: FilterTypeValue;
-  goToValueManuallyChanged: boolean;
+  gotoValueManuallyChanged: boolean;
   gotoValue: string;
 
   columnHeaderGroups: readonly ColumnHeaderGroup[];
@@ -852,7 +852,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       gotoValueSelectedColumnName: model.columns[0]?.name ?? '',
       gotoValueSelectedFilter: FilterType.eqIgnoreCase,
       gotoValue: '',
-      goToValueManuallyChanged: false,
+      gotoValueManuallyChanged: false,
       columnHeaderGroups: columnHeaderGroups ?? model.initialColumnHeaderGroups,
     };
   }
@@ -2608,6 +2608,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
         gotoValueSelectedColumnName: columnName,
         gotoRowError: '',
         gotoValueError: '',
+        gotoValueManuallyChanged: false,
       });
       this.focusRowInGrid(row);
       this.gotoRowRef.current?.focus();
@@ -2625,6 +2626,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
         gotoValue: '',
         gotoRowError: '',
         gotoValueError: '',
+        gotoValueManuallyChanged: false,
       });
       return;
     }
@@ -2641,6 +2643,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       gotoValueSelectedColumnName: name,
       gotoRowError: '',
       gotoValueError: '',
+      gotoValueManuallyChanged: false,
     });
   }
 
@@ -2789,11 +2792,11 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
   }
 
   handleGotoRowOpened(): void {
-    this.setState({ isGotoShown: true });
+    this.setState({ isGotoShown: true, gotoValueManuallyChanged: false });
   }
 
   handleGotoRowClosed(): void {
-    this.setState({ isGotoShown: false });
+    this.setState({ isGotoShown: false, gotoValueManuallyChanged: false });
   }
 
   handleAdvancedMenuClosed(columnIndex: number): void {
@@ -3939,7 +3942,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     const cursorRow = this.grid?.state.cursorRow;
     const {
       gotoValueSelectedColumnName: prevColumnName,
-      goToValueManuallyChanged,
+      gotoValueManuallyChanged,
     } = this.state;
 
     if (cursorRow != null) {
@@ -3956,7 +3959,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       const text = IrisGridUtils.convertValueToText(value, column.type);
 
       // do NOT update value if user manually changed value AND column type remains the same
-      if (goToValueManuallyChanged && column.type === prevColumn?.type) {
+      if (gotoValueManuallyChanged && column.type === prevColumn?.type) {
         this.setState({
           gotoValueSelectedColumnName: columnName,
           gotoValueError: '',
@@ -3967,7 +3970,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
           gotoValueSelectedColumnName: columnName,
           gotoValue: text,
           gotoValueError: '',
-          goToValueManuallyChanged: false,
+          gotoValueManuallyChanged: false,
         });
       }
     }
@@ -3982,7 +3985,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
   }
 
   handleGotoValueChanged = (input: string): void => {
-    this.setState({ gotoValue: input, goToValueManuallyChanged: true });
+    this.setState({ gotoValue: input, gotoValueManuallyChanged: true });
     this.debouncedSeekRow(input);
   };
 

From 09805862278fb94ec71fe0f3f9189b2c9ddf877f Mon Sep 17 00:00:00 2001
From: Steven Wu <wusteven815@gmail.com>
Date: Tue, 23 Jan 2024 15:18:36 -0500
Subject: [PATCH 6/6] test(GoToRow): add test for toggled GoToRow

---
 tests/table-gotorow.spec.ts | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/tests/table-gotorow.spec.ts b/tests/table-gotorow.spec.ts
index 4984719b10..bcc5d27c5a 100644
--- a/tests/table-gotorow.spec.ts
+++ b/tests/table-gotorow.spec.ts
@@ -15,14 +15,17 @@ ordered_int_and_offset = empty_table(20).update([
 async function setColumnAndExpectInputValue({
   page,
   setInputValueTo,
+  toggleGoToRow = false,
   setColumnNameTo,
   expectInputValueToBe,
 }: {
   page: Page;
   setInputValueTo?: string;
+  toggleGoToRow?: boolean;
   setColumnNameTo: string;
   expectInputValueToBe: string;
 }) {
+  // get the input value and set it
   if (setInputValueTo !== undefined) {
     const inputValue = page.locator('input[aria-label="Value Input"]');
     await expect(inputValue).toHaveCount(1);
@@ -30,10 +33,19 @@ async function setColumnAndExpectInputValue({
     await page.waitForTimeout(300);
   }
 
+  if (toggleGoToRow) {
+    await page.keyboard.down('Control');
+    await page.keyboard.press('g');
+    await page.keyboard.press('g');
+    await page.keyboard.up('Control');
+  }
+
+  // change the column select to the target
   const columnSelect = page.locator('#column-name-select');
   await expect(columnSelect).toHaveCount(1);
   await columnSelect.selectOption(setColumnNameTo);
 
+  // check the input value
   const inputValue = page.locator('input[aria-label="Value Input"]');
   await expect(inputValue).toHaveCount(1);
   await expect(inputValue).toHaveValue(expectInputValueToBe);
@@ -86,7 +98,7 @@ test.describe('GoToRow change column', () => {
     await expect(page.locator('.iris-grid-bottom-bar')).toHaveCount(1);
   });
 
-  test('unmodified value, different column type', async () => {
+  test('unmodified set value, different column type > change value', async () => {
     await setColumnAndExpectInputValue({
       page,
       setColumnNameTo: 'MyInt1',
@@ -99,7 +111,7 @@ test.describe('GoToRow change column', () => {
     });
   });
 
-  test('modified set value, different column type', async () => {
+  test('modified set value, different column type > change value', async () => {
     await setColumnAndExpectInputValue({
       page,
       setInputValueTo: 'str5',
@@ -114,7 +126,7 @@ test.describe('GoToRow change column', () => {
     });
   });
 
-  test('unmodified set value, same column type', async () => {
+  test('unmodified set value, same column type > change value', async () => {
     // set to int1 first (from string)
     await setColumnAndExpectInputValue({
       page,
@@ -133,7 +145,7 @@ test.describe('GoToRow change column', () => {
     });
   });
 
-  test('modified set value, same column type', async () => {
+  test('modified set value, same column type > keep value', async () => {
     await setColumnAndExpectInputValue({
       page,
       setInputValueTo: '115',
@@ -147,4 +159,21 @@ test.describe('GoToRow change column', () => {
       expectInputValueToBe: '216',
     });
   });
+
+  test('modified set value, same column type, toggled GoToRow > change value', async () => {
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: '110',
+      toggleGoToRow: true,
+      setColumnNameTo: 'MyInt2',
+      expectInputValueToBe: '210',
+    });
+    await setColumnAndExpectInputValue({
+      page,
+      setInputValueTo: '212',
+      toggleGoToRow: true,
+      setColumnNameTo: 'MyInt1',
+      expectInputValueToBe: '112',
+    });
+  });
 });