diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index 52261aa2..54e137e5 100644 --- a/app/client/components/GridView.js +++ b/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() && !isSummary(); + return goodIndex() && !isReadonly(); }, 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), diff --git a/app/client/ui/ColumnTitle.ts b/app/client/ui/ColumnTitle.ts index 5ce862aa..9b14e1a9 100644 --- a/app/client/ui/ColumnTitle.ts +++ b/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; + /** + * 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; + /** + * Optional computed or boolean to determine if the description can be changed. Defaults to true. + */ + canChangeDesc?: ko.Computed|boolean; } export function buildRenameColumn(options: IColumnTitleOptions) { @@ -49,9 +66,8 @@ export function buildRenameColumn(options: IColumnTitleOptions) { }; } -function buildColumnRenamePopup( - ctrl: IOpenController, {field, isEditing, optCommands}: IColumnTitleOptions -) { +function buildColumnRenamePopup(ctrl: IOpenController, options: 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|undefined) => { + return val === undefined ? true : val; + }; + const toComputed = (val: boolean|ko.Computed) => + typeof val === 'boolean' ? Computed.create(ctrl, () => val) : Computed.create(ctrl, use => use(val)); + + const not = (val: Observable) => 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; } - labelInput?.focus(); - labelInput?.select(); + 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 diff --git a/app/client/ui/FieldConfig.ts b/app/client/ui/FieldConfig.ts index 5f528a5d..5c8af5e0 100644 --- a/app/client/ui/FieldConfig.ts +++ b/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)) ]; } diff --git a/app/client/ui/tooltips.ts b/app/client/ui/tooltips.ts index 48b7e5c3..3732cbfd 100644 --- a/app/client/ui/tooltips.ts +++ b/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; } 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; } diff --git a/app/common/gutil.ts b/app/common/gutil.ts index be9a8b1a..052e4271 100644 --- a/app/common/gutil.ts +++ b/app/common/gutil.ts @@ -999,7 +999,11 @@ export function useBindable(use: UseCBOwner, obs: BindableValue): T { /** * Useful helper for simple boolean negation. */ -export const not = (obs: Observable|IKnockoutReadObservable) => (use: UseCBOwner) => !use(obs); +export const not = (obs: Observable|IKnockoutReadObservable|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`. diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts index 7aa02696..d9617f95 100644 --- a/app/server/lib/HostedStorageManager.ts +++ b/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, diff --git a/test/nbrowser/DescriptionColumn.ts b/test/nbrowser/DescriptionColumn.ts index aa7dc57c..054c5264 100644 --- a/test/nbrowser/DescriptionColumn.ts +++ b/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() {