(core) Rename popup for group by columns

Summary:
Showing rename popup on group by column with disabled label section. It only allows to set description.

Unrelated:
- Fixing HostedMetadataManager test

Test Plan: Updated tests

Reviewers: Spoffy

Reviewed By: Spoffy

Subscribers: Spoffy

Differential Revision: https://phab.getgrist.com/D4371

app/client/components/GridView.js

@@ -1270,12 +1270,12 @@ GridView.prototype.buildDom = function() {
             kd.style('minWidth', '100%'),
             kd.style('borderLeftWidth', v.borderWidthPx),
             kd.foreach(v.viewFields(), field => {
+              const canRename = ko.pureComputed(() => !field.column().disableEditData());
               const isEditingLabel = koUtil.withKoUtils(ko.pureComputed({
                 read: () => {
                   const goodIndex = () => editIndex() === field._index();
                   const isReadonly = () => this.isReadonly || self.isPreview;
-                  const isSummary = () => Boolean(field.column().disableEditData());
+                  return goodIndex() && !isReadonly();
-                  return goodIndex() && !isReadonly() && !isSummary();
                 },
                 write: val => {
                   if (val) {
@@ -1306,6 +1306,7 @@ GridView.prototype.buildDom = function() {
 
               return dom(
                 'div.column_name.field',
+                dom.autoDispose(canRename),
                 dom.autoDispose(headerTextColor),
                 dom.autoDispose(headerFillColor),
                 dom.autoDispose(headerFontBold),
@@ -1355,7 +1356,8 @@ GridView.prototype.buildDom = function() {
                   buildRenameColumn({
                     field,
                     isEditing: isEditingLabel,
-                    optCommands: renameCommands
+                    optCommands: renameCommands,
+                    canRename,
                   }),
                 ),
                 self._showTooltipOnHover(field, isTooltip),

app/client/ui/ColumnTitle.ts

@@ -5,26 +5,43 @@ import {FocusLayer} from 'app/client/lib/FocusLayer';
 import {makeT} from 'app/client/lib/localization';
 import {setTestState} from 'app/client/lib/testState';
 import {ViewFieldRec} from 'app/client/models/DocModel';
+import {LIMITED_COLUMN_OPTIONS} from 'app/client/ui/FieldConfig';
 import {autoGrow} from 'app/client/ui/forms';
-import {showTransientTooltip} from 'app/client/ui/tooltips';
+import {cssInput, cssLabel, cssRenamePopup, cssTextArea} from 'app/client/ui/RenamePopupStyles';
+import {hoverTooltip, showTransientTooltip} from 'app/client/ui/tooltips';
 import {basicButton, primaryButton, textButton} from 'app/client/ui2018/buttons';
 import {theme, vars} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {menuCssClass} from 'app/client/ui2018/menus';
-
 import {Computed, dom, makeTestId, Observable, styled} from 'grainjs';
 import * as ko from 'knockout';
 import {IOpenController, PopupControl, setPopupToCreateDom} from 'popweasel';
-import { cssInput, cssLabel, cssRenamePopup, cssTextArea } from 'app/client/ui/RenamePopupStyles';
 
 
 const testId = makeTestId('test-column-title-');
 const t = makeT('ColumnTitle');
 
 interface IColumnTitleOptions {
+  /**
+   * The field to rename.
+   */
   field: ViewFieldRec;
+  /**
+   * An observable that triggers the popup to open when set to true.
+   */
   isEditing: ko.Computed<boolean>;
+  /**
+   * Optional commands to bind when the popup is open.
+   */
   optCommands?: any;
+  /**
+   * Optional computed or boolean to determine if the column can be renamed. Defaults to true.
+   */
+  canRename?: ko.Computed<boolean>|boolean;
+  /**
+   * Optional computed or boolean to determine if the description can be changed. Defaults to true.
+   */
+  canChangeDesc?: ko.Computed<boolean>|boolean;
 }
 
 export function buildRenameColumn(options: IColumnTitleOptions) {
@@ -49,9 +66,8 @@ export function buildRenameColumn(options: IColumnTitleOptions) {
   };
 }
 
-function buildColumnRenamePopup(
+function buildColumnRenamePopup(ctrl: IOpenController, options: IColumnTitleOptions) {
-  ctrl: IOpenController, {field, isEditing, optCommands}: IColumnTitleOptions
+  const {field, isEditing, optCommands} = options;
-) {
   // Store temporary values for the label and description.
   const editedLabel = Observable.create(ctrl, field.displayLabel.peek());
   const editedDesc = Observable.create(ctrl, field.description.peek());
@@ -159,20 +175,30 @@ function buildColumnRenamePopup(
 
   // Create this group and attach it to the popup and both inputs.
   const commandGroup = commands.createGroup({...optCommands, ...myCommands}, ctrl, true);
+  ctrl.autoDispose(commandGroup);
 
   // We will still focus from other elements and restore it on either the label or description input.
   let lastFocus: HTMLElement | undefined;
   const rememberFocus = (el: HTMLElement) => dom.on('focus', () => lastFocus = el);
   const restoreFocus = (el: HTMLElement) => dom.on('focus', () => lastFocus?.focus());
 
-  const showDesc = Observable.create(null, Boolean(field.description.peek() !== ''));
+  const showDesc = Observable.create(ctrl, Boolean(field.description.peek() !== ''));
+
+  const defaultTrue = (val: boolean|ko.Computed<boolean>|undefined) => {
+    return val === undefined ? true : val;
+  };
+  const toComputed = (val: boolean|ko.Computed<boolean>) =>
+    typeof val === 'boolean' ? Computed.create(ctrl, () => val) : Computed.create(ctrl, use => use(val));
+
+  const not = (val: Observable<boolean>) => Computed.create(ctrl, (use) => !use(val));
+
+  const canRename = toComputed(defaultTrue(options.canRename));
+  const canChangeDesc = toComputed(defaultTrue(options.canChangeDesc));
 
   let labelInput: HTMLInputElement | undefined;
   let descInput: HTMLTextAreaElement | undefined;
   return cssRenamePopup(
     dom.onDispose(onClose),
-    dom.autoDispose(commandGroup),
-    dom.autoDispose(showDesc),
     testId('popup'),
     dom.cls(menuCssClass),
     cssLabel(t("Column label")),
@@ -184,6 +210,9 @@ function buildColumnRenamePopup(
         testId('label'),
         commandGroup.attach(),
         rememberFocus,
+        hoverTooltip(LIMITED_COLUMN_OPTIONS, {hidden: canRename}),
+        dom.boolAttr('disabled', not(canRename)),
+        dom.style('pointer-events', 'all')
       ),
       cssColId(
         t("COLUMN ID: "),
@@ -219,6 +248,7 @@ function buildColumnRenamePopup(
         commandGroup.attach(),
         rememberFocus,
         autoGrow(editedDesc),
+        dom.boolAttr('disabled', not(canChangeDesc)),
       ),
     ]),
     dom.onKeyDown({
@@ -251,8 +281,13 @@ function buildColumnRenamePopup(
     // After showing the popup, focus the label input and select it's content.
     elem => { setTimeout(() => {
       if (ctrl.isDisposed()) { return; }
+      if (canRename.get()) {
         labelInput?.focus();
         labelInput?.select();
+      } else if (canChangeDesc.get()) {
+        descInput?.focus();
+        descInput?.select();
+      }
     }, 0); },
     // Create a FocusLayer to keep focus in this popup while it's active, by default when focus is stolen
     // by someone else, we will bring back it to the label element. Clicking anywhere outside the popup

app/client/ui/FieldConfig.ts

@@ -20,6 +20,8 @@ import * as ko from 'knockout';
 
 const t = makeT('FieldConfig');
 
+export const LIMITED_COLUMN_OPTIONS = t("Column options are limited in summary tables.");
+
 export function buildNameConfig(
   owner: MultiHolder,
   origColumn: ColumnRec,
@@ -86,7 +88,7 @@ export function buildNameConfig(
       )
     ),
     dom.maybe(isSummaryTable,
-      () => cssRow(t("Column options are limited in summary tables.")))
+      () => cssRow(LIMITED_COLUMN_OPTIONS))
   ];
 }
 

app/client/ui/tooltips.ts

@@ -12,7 +12,7 @@ import {testId, theme, vars} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {makeLinks} from 'app/client/ui2018/links';
 import {menuCssClass} from 'app/client/ui2018/menus';
-import {BindableValue, dom, DomContents, DomElementArg, DomElementMethod, styled} from 'grainjs';
+import {BindableValue, dom, DomContents, DomElementArg, DomElementMethod, Observable, styled} from 'grainjs';
 import Popper from 'popper.js';
 import {cssMenu, cssMenuItem, defaultMenuOptions, IPopupOptions, setPopupToCreateDom} from 'popweasel';
 import merge = require('lodash/merge');
@@ -79,6 +79,11 @@ export interface IHoverTipOptions extends ITransientTipOptions {
 
   /** Whether to show the tooltip only when the ref element overflows horizontally. */
   overflowOnly?: boolean;
+
+  /**
+   * If set tooltip won't be shown on hover. Default to false.
+   */
+  hidden?: Observable<boolean>;
 }
 
 export type ITooltipContent = ITooltipContentFunc | DomContents;
@@ -250,6 +255,7 @@ export function setHoverTooltip(
     }
   }
   function open() {
+    if (options.hidden?.get()) { return; }
     clearTimer();
     tipControl = showTooltip(refElem, ctl => tipContentFunc({...ctl, close}), options);
     const tipDom = tipControl.getDom();
@@ -260,6 +266,7 @@ export function setHoverTooltip(
     if (timeoutMs) { resetTimer(close, timeoutMs); }
   }
   function close() {
+    if (options.hidden?.get()) { return; }
     clearTimer();
     tipControl?.close();
     tipControl = undefined;
@@ -267,6 +274,8 @@ export function setHoverTooltip(
 
   // We simulate hover effect by handling mouseenter/mouseleave.
   dom.onElem(refElem, 'mouseenter', () => {
+    if (options.hidden?.get()) { return; }
+
     if (overflowOnly && (refElem as HTMLElement).offsetWidth >= refElem.scrollWidth) {
       return;
     }

app/common/gutil.ts

@@ -999,7 +999,11 @@ export function useBindable<T>(use: UseCBOwner, obs: BindableValue<T>): T {
 /**
  * Useful helper for simple boolean negation.
  */
-export const not = (obs: Observable<any>|IKnockoutReadObservable<any>) => (use: UseCBOwner) => !use(obs);
+export const not = (obs: Observable<any>|IKnockoutReadObservable<any>|boolean|undefined|null) => (use: UseCBOwner) =>  {
+  if (typeof obs === 'boolean') { return !obs; }
+  if (obs === null || obs === undefined) { return true; }
+  return !use(obs);
+};
 
 /**
  * Get a set of up to `count` distinct values of `values`.

app/server/lib/HostedStorageManager.ts

@@ -151,7 +151,7 @@ export class HostedStorageManager implements IDocStorageManager {
     if (!externalStoreDoc) { this._disableS3 = true; }
     const secondsBeforePush = options.secondsBeforePush;
     if (options.pushDocUpdateTimes) {
-      this._metadataManager = new HostedMetadataManager(callbacks.setDocsMetadata);
+      this._metadataManager = new HostedMetadataManager(callbacks.setDocsMetadata.bind(callbacks));
     }
     this._uploads = new KeyedOps(key => this._pushToS3(key), {
       delayBeforeOperationMs: secondsBeforePush * 1000,

test/nbrowser/DescriptionColumn.ts

@@ -14,6 +14,7 @@ describe('DescriptionColumn', function() {
   });
 
   it('should allow to edit description on summary table', async () => {
+    const revert = await gu.begin();
     await gu.toggleSidePanel('left', 'close');
     // Add summary table.
     await gu.addNewSection('Table', 'Table1', {summarize: ['A']});
@@ -37,7 +38,45 @@ describe('DescriptionColumn', function() {
     assert.equal(await getDescriptionInput().getAttribute('value'), 'testA');
     await gu.openColumnPanel('count');
     assert.equal(await getDescriptionInput().getAttribute('value'), 'testCount');
-    await gu.undo(4);
+    await gu.undo(2);
+
+    // Now add description through the modal.
+    await doubleClickHeader('A', null);
+    assert.isTrue(await popupVisible());
+
+    // Name should be disabled.
+    assert.equal(await getLabel().getAttribute('disabled'), 'true');
+
+    // We see add description button.
+    await addDescriptionIsVisible();
+
+    // We have tooltip explaining what it is.
+    await getLabel().mouseMove();
+
+    // Wait for hover tooltip to show up.
+    await gu.waitToPass(
+      async () => assert.isTrue(await driver.find('.test-tooltip').isDisplayed()),
+      500
+    );
+    assert.equal(await driver.find('.test-tooltip').getText(), 'Column options are limited in summary tables.');
+
+    // It works.
+    await clickAddDescription();
+
+    // Now we see description field.
+    await descriptionIsVisible();
+
+    // Type something.
+    await gu.sendKeys('ColumnA description');
+
+    // Save it.
+    await pressSave();
+
+    // Make sure it is saved.
+    await clickTooltip('A');
+    await gu.waitToPass(async () =>
+      assert.equal(await driver.find(".test-column-info-tooltip-popup").getText(), 'ColumnA description'));
+    await revert();
   });
 
   it('should switch between close and save', async () => {
@@ -94,7 +133,7 @@ describe('DescriptionColumn', function() {
 
     // Clear label completely, we have change, but we can't save.
     await gu.sendKeys(Key.BACK_SPACE);
-    assert.isEmpty(await getLabel());
+    assert.isEmpty(await getLabelText());
     assert.isFalse(await closeVisible());
     assert.isTrue(await saveVisible());
     // But save button is disabled.
@@ -535,8 +574,12 @@ function getDescriptionInput() {
   return driver.find('.test-right-panel .test-column-description');
 }
 
+function getLabelText() {
+  return getLabel().getAttribute('value');
+}
+
 function getLabel() {
-  return driver.findWait(".test-column-title-label", 1000).getAttribute('value');
+  return driver.findWait(".test-column-title-label", 1000);
 }
 
 async function popupVisible() {
@@ -549,7 +592,7 @@ async function popupVisible() {
 
 async function popupIsAt(col: string) {
   // Make sure we are now at column.
-  assert.equal(await getLabel(), col);
+  assert.equal(await getLabelText(), col);
   // Make sure that popup is near the column.
   const headerCRect = await gu.getColumnHeader({col}).getRect();
   const popup = await driver.find(".test-column-title-popup").getRect();
@@ -559,15 +602,18 @@ async function popupIsAt(col: string) {
   assert.isBelow(popup.y, headerCRect.y + headerCRect.height + 2);
 }
 
-async function doubleClickHeader(col: string) {
+async function doubleClickHeader(col: string, focus: 'label'|'description'|null = 'label') {
   const header = await gu.getColumnHeader({col});
   await header.click();
   await header.click();
-  await waitForFocus('label');
+  if (focus) {
+    await waitForFocus(focus);
+  }
 }
 
 async function waitForFocus(field: 'label'|'description') {
-  await gu.waitToPass(async () => assert.isTrue(await driver.find(`.test-column-title-${field}`).hasFocus()), 200);
+  await gu.waitToPass(async () => assert.isTrue(
+    await driver.find(`.test-column-title-${field}`).hasFocus(), `${field} doesn't have focus`), 200);
 }
 
 async function waitForTooltip() {