From 24dca831c0a0bc0ccc88edd5b5a64b85d8744749 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?=
 <jaroslaw.sadzinski@gmail.com>
Date: Tue, 8 Jun 2021 09:07:20 +0200
Subject: [PATCH] (core) Renaming column by clicking away resulted in renaming
 different column.

Summary:
Bug summary: if in right bar user starts changing name of column, but then clicks on a different column name in table, THAT column will have its name changed.

This bug occurs because the save method is invoked by a blur event on a input field, which is triggered after all computed observables are calculated. Save method gets an observable to update, which by the time a blur event triggers, is changed to a new column.

The solution was to forcefully trigger the blur event as soon as possible - here by subscribing to the cursor position observable.

Test Plan: Browser tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2845
---
 app/client/ui/FieldConfig.ts | 18 ++++++++++++++++--
 app/client/ui/RightPanel.ts  |  8 +++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/app/client/ui/FieldConfig.ts b/app/client/ui/FieldConfig.ts
index 4a7d3fc3..1ea1ca75 100644
--- a/app/client/ui/FieldConfig.ts
+++ b/app/client/ui/FieldConfig.ts
@@ -1,5 +1,6 @@
 import type {GristDoc} from 'app/client/components/GristDoc';
 import type {ColumnRec} from 'app/client/models/entities/ColumnRec';
+import type {CursorPos} from "app/client/components/Cursor";
 import {buildHighlightedCode, cssCodeBlock} from 'app/client/ui/CodeHighlight';
 import {cssLabel, cssRow} from 'app/client/ui/RightPanel';
 import {colors, testId, vars} from 'app/client/ui2018/cssVars';
@@ -8,9 +9,10 @@ import {cssIconButton, icon} from 'app/client/ui2018/icons';
 import {menu, menuItem} from 'app/client/ui2018/menus';
 import {sanitizeIdent} from 'app/common/gutil';
 import {Computed, dom, fromKo, MultiHolder, Observable, styled, subscribe} from 'grainjs';
+import * as ko from 'knockout';
 import debounce = require('lodash/debounce');
 
-export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec) {
+export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec, cursor: ko.Computed<CursorPos>) {
   const untieColId = origColumn.untieColIdFromLabel;
 
   const editedLabel = Observable.create(owner, '');
@@ -18,11 +20,23 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec) {
     '$' + (edited ? sanitizeIdent(edited) : use(origColumn.colId)));
   const saveColId = (val: string) => origColumn.colId.saveOnly(val.startsWith('$') ? val.slice(1) : val);
 
+  // We will listen to cursor position and force a blur event on
+  // the text input, 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;
+  owner.autoDispose(
+   cursor.subscribe(() => {
+     editor?.blur();
+   })
+  );
+
   return [
     cssLabel('COLUMN LABEL AND ID'),
     cssRow(
       cssColLabelBlock(
-        textInput(fromKo(origColumn.label),
+        editor = textInput(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('disabled', origColumn.disableModify),
diff --git a/app/client/ui/RightPanel.ts b/app/client/ui/RightPanel.ts
index 36494b70..8f15b0cb 100644
--- a/app/client/ui/RightPanel.ts
+++ b/app/client/ui/RightPanel.ts
@@ -195,11 +195,17 @@ export class RightPanel extends Disposable {
     // Builder for the reference display column multiselect.
     const refSelect = owner.autoDispose(RefSelect.create({docModel, origColumn, fieldBuilder}));
 
+    // build cursor position observable
+    const cursor = owner.autoDispose(ko.computed(() => {
+      const vsi = this._gristDoc.viewModel.activeSection().viewInstance();
+      return vsi?.cursor.currentPosition() ?? {};
+    }));
+
     return domAsync(imports.loadViewPane().then(ViewPane => {
       const {buildNameConfig, buildFormulaConfig} = ViewPane.FieldConfig;
       return dom.maybe(isColumnValid, () =>
         buildConfigContainer(
-          dom.create(buildNameConfig, origColumn),
+          dom.create(buildNameConfig, origColumn, cursor),
           cssSeparator(),
           dom.create(buildFormulaConfig, origColumn, this._gristDoc, this._activateFormulaEditor.bind(this)),
           cssSeparator(),