(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
This commit is contained in:
Jarosław Sadziński 2021-06-08 09:07:20 +02:00
parent 2f3a0e0c7f
commit 24dca831c0
2 changed files with 23 additions and 3 deletions

View File

@ -1,5 +1,6 @@
import type {GristDoc} from 'app/client/components/GristDoc'; import type {GristDoc} from 'app/client/components/GristDoc';
import type {ColumnRec} from 'app/client/models/entities/ColumnRec'; 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 {buildHighlightedCode, cssCodeBlock} from 'app/client/ui/CodeHighlight';
import {cssLabel, cssRow} from 'app/client/ui/RightPanel'; import {cssLabel, cssRow} from 'app/client/ui/RightPanel';
import {colors, testId, vars} from 'app/client/ui2018/cssVars'; 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 {menu, menuItem} from 'app/client/ui2018/menus';
import {sanitizeIdent} from 'app/common/gutil'; import {sanitizeIdent} from 'app/common/gutil';
import {Computed, dom, fromKo, MultiHolder, Observable, styled, subscribe} from 'grainjs'; import {Computed, dom, fromKo, MultiHolder, Observable, styled, subscribe} from 'grainjs';
import * as ko from 'knockout';
import debounce = require('lodash/debounce'); 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 untieColId = origColumn.untieColIdFromLabel;
const editedLabel = Observable.create(owner, ''); const editedLabel = Observable.create(owner, '');
@ -18,11 +20,23 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec) {
'$' + (edited ? sanitizeIdent(edited) : use(origColumn.colId))); '$' + (edited ? sanitizeIdent(edited) : use(origColumn.colId)));
const saveColId = (val: string) => origColumn.colId.saveOnly(val.startsWith('$') ? val.slice(1) : val); 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 [ return [
cssLabel('COLUMN LABEL AND ID'), cssLabel('COLUMN LABEL AND ID'),
cssRow( cssRow(
cssColLabelBlock( cssColLabelBlock(
textInput(fromKo(origColumn.label), editor = textInput(fromKo(origColumn.label),
async val => { await origColumn.label.saveOnly(val); editedLabel.set(''); }, async val => { await origColumn.label.saveOnly(val); editedLabel.set(''); },
dom.on('input', (ev, elem) => { if (!untieColId.peek()) { editedLabel.set(elem.value); } }), dom.on('input', (ev, elem) => { if (!untieColId.peek()) { editedLabel.set(elem.value); } }),
dom.boolAttr('disabled', origColumn.disableModify), dom.boolAttr('disabled', origColumn.disableModify),

View File

@ -195,11 +195,17 @@ export class RightPanel extends Disposable {
// Builder for the reference display column multiselect. // Builder for the reference display column multiselect.
const refSelect = owner.autoDispose(RefSelect.create({docModel, origColumn, fieldBuilder})); 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 => { return domAsync(imports.loadViewPane().then(ViewPane => {
const {buildNameConfig, buildFormulaConfig} = ViewPane.FieldConfig; const {buildNameConfig, buildFormulaConfig} = ViewPane.FieldConfig;
return dom.maybe(isColumnValid, () => return dom.maybe(isColumnValid, () =>
buildConfigContainer( buildConfigContainer(
dom.create(buildNameConfig, origColumn), dom.create(buildNameConfig, origColumn, cursor),
cssSeparator(), cssSeparator(),
dom.create(buildFormulaConfig, origColumn, this._gristDoc, this._activateFormulaEditor.bind(this)), dom.create(buildFormulaConfig, origColumn, this._gristDoc, this._activateFormulaEditor.bind(this)),
cssSeparator(), cssSeparator(),