(core) Change how formula columns can be converted to data.

Summary: - No longer convert data columns to formula by typing a leading "=". Instead, show a tooltip with a link to click if the conversion was intended. - No longer convert a formula column to data by deleting its formula. Leave the column empty instead. - Offer the option "Convert formula to data" in column menu for formulas. - Offer the option to "Clear column" - If a subset of rows is shown, offer "Clear values" and "Clear entire column". - Add logic to detect when a view shows a subset of all rows. - Factor out showTooltip() from showTransientTooltip(). - Add a bunch of test cases to cover various combinations (there are small variations in options depending on whether all rows are shown, on whether multiple columns are selected, and whether columns include data columns). Test Plan: Added a bunch of test cases. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D2746
2026-03-02 04:09:24 +00:00 · 2021-03-05 10:17:07 -05:00
parent 8a1e803316
commit 48e90c4998
16 changed files with 377 additions and 156 deletions
--- a/app/client/ui/GridViewMenus.ts
+++ b/app/client/ui/GridViewMenus.ts
@@ -1,4 +1,5 @@
 import {allCommands} from 'app/client/components/commands';
+import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec';
 import {testId, vars} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {menuDivider, menuItem, menuItemCmd} from 'app/client/ui2018/menus';
@@ -66,119 +67,123 @@ export function RowContextMenu({ disableInsert, disableDelete, isViewSorted }: I
  return result;
 }

-interface IColumnContextMenu {
-  disableModify: boolean;
+interface IMultiColumnContextMenu {
+  // For multiple selection, true/false means the value applies to all columns, 'mixed' means it's
+  // true for some columns, but not all.
+  numColumns: number;
+  disableModify: boolean|'mixed';  // If the columns are read-only.
+  isReadonly: boolean;
+  isFiltered: boolean;            // If this view shows a proper subset of all rows in the table.
+  isFormula: boolean|'mixed';
+}
+
+interface IColumnContextMenu extends IMultiColumnContextMenu {
  filterOpenFunc: () => void;
-  useNewUI: boolean;
  sortSpec: number[];
  colId: number;
-  isReadonly: boolean;
+}
+
+export function calcFieldsCondition(fields: ViewFieldRec[], condition: (f: ViewFieldRec) => boolean): boolean|"mixed" {
+  return fields.every(condition) ? true : (fields.some(condition) ? "mixed" : false);
 }

 export function ColumnContextMenu(options: IColumnContextMenu) {
-  const { disableModify, filterOpenFunc, useNewUI, colId, sortSpec, isReadonly } = options;
+  const { disableModify, filterOpenFunc, colId, sortSpec, isReadonly } = options;

-  if (useNewUI) {
+  const disableForReadonlyColumn = dom.cls('disabled', Boolean(disableModify) || isReadonly);
+  const disableForReadonlyView = dom.cls('disabled', isReadonly);

-    const addToSortLabel = getAddToSortLabel(sortSpec, colId);
-    return [
-      menuItemCmd(allCommands.fieldTabOpen, 'Column Options'),
-      menuItem(filterOpenFunc, 'Filter Data'),
-      menuDivider({style: 'margin-bottom: 0;'}),
+  const addToSortLabel = getAddToSortLabel(sortSpec, colId);
+  return [
+    menuItemCmd(allCommands.fieldTabOpen, 'Column Options'),
+    menuItem(filterOpenFunc, 'Filter Data'),
+    menuDivider({style: 'margin-bottom: 0;'}),
+    cssRowMenuItem(
+      customMenuItem(
+        allCommands.sortAsc.run,
+        dom('span', 'Sort', {style: 'flex: 1  0 auto; margin-right: 8px;'},
+            testId('sort-label')),
+        icon('Sort', dom.style('transform', 'scaley(-1)')),
+        'A-Z',
+        dom.style('flex', ''),
+        cssCustomMenuItem.cls('-selected', isEqual(sortSpec, [colId])),
+        testId('sort-asc'),
+      ),
+      customMenuItem(
+        allCommands.sortDesc.run,
+        icon('Sort'),
+        'Z-A',
+        cssCustomMenuItem.cls('-selected', isEqual(sortSpec, [-colId])),
+        testId('sort-dsc'),
+      ),
+      testId('sort'),
+    ),
+    menuDivider({style: 'margin-bottom: 0; margin-top: 0;'}),
+    addToSortLabel ? [
      cssRowMenuItem(
        customMenuItem(
-          allCommands.sortAsc.run,
-          dom('span', 'Sort', {style: 'flex: 1  0 auto; margin-right: 8px;'},
-              testId('sort-label')),
+          allCommands.addSortAsc.run,
+          cssRowMenuLabel(addToSortLabel, testId('add-to-sort-label')),
          icon('Sort', dom.style('transform', 'scaley(-1)')),
          'A-Z',
-          dom.style('flex', ''),
-          cssCustomMenuItem.cls('-selected', isEqual(sortSpec, [colId])),
-          testId('sort-asc'),
+          cssCustomMenuItem.cls('-selected', sortSpec.includes(colId)),
+          testId('add-to-sort-asc'),
        ),
        customMenuItem(
-          allCommands.sortDesc.run,
+          allCommands.addSortDesc.run,
          icon('Sort'),
          'Z-A',
-          cssCustomMenuItem.cls('-selected', isEqual(sortSpec, [-colId])),
-          testId('sort-dsc'),
+          cssCustomMenuItem.cls('-selected', sortSpec.includes(-colId)),
+          testId('add-to-sort-dsc'),
        ),
-        testId('sort'),
+        testId('add-to-sort'),
      ),
-      menuDivider({style: 'margin-bottom: 0; margin-top: 0;'}),
-      addToSortLabel ? [
-        cssRowMenuItem(
-          customMenuItem(
-            allCommands.addSortAsc.run,
-            cssRowMenuLabel(addToSortLabel, testId('add-to-sort-label')),
-            icon('Sort', dom.style('transform', 'scaley(-1)')),
-            'A-Z',
-            cssCustomMenuItem.cls('-selected', sortSpec.includes(colId)),
-            testId('add-to-sort-asc'),
-          ),
-          customMenuItem(
-            allCommands.addSortDesc.run,
-            icon('Sort'),
-            'Z-A',
-            cssCustomMenuItem.cls('-selected', sortSpec.includes(-colId)),
-            testId('add-to-sort-dsc'),
-          ),
-          testId('add-to-sort'),
-        ),
-        menuDivider({style: 'margin-top: 0;'}),
-      ] : null,
-      menuItemCmd(allCommands.renameField, 'Rename column',
-        dom.cls('disabled', disableModify || isReadonly)),
-      menuItemCmd(allCommands.hideField, 'Hide column',
-        dom.cls('disabled', isReadonly)),
-      menuItemCmd(allCommands.deleteFields, 'Delete column',
-        dom.cls('disabled', disableModify || isReadonly)),
-      testId('column-menu'),
+      menuDivider({style: 'margin-top: 0;'}),
+    ] : null,
+    menuItemCmd(allCommands.renameField, 'Rename column', disableForReadonlyColumn),
+    menuItemCmd(allCommands.hideField, 'Hide column', disableForReadonlyView),

-      // TODO: this piece should be removed after adding the new way to add column
-      menuDivider(),
-      menuItemCmd(allCommands.insertFieldBefore, 'Insert column to the left',
-        dom.cls('disabled', isReadonly)),
-      menuItemCmd(allCommands.insertFieldAfter, 'Insert column to the right',
-        dom.cls('disabled', isReadonly)),
-    ];
-  } else {
-    return [
-      menuItemCmd(allCommands.fieldTabOpen, 'FieldOptions'),
-      menuDivider(),
-      menuItemCmd(allCommands.insertFieldBefore, 'Insert column to the left'),
-      menuItemCmd(allCommands.insertFieldAfter, 'Insert column to the right'),
-      menuDivider(),
-      menuItemCmd(allCommands.renameField, 'Rename column',
-        dom.cls('disabled', disableModify)),
-      menuItemCmd(allCommands.hideField, 'Hide column'),
-      menuItemCmd(allCommands.deleteFields, 'Delete column',
-        dom.cls('disabled', disableModify)),
-      menuItem(filterOpenFunc, 'Filter'),
-      menuDivider(),
-      menuItemCmd(allCommands.sortAsc, 'Sort ascending'),
-      menuItemCmd(allCommands.sortDesc, 'Sort descending'),
-      menuItemCmd(allCommands.addSortAsc, 'Add to sort as ascending'),
-      menuItemCmd(allCommands.addSortDesc, 'Add to sort as descending'),
-    ];
-  }
-}
-
-
-interface IMultiColumnContextMenu {
-  isReadonly: boolean;
-}
-
-export function MultiColumnMenu(options: IMultiColumnContextMenu) {
-  const {isReadonly} = options;
-  return [
-    menuItemCmd(allCommands.insertFieldBefore, 'Insert column to the left',
-      dom.cls('disabled', isReadonly)),
-    menuItemCmd(allCommands.insertFieldAfter, 'Insert column to the right',
-      dom.cls('disabled', isReadonly)),
    menuDivider(),
-    menuItemCmd(allCommands.deleteFields, 'Delete columns',
-      dom.cls('disabled', isReadonly)),
+    MultiColumnMenu(options),
+    testId('column-menu'),
+  ];
+}
+
+/**
+ * Note about available options. There is a difference between clearing values (writing empty
+ * string, which makes cells blank, including Numeric cells) and converting a column to an empty
+ * column (i.e. column with empty formula; in this case a Numeric column becomes all 0s today).
+ *
+ * We offer both options if data columns are selected. If only formulas, only the second option
+ * makes sense.
+ */
+export function MultiColumnMenu(options: IMultiColumnContextMenu) {
+  const disableForReadonlyColumn = dom.cls('disabled', Boolean(options.disableModify) || options.isReadonly);
+  const disableForReadonlyView = dom.cls('disabled', options.isReadonly);
+  const num: number = options.numColumns;
+  const nameClearColumns = options.isFiltered ?
+    (num > 1 ? `Clear ${num} entire columns` : 'Clear entire column') :
+    (num > 1 ? `Clear ${num} columns` : 'Clear column');
+  const nameDeleteColumns = num > 1 ? `Delete ${num} columns` : 'Delete column';
+  return [
+    // TODO This should be made to work too for multiple columns.
+    // menuItemCmd(allCommands.hideField, 'Hide column', disableForReadonlyView),
+
+    // Offered only when selection includes formula columns, and converts only those.
+    (options.isFormula ?
+      menuItemCmd(allCommands.convertFormulasToData, 'Convert formula to data',
+        disableForReadonlyColumn) : null),
+
+    // With data columns selected, offer an additional option to clear out selected cells.
+    (options.isFormula !== true ?
+      menuItemCmd(allCommands.clearValues, 'Clear values', disableForReadonlyColumn) : null),
+
+    menuItemCmd(allCommands.clearColumns, nameClearColumns, disableForReadonlyColumn),
+    menuItemCmd(allCommands.deleteFields, nameDeleteColumns, disableForReadonlyColumn),
+
+    menuDivider(),
+    menuItemCmd(allCommands.insertFieldBefore, 'Insert column to the left', disableForReadonlyView),
+    menuItemCmd(allCommands.insertFieldAfter, 'Insert column to the right', disableForReadonlyView),
  ];
 }

--- a/app/client/ui/tooltips.ts
+++ b/app/client/ui/tooltips.ts
@@ -7,35 +7,68 @@

 import {prepareForTransition} from 'app/client/ui/transitions';
 import {testId} from 'app/client/ui2018/cssVars';
-import {dom, styled} from 'grainjs';
+import {dom, DomContents, styled} from 'grainjs';
 import Popper from 'popper.js';

-interface ITipOptions {
+export interface ITipOptions {
  // Where to place the tooltip relative to the reference element. Defaults to 'top'.
  // See https://popper.js.org/docs/v1/#popperplacements--codeenumcode
  placement?: Popper.Placement;

-  // When to remove the transient tooltip. Defaults to 2000ms.
-  timeoutMs?: number;
-
  // When set, a tooltip will replace any previous tooltip with the same key.
  key?: string;
 }

-// Map of open tooltips, mapping the key (from ITipOptions) to the cleanup function that removes
-// the tooltip.
-const openTooltips = new Map<string, () => void>();
+export interface ITransientTipOptions extends ITipOptions {
+  // When to remove the transient tooltip. Defaults to 2000ms.
+  timeoutMs?: number;
+}

-export function showTransientTooltip(refElem: Element, text: string, options: ITipOptions = {}) {
+export interface ITooltipControl {
+  close(): void;
+}
+
+// Map of open tooltips, mapping the key (from ITipOptions) to ITooltipControl that allows
+// removing the tooltip.
+const openTooltips = new Map<string, ITooltipControl>();
+
+/**
+ * Show tipContent briefly (2s by default), in a tooltip next to refElem (on top of it, by default).
+ * See also ITipOptions.
+ */
+export function showTransientTooltip(refElem: Element, tipContent: DomContents, options: ITransientTipOptions = {}) {
+  const ctl = showTooltip(refElem, () => tipContent, options);
+  const origClose = ctl.close;
+  ctl.close = () => { clearTimeout(timer); origClose(); };
+
+  const timer = setTimeout(ctl.close, options.timeoutMs || 2000);
+}
+
+/**
+ * Show the return value of tipContent(ctl) in a tooltip next to refElem (on top of it, by default).
+ * Returns ctl. In both places, ctl is an object with a close() method, which closes the tooltip.
+ * See also ITipOptions.
+ */
+export function showTooltip(
+  refElem: Element, tipContent: (ctl: ITooltipControl) => DomContents, options: ITipOptions = {}
+): ITooltipControl {
  const placement: Popper.Placement = options.placement || 'top';
-  const timeoutMs: number = options.timeoutMs || 2000;
  const key = options.key;

  // If we had a previous tooltip with the same key, clean it up.
-  if (key) { openTooltips.get(key)?.(); }
+  if (key) { openTooltips.get(key)?.close(); }
+
+  // Cleanup involves destroying the Popper instance, removing the element, etc.
+  function close() {
+    popper.destroy();
+    dom.domDispose(content);
+    content.remove();
+    if (key) { openTooltips.delete(key); }
+  }
+  const ctl: ITooltipControl = {close};

  // Add the content element.
-  const content = cssTooltip({role: 'tooltip'}, text, testId(`transient-tooltip`));
+  const content = cssTooltip({role: 'tooltip'}, tipContent(ctl), testId(`transient-tooltip`));
  document.body.appendChild(content);

  // Create a popper for positioning the tooltip content relative to refElem.
@@ -49,16 +82,8 @@ export function showTransientTooltip(refElem: Element, text: string, options: IT
  prepareForTransition(content, () => { content.style.opacity = '0'; });
  content.style.opacity = '';

-  // Cleanup involves destroying the Popper instance, removing the element, etc.
-  function cleanup() {
-    popper.destroy();
-    dom.domDispose(content);
-    content.remove();
-    if (key) { openTooltips.delete(key); }
-    clearTimeout(timer);
-  }
-  const timer = setTimeout(cleanup, timeoutMs);
-  if (key) { openTooltips.set(key, cleanup); }
+  if (key) { openTooltips.set(key, ctl); }
+  return ctl;
 }


@@ -75,6 +100,6 @@ const cssTooltip = styled('div', `
  font-size: 10pt;
  padding: 8px 16px;
  margin: 4px;
-  opacity: 0.65;
+  opacity: 0.75;
  transition: opacity 0.2s;
 `);