(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
This commit is contained in:
Jarosław Sadziński 2024-09-19 17:09:09 +02:00
parent dd22ebdaf0
commit 11e22dd84e
2 changed files with 40 additions and 5 deletions

View File

@ -34,17 +34,18 @@ export function buildNameConfig(
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);
const isSummaryTable = Computed.create(owner, use => Boolean(use(use(origColumn.table).summarySourceTable))); const isSummaryTable = Computed.create(owner, use => Boolean(use(use(origColumn.table).summarySourceTable)));
// We will listen to cursor position and force a blur event on // We will listen to cursor position and force a blur event on both the id and
// the text input, which will trigger save before the column observable // the label inputs, which will trigger save before the column observable
// will change its value. // will change its value.
// Otherwise, blur will be invoked after column change and save handler will // Otherwise, blur will be invoked after column change and save handler will
// update a different column. // update a different column.
let editor: HTMLInputElement | undefined; const editors: HTMLInputElement[] = [];
owner.autoDispose( owner.autoDispose(
cursor.subscribe(() => { cursor.subscribe(() => {
editor?.blur(); editors.forEach(e => e.blur());
}) })
); );
const setEditor = (id: number) => (el: HTMLInputElement) => { editors[id] = el; };
const toggleUntieColId = () => { const toggleUntieColId = () => {
if (!origColumn.disableModify.peek() && !disabled.peek()) { if (!origColumn.disableModify.peek() && !disabled.peek()) {
@ -57,11 +58,12 @@ export function buildNameConfig(
cssRow( cssRow(
dom.cls(cssBlockedCursor.className, origColumn.disableModify), dom.cls(cssBlockedCursor.className, origColumn.disableModify),
cssColLabelBlock( cssColLabelBlock(
editor = cssInput(fromKo(origColumn.label), cssInput(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('readonly', use => use(origColumn.disableModify) || use(disabled)), dom.boolAttr('readonly', use => use(origColumn.disableModify) || use(disabled)),
testId('field-label'), testId('field-label'),
setEditor(0),
), ),
cssInput(editableColId, cssInput(editableColId,
saveColId, saveColId,
@ -70,6 +72,7 @@ export function buildNameConfig(
cssCodeBlock.cls(''), cssCodeBlock.cls(''),
{style: 'margin-top: 8px'}, {style: 'margin-top: 8px'},
testId('field-col-id'), testId('field-col-id'),
setEditor(1),
), ),
), ),
cssColTieBlock( cssColTieBlock(

View File

@ -15,6 +15,38 @@ describe('GridViewBugs', function() {
api = session.createHomeApi(); 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. // 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. // Selection wasn't updated and the column index was out of bounds.
it('should hide multiple columns without an error', async function() { it('should hide multiple columns without an error', async function() {