mirror of
				https://github.com/gristlabs/grist-core.git
				synced 2025-06-13 20:53:59 +00:00 
			
		
		
		
	(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
This commit is contained in:
		
							parent
							
								
									8aa6ddba0b
								
							
						
					
					
						commit
						ae6342810d
					
				@ -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) {
 | 
			
		||||
 | 
			
		||||
@ -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' */ [
 | 
			
		||||
 | 
			
		||||
@ -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;
 | 
			
		||||
 | 
			
		||||
@ -369,8 +369,9 @@ export function selectTitle(label: BindableValue<string>, iconName?: BindableVal
 | 
			
		||||
export function selectOption(
 | 
			
		||||
  action: (item: HTMLElement) => void,
 | 
			
		||||
  label: BindableValue<string>,
 | 
			
		||||
  iconName?: BindableValue<IconName>) {
 | 
			
		||||
  return menuItem(action, selectTitle(label, iconName));
 | 
			
		||||
  iconName?: BindableValue<IconName>,
 | 
			
		||||
  ...args: IDomArgs<HTMLElement>) {
 | 
			
		||||
  return menuItem(action, selectTitle(label, iconName), ...args);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
export const menuSubHeader = styled('div', `
 | 
			
		||||
 | 
			
		||||
@ -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;
 | 
			
		||||
  }
 | 
			
		||||
`);
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@ -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, () => {
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user