From 11e22dd84ec8e20f9d5d589c6b2e23a7f1ef3e4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?=
 <jaroslaw.sadzinski@gmail.com>
Date: Thu, 19 Sep 2024 17:09:09 +0200
Subject: [PATCH] (core) Renaming column before cursor moves

Summary:
Fix for a bug. If a user tried to trigger the rename
column action by changing a cursor position, the wrong column
was renamed.

Test Plan: Added new test

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D4352
---
 app/client/ui/FieldConfig.ts  | 13 ++++++++-----
 test/nbrowser/GridViewBugs.ts | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/app/client/ui/FieldConfig.ts b/app/client/ui/FieldConfig.ts
index 21286725..5f528a5d 100644
--- a/app/client/ui/FieldConfig.ts
+++ b/app/client/ui/FieldConfig.ts
@@ -34,17 +34,18 @@ export function buildNameConfig(
   const saveColId = (val: string) => origColumn.colId.saveOnly(val.startsWith('$') ? val.slice(1) : val);
 
   const isSummaryTable = Computed.create(owner, use => Boolean(use(use(origColumn.table).summarySourceTable)));
-  // We will listen to cursor position and force a blur event on
-  // the text input, which will trigger save before the column observable
+  // We will listen to cursor position and force a blur event on both the id and
+  // the label inputs, which will trigger save before the column observable
   // will change its value.
   // Otherwise, blur will be invoked after column change and save handler will
   // update a different column.
-  let editor: HTMLInputElement | undefined;
+  const editors: HTMLInputElement[] = [];
   owner.autoDispose(
    cursor.subscribe(() => {
-     editor?.blur();
+     editors.forEach(e => e.blur());
    })
   );
+  const setEditor = (id: number) => (el: HTMLInputElement) => { editors[id] = el; };
 
   const toggleUntieColId = () => {
     if (!origColumn.disableModify.peek() && !disabled.peek()) {
@@ -57,11 +58,12 @@ export function buildNameConfig(
     cssRow(
       dom.cls(cssBlockedCursor.className, origColumn.disableModify),
       cssColLabelBlock(
-        editor = cssInput(fromKo(origColumn.label),
+        cssInput(fromKo(origColumn.label),
           async val => { await origColumn.label.saveOnly(val); editedLabel.set(''); },
           dom.on('input', (ev, elem) => { if (!untieColId.peek()) { editedLabel.set(elem.value); } }),
           dom.boolAttr('readonly', use => use(origColumn.disableModify) || use(disabled)),
           testId('field-label'),
+          setEditor(0),
         ),
         cssInput(editableColId,
           saveColId,
@@ -70,6 +72,7 @@ export function buildNameConfig(
           cssCodeBlock.cls(''),
           {style: 'margin-top: 8px'},
           testId('field-col-id'),
+          setEditor(1),
         ),
       ),
       cssColTieBlock(
diff --git a/test/nbrowser/GridViewBugs.ts b/test/nbrowser/GridViewBugs.ts
index 808ce307..751857d4 100644
--- a/test/nbrowser/GridViewBugs.ts
+++ b/test/nbrowser/GridViewBugs.ts
@@ -15,6 +15,38 @@ describe('GridViewBugs', function() {
     api = session.createHomeApi();
   });
 
+  it('should rename valid column when clicked away', async function() {
+    await gu.openColumnPanel('A');
+
+    // Rename column A to Dummy
+    await toggleDerived();
+    await colId().click();
+    await gu.clearInput();
+    await colId().sendKeys('$Dummy');
+
+    // Now click away, it used to rename the new column to Dummy
+    await gu.getCell('B', 1).click();
+    await gu.waitForServer();
+
+    // Now make sure that column A was renamed to $Dummy
+    await gu.getCell('A', 1).click();
+    assert.equal(await colId().value(), '$Dummy');
+
+    // And that column B is still named $B.
+    await gu.getCell('B', 1).click();
+    assert.equal(await colId().value(), '$B');
+    await gu.undo();
+
+    async function toggleDerived() {
+      await driver.find(".test-field-derive-id").click();
+      await gu.waitForServer();
+    }
+
+    function colId() {
+      return driver.find('.test-field-col-id');
+    }
+  });
+
   // This test is for a bug where hiding multiple columns at once would cause an error in the menu.
   // Selection wasn't updated and the column index was out of bounds.
   it('should hide multiple columns without an error', async function() {