From ae6342810d5d599ae430794c106455539239d880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 25 Jan 2022 10:38:21 +0100 Subject: [PATCH] (core) Summary columns improvemnt. Summary: Improving user experience on summary columns. - Showing 'not-allowed' cursor on sections/menus that can't be changed - Disabling menu options and buttons in the column behavior section that converts a formula column to a data column - Showing nicer error message about converting formula to a data column. Test Plan: manual tests, no behavior change Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3222 --- app/client/models/errors.ts | 6 +++++- app/client/ui/FieldConfig.ts | 26 ++++++++++++++++++-------- app/client/ui/RightPanel.ts | 6 ++++++ app/client/ui2018/menus.ts | 5 +++-- app/client/widgets/ChoiceListEntry.ts | 6 ++++-- app/client/widgets/FieldBuilder.ts | 3 ++- 6 files changed, 38 insertions(+), 14 deletions(-) diff --git a/app/client/models/errors.ts b/app/client/models/errors.ts index 4b6b4861..29f967bf 100644 --- a/app/client/models/errors.ts +++ b/app/client/models/errors.ts @@ -120,7 +120,11 @@ export function reportError(err: Error|string): void { } else if (code === 'AUTH_NO_EDIT' || code === 'ACL_DENY') { // Show the error as a message _notifier.createUserMessage(err.message, {key: code, memos: details?.memos}); - } else { + } else if (message.match(/\[Sandbox\].*between formula and data/)) { + // Show nicer error message for summary tables. + _notifier.createUserMessage("Summary tables can only contain formula columns.", + {key: 'summary', actions: ['ask-for-help']}); + } else { // If we don't recognize it, consider it an application error (bug) that the user should be // able to report. if (details?.userError) { diff --git a/app/client/ui/FieldConfig.ts b/app/client/ui/FieldConfig.ts index a6a37fdf..c965629d 100644 --- a/app/client/ui/FieldConfig.ts +++ b/app/client/ui/FieldConfig.ts @@ -2,7 +2,7 @@ import {CursorPos} from 'app/client/components/Cursor'; import {GristDoc} from 'app/client/components/GristDoc'; import {ColumnRec} from 'app/client/models/entities/ColumnRec'; import {buildHighlightedCode, cssCodeBlock} from 'app/client/ui/CodeHighlight'; -import {cssEmptySeparator, cssLabel, cssRow} from 'app/client/ui/RightPanel'; +import {cssBlockedCursor, cssEmptySeparator, cssLabel, cssRow} from 'app/client/ui/RightPanel'; import {buildFormulaTriggers} from 'app/client/ui/TriggerFormulas'; import {textButton} from 'app/client/ui2018/buttons'; import {colors, testId} from 'app/client/ui2018/cssVars'; @@ -10,8 +10,8 @@ import {textInput} from 'app/client/ui2018/editableLabel'; import {cssIconButton, icon} from 'app/client/ui2018/icons'; import {selectMenu, selectOption, selectTitle} from 'app/client/ui2018/menus'; import {sanitizeIdent} from 'app/common/gutil'; -import {bundleChanges, Computed, dom, DomContents, DomElementArg, fromKo, MultiHolder, Observable, - styled, subscribe} from 'grainjs'; +import {bundleChanges, Computed, dom, DomContents, DomElementArg, fromKo, MultiHolder, + Observable, styled, subscribe} from 'grainjs'; import * as ko from 'knockout'; import debounce = require('lodash/debounce'); import {IconName} from 'app/client/ui2018/IconList'; @@ -24,6 +24,7 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec, curso '$' + (edited ? sanitizeIdent(edited) : use(origColumn.colId))); 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 // will change its value. @@ -39,6 +40,7 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec, curso return [ cssLabel('COLUMN LABEL AND ID'), cssRow( + dom.cls(cssBlockedCursor.className, origColumn.disableModify), cssColLabelBlock( editor = textInput(fromKo(origColumn.label), async val => { await origColumn.label.saveOnly(val); editedLabel.set(''); }, @@ -64,6 +66,8 @@ export function buildNameConfig(owner: MultiHolder, origColumn: ColumnRec, curso ), ) ), + dom.maybe(isSummaryTable, + () => cssRow('Column options are limited in summary tables.')) ]; } @@ -86,6 +90,9 @@ export function buildFormulaConfig( // Intermediate state - user wants to specify formula, but haven't done yet const maybeTrigger = Observable.create(owner, false); + // If this column belongs to a summary table. + const isSummaryTable = Computed.create(owner, use => Boolean(use(use(origColumn.table).summarySourceTable))); + // Column behaviour. There are 3 types of behaviors: // - empty: isFormula and formula == '' // - formula: isFormula and formula != '' @@ -127,6 +134,7 @@ export function buildFormulaConfig( // this element focusable and will steal focus from clipboard. This in turn, // will not dispose the formula editor when menu is clicked. (el) => el.removeAttribute("tabindex"), + dom.cls(cssBlockedCursor.className, origColumn.disableModify), dom.cls("disabled", origColumn.disableModify)), ); @@ -158,7 +166,9 @@ export function buildFormulaConfig( const convertToData = () => gristDoc.convertIsFormula([origColumn.id.peek()], {toFormula: false, noRecalc: true}); const convertToDataOption = () => selectOption( convertToData, - 'Convert column to data', 'Database'); + 'Convert column to data', 'Database', + dom.cls('disabled', isSummaryTable) + ); // Clears the column const clearAndResetOption = () => selectOption( @@ -232,7 +242,7 @@ export function buildFormulaConfig( cssLabel('COLUMN BEHAVIOR'), ...(type === "empty" ? [ menu(behaviourLabel(), [ - convertToDataOption() + convertToDataOption(), ]), cssEmptySeparator(), cssRow(textButton( @@ -244,13 +254,13 @@ export function buildFormulaConfig( cssRow(textButton( "Set trigger formula", dom.on("click", setTrigger), - dom.prop("disabled", origColumn.disableModify), + dom.prop("disabled", use => use(isSummaryTable) || use(origColumn.disableModify)), testId("field-set-trigger") )), cssRow(textButton( "Make into data column", dom.on("click", convertToData), - dom.prop("disabled", origColumn.disableModify), + dom.prop("disabled", use => use(isSummaryTable) || use(origColumn.disableModify)), testId("field-set-data") )) ] : type === "formula" ? [ @@ -264,7 +274,7 @@ export function buildFormulaConfig( "Convert to trigger formula", dom.on("click", convertFormulaToTrigger), dom.hide(maybeFormula), - dom.prop("disabled", origColumn.disableModify), + dom.prop("disabled", use => use(isSummaryTable) || use(origColumn.disableModify)), testId("field-set-trigger") )) ] : /* type == 'data' */ [ diff --git a/app/client/ui/RightPanel.ts b/app/client/ui/RightPanel.ts index 855553a8..9ef4af76 100644 --- a/app/client/ui/RightPanel.ts +++ b/app/client/ui/RightPanel.ts @@ -556,6 +556,12 @@ export const cssRow = styled('div', ` } `); +export const cssBlockedCursor = styled('span', ` + &, & * { + cursor: not-allowed !important; + } +`); + export const cssButtonRow = styled(cssRow, ` margin-left: 0; margin-right: 0; diff --git a/app/client/ui2018/menus.ts b/app/client/ui2018/menus.ts index 6c77229e..76a6c863 100644 --- a/app/client/ui2018/menus.ts +++ b/app/client/ui2018/menus.ts @@ -369,8 +369,9 @@ export function selectTitle(label: BindableValue, iconName?: BindableVal export function selectOption( action: (item: HTMLElement) => void, label: BindableValue, - iconName?: BindableValue) { - return menuItem(action, selectTitle(label, iconName)); + iconName?: BindableValue, + ...args: IDomArgs) { + return menuItem(action, selectTitle(label, iconName), ...args); } export const menuSubHeader = styled('div', ` diff --git a/app/client/widgets/ChoiceListEntry.ts b/app/client/widgets/ChoiceListEntry.ts index fcfbac8d..12591599 100644 --- a/app/client/widgets/ChoiceListEntry.ts +++ b/app/client/widgets/ChoiceListEntry.ts @@ -1,4 +1,5 @@ import {IToken, TokenField} from 'app/client/lib/TokenField'; +import {cssBlockedCursor} from 'app/client/ui/RightPanel'; import {basicButton, primaryButton} from 'app/client/ui2018/buttons'; import {colorButton} from 'app/client/ui2018/ColorSelect'; import {colors, testId} from 'app/client/ui2018/cssVars'; @@ -156,6 +157,7 @@ export class ChoiceListEntry extends Disposable { return cssVerticalFlex( cssListBoxInactive( + dom.cls(cssBlockedCursor.className, this._disabled), dom.autoDispose(someValues), dom.maybe(use => use(someValues).length === 0, () => row('No choices configured') @@ -372,11 +374,11 @@ const cssListBoxInactive = styled(cssListBox, ` cursor: pointer; border: 1px solid ${colors.darkGrey}; - &:hover { + &:hover:not(&-disabled) { border: 1px solid ${colors.hover}; } &-disabled { - cursor: default; + opacity: 0.6; } `); diff --git a/app/client/widgets/FieldBuilder.ts b/app/client/widgets/FieldBuilder.ts index 417143a5..45c65f73 100644 --- a/app/client/widgets/FieldBuilder.ts +++ b/app/client/widgets/FieldBuilder.ts @@ -14,7 +14,7 @@ import { DataRowModel } from 'app/client/models/DataRowModel'; import { ColumnRec, DocModel, ViewFieldRec } from 'app/client/models/DocModel'; import { SaveableObjObservable, setSaveValue } from 'app/client/models/modelUtil'; import { FieldSettingsMenu } from 'app/client/ui/FieldMenus'; -import { cssLabel, cssRow } from 'app/client/ui/RightPanel'; +import { cssBlockedCursor, cssLabel, cssRow } from 'app/client/ui/RightPanel'; import { buttonSelect } from 'app/client/ui2018/buttonSelect'; import { IOptionFull, menu, select } from 'app/client/ui2018/menus'; import { DiffBox } from 'app/client/widgets/DiffBox'; @@ -228,6 +228,7 @@ export class FieldBuilder extends Disposable { }), testId('type-select'), grainjsDom.cls('tour-type-selector'), + grainjsDom.cls(cssBlockedCursor.className, this.origColumn.disableModifyBase) ), grainjsDom.maybe((use) => use(this._isRef) && !use(this._isTransformingType), () => this._buildRefTableSelect()), grainjsDom.maybe(this._isTransformingType, () => {