From e482427e8312f40b0d04c1423c35cb9d9b636304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 30 Nov 2021 09:59:04 +0100 Subject: [PATCH] (core) JS error on creator panel and formula editor. Summary: Fixing js error that happens when closing creator panel with active formula editor. Styling behavior menu with common styles. Test Plan: Browser tests Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D3150 --- app/client/components/GristDoc.ts | 13 ++++++-- app/client/ui/FieldConfig.ts | 50 ++++++++++++++++++++---------- app/client/ui/RightPanel.ts | 4 +-- app/client/ui2018/menus.ts | 3 +- app/client/widgets/FieldBuilder.ts | 2 +- app/client/widgets/FieldEditor.ts | 4 +-- 6 files changed, 51 insertions(+), 25 deletions(-) diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 573b47ce..6b5797b0 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -625,10 +625,19 @@ export class GristDoc extends DisposableWithEvents { ); } + // Updates formula for a column. + public async updateFormula(colRef: number, formula: string): Promise { + return this.docModel.columns.sendTableAction( + ['UpdateRecord', colRef, { + formula, + }] + ); + } + // Convert column to pure formula column. - public async convertToFormula(colRefs: number, formula: string): Promise { + public async convertToFormula(colRef: number, formula: string): Promise { return this.docModel.columns.sendTableAction( - ['UpdateRecord', colRefs, { + ['UpdateRecord', colRef, { isFormula: true, formula, recalcWhen: RecalcWhen.DEFAULT, diff --git a/app/client/ui/FieldConfig.ts b/app/client/ui/FieldConfig.ts index f1c6f243..295a5c97 100644 --- a/app/client/ui/FieldConfig.ts +++ b/app/client/ui/FieldConfig.ts @@ -66,10 +66,11 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec, curso ]; } +type SaveHandler = (column: ColumnRec, formula: string) => Promise; type BuildEditor = ( cellElem: Element, editValue?: string, - onSave?: (formula: string) => Promise, + onSave?: SaveHandler, onCancel?: () => void) => void; type BEHAVIOR = "empty"|"formula"|"data"; @@ -111,6 +112,8 @@ export function buildFormulaConfig( // Clear state when column has changed owner.autoDispose(origColumn.id.subscribe(clearState)); + owner.autoDispose(origColumn.formula.subscribe(clearState)); + owner.autoDispose(origColumn.isFormula.subscribe(clearState)); // Menu helper that will show normal menu with some default options const menu = (label: DomContents, options: DomElementArg[]) => @@ -177,31 +180,44 @@ export function buildFormulaConfig( const setFormula = () => (maybeFormula.set(true), formulaField?.focus()); const setTrigger = () => (maybeTrigger.set(true), formulaField?.focus()); - // Actions on save formula + // Actions on save formula. Those actions are using column that comes from FormulaEditor. + // Formula editor scope is broader then RightPanel, it can be disposed after RightPanel is closed, + // and in some cases, when window is in background, it won't be disposed at all when panel is closed. - // Converts column to formula column or updates formula on a formula column. - const onSaveConvertToFormula = async (formula: string) => { + // Converts column to formula column. + const onSaveConvertToFormula = async (column: ColumnRec, formula: string) => { + // For non formula column, we will not convert it to formula column when expression is empty, + // as it means we were trying to convert data column to formula column, but changed our mind. const notBlank = Boolean(formula); - const trueFormula = !maybeFormula.get(); - if (notBlank || trueFormula) { await gristDoc.convertToFormula(origColumn.id.peek(), formula); } - clearState(); + // But when the column is a formula column, empty formula expression is acceptable (it will + // convert column to empty column). + const trueFormula = column.formula.peek(); + if (notBlank || trueFormula) { await gristDoc.convertToFormula(column.id.peek(), formula); } + // Clear state only when owner was not disposed + if (!owner.isDisposed()) { + clearState(); + } }; // Updates formula or convert column to trigger formula column if necessary. - const onSaveConvertToTrigger = async (formula: string) => { - if (formula && maybeTrigger.get()) { - // Convert column to trigger - await gristDoc.convertToTrigger(origColumn.id.peek(), formula); - } else if (origColumn.hasTriggerFormula.peek()) { - // This is true trigger formula, just update the formula (or make it blank) - await origColumn.formula.setAndSave(formula); + const onSaveConvertToTrigger = async (column: ColumnRec, formula: string) => { + // If formula expression is not empty, and column was plain data column (without a formula) + if (formula && !column.hasTriggerFormula.peek()) { + // then convert column to a trigger formula column + await gristDoc.convertToTrigger(column.id.peek(), formula); + } else if (column.hasTriggerFormula.peek()) { + // else, if it was already a trigger formula column, just update formula. + await gristDoc.updateFormula(column.id.peek(), formula); + } + // Clear state only when owner was not disposed + if (!owner.isDisposed()) { + clearState(); } - clearState(); }; const errorMessage = createFormulaErrorObs(owner, gristDoc, origColumn); // Helper that will create different flavors for formula builder. - const formulaBuilder = (onSave: (formula: string) => Promise) => [ + const formulaBuilder = (onSave: SaveHandler) => [ cssRow(formulaField = buildFormula( origColumn, buildEditor, @@ -286,7 +302,7 @@ function buildFormula( column: ColumnRec, buildEditor: BuildEditor, placeholder: string, - onSave?: (formula: string) => Promise, + onSave?: SaveHandler, onCancel?: () => void) { return cssFieldFormula(column.formula, {placeholder, maxLines: 2}, dom.cls('formula_field_sidepane'), diff --git a/app/client/ui/RightPanel.ts b/app/client/ui/RightPanel.ts index ac0498cf..015b7a12 100644 --- a/app/client/ui/RightPanel.ts +++ b/app/client/ui/RightPanel.ts @@ -22,7 +22,7 @@ import {domAsync} from 'app/client/lib/domAsync'; import * as imports from 'app/client/lib/imports'; import {createSessionObs} from 'app/client/lib/sessionObs'; import {reportError} from 'app/client/models/AppModel'; -import {ViewSectionRec} from 'app/client/models/DocModel'; +import {ColumnRec, ViewSectionRec} from 'app/client/models/DocModel'; import {GridOptions} from 'app/client/ui/GridOptions'; import {attachPageWidgetPicker, IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker'; import {linkFromId, linkId, selectBy} from 'app/client/ui/selectBy'; @@ -236,7 +236,7 @@ export class RightPanel extends Disposable { // Simulate user typing on the cell - open editor with an initial value. editValue?: string, // Custom save handler. - onSave?: (formula: string) => Promise, + onSave?: (column: ColumnRec, formula: string) => Promise, // Custom cancel handler. onCancel?: () => void,) { const vsi = this._gristDoc.viewModel.activeSection().viewInstance(); diff --git a/app/client/ui2018/menus.ts b/app/client/ui2018/menus.ts index b70b3edf..6c77229e 100644 --- a/app/client/ui2018/menus.ts +++ b/app/client/ui2018/menus.ts @@ -335,6 +335,7 @@ export function selectMenu( items: () => DomElementArg[], ...args: IDomArgs ) { + const _menu = cssSelectMenuElem(testId('select-menu')); return cssSelectBtn( label, icon('Dropdown'), @@ -342,7 +343,7 @@ export function selectMenu( items, { ...weasel.defaultMenuOptions, - menuCssClass: cssSelectMenuElem.className + ' grist-floating-menu', + menuCssClass: _menu.className + ' grist-floating-menu', stretchToSelector : `.${cssSelectBtn.className}`, trigger : [(triggerElem, ctl) => { const isDisabled = () => triggerElem.classList.contains('disabled'); diff --git a/app/client/widgets/FieldBuilder.ts b/app/client/widgets/FieldBuilder.ts index 6525fe3c..417143a5 100644 --- a/app/client/widgets/FieldBuilder.ts +++ b/app/client/widgets/FieldBuilder.ts @@ -522,7 +522,7 @@ export class FieldBuilder extends Disposable { editRow: DataRowModel, refElem: Element, editValue?: string, - onSave?: (formula: string) => Promise, + onSave?: (column: ColumnRec, formula: string) => Promise, onCancel?: () => void) { const editorHolder = openFormulaEditor({ gristDoc: this.gristDoc, diff --git a/app/client/widgets/FieldEditor.ts b/app/client/widgets/FieldEditor.ts index c75b10d1..5f22b81f 100644 --- a/app/client/widgets/FieldEditor.ts +++ b/app/client/widgets/FieldEditor.ts @@ -383,7 +383,7 @@ export function openFormulaEditor(options: { // Element over which to position the editor. refElem: Element, editValue?: string, - onSave?: (formula: string) => Promise, + onSave?: (column: ColumnRec, formula: string) => Promise, onCancel?: () => void, // Called after editor is created to set up editor cleanup (e.g. saving on click-away). setupCleanup: ( @@ -401,7 +401,7 @@ export function openFormulaEditor(options: { const saveEdit = asyncOnce(async () => { const formula = editor.getCellValue(); if (options.onSave) { - await options.onSave(formula as string); + await options.onSave(column, formula as string); } else if (formula !== column.formula.peek()) { await column.updateColValues({formula}); }