diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index 106ea871..b71f190b 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -642,4 +642,11 @@ BaseView.prototype.createFilterMenu = function(openCtl, field) { return createFilterMenu(openCtl, this._sectionFilter, field, this._filteredRowSource, this.tableModel.tableData); }; +/** + * Whether the rows shown by this view are a proper subset of all rows in the table. + */ +BaseView.prototype.isFiltered = function() { + return this._filteredRowSource.getNumRows() < this.tableModel.tableData.numRecords(); +}; + module.exports = BaseView; diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index a4ee4f60..6e05e99a 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -28,6 +28,7 @@ const {onDblClickMatchElem} = require('app/client/lib/dblclick'); // Grist UI Components const {Holder} = require('grainjs'); const {menu} = require('../ui2018/menus'); +const {calcFieldsCondition} = require('../ui/GridViewMenus'); const {ColumnAddMenu, ColumnContextMenu, MultiColumnMenu, RowContextMenu} = require('../ui/GridViewMenus'); const {setPopupToCreateDom} = require('popweasel'); const {testId} = require('app/client/ui2018/cssVars'); @@ -184,6 +185,8 @@ GridView.gridCommands = { hideField: function() { this.hideField(this.cursor.fieldIndex()); }, deleteFields: function() { this.deleteColumns(this.getSelection()); }, clearValues: function() { this.clearValues(this.getSelection()); }, + clearColumns: function() { this._clearColumns(this.getSelection()); }, + convertFormulasToData: function() { this._convertFormulasToData(this.getSelection()); }, copy: function() { return this.copy(this.getSelection()); }, cut: function() { return this.cut(this.getSelection()); }, paste: function(pasteObj, cutCallback) { return this.paste(pasteObj, cutCallback); }, @@ -409,16 +412,18 @@ GridView.prototype.clearSelection = function() { }; /** - * Given a selection object, sets all references in the object to the empty string. - * @param {Object} selection + * Given a selection object, sets all cells referred to by the selection to the empty string. If + * only formula columns are selected, only open the formula editor to the empty formula. + * @param {CopySelection} selection */ GridView.prototype.clearValues = function(selection) { console.debug('GridView.clearValues', selection); selection.rowIds = _.without(selection.rowIds, 'new'); - if (selection.rowIds.length === 0) { - // If only the addRow was selected, don't send an action. - return; - } else if (selection.fields.length === 1 && selection.fields[0].column().isRealFormula()) { + // If only the addRow was selected, don't send an action. + if (selection.rowIds.length === 0) { return; } + + const options = this._getColumnMenuOptions(selection); + if (options.isFormula === true) { this.activateEditorAtCursor(''); } else { let clearAction = tableUtil.makeDeleteAction(selection); @@ -428,6 +433,30 @@ GridView.prototype.clearValues = function(selection) { } }; +GridView.prototype._clearColumns = function(selection) { + const fields = selection.fields; + return this.gristDoc.docModel.columns.sendTableAction( + ['BulkUpdateRecord', fields.map(f => f.colRef.peek()), { + isFormula: fields.map(f => true), + formula: fields.map(f => ''), + }] + ); +}; + +GridView.prototype._convertFormulasToData = function(selection) { + // Convert all isFormula columns to data, including empty columns. This is sometimes useful + // (e.g. since a truly empty column undergoes a conversion on first data entry, which may be + // prevented by ACL rules). + const fields = selection.fields.filter(f => f.column.peek().isFormula.peek()); + if (!fields.length) { return null; } + return this.gristDoc.docModel.columns.sendTableAction( + ['BulkUpdateRecord', fields.map(f => f.colRef.peek()), { + isFormula: fields.map(f => false), + formula: fields.map(f => ''), + }] + ); +}; + GridView.prototype.selectAll = function() { this.cellSelector.selectArea(0, 0, this.getLastDataRowIndex(), this.viewSection.viewFields().peekLength - 1); @@ -801,7 +830,7 @@ GridView.prototype.buildDom = function() { trigger: [], }); }, - menu(ctl => this.columnContextMenu(ctl, this.getSelection().colIds, field, filterTriggerCtl)), + menu(ctl => this.columnContextMenu(ctl, this.getSelection(), field, filterTriggerCtl)), testId('column-menu-trigger'), ) ); @@ -1223,23 +1252,33 @@ GridView.prototype._selectMovedElements = function(start, end, newIndex, numEles // =========================================================================== // CONTEXT MENUS -GridView.prototype.columnContextMenu = function (ctl, selectedColIds, field, filterTriggerCtl) { +GridView.prototype.columnContextMenu = function(ctl, copySelection, field, filterTriggerCtl) { + const selectedColIds = copySelection.colIds; this.ctxMenuHolder.autoDispose(ctl); - const isReadonly = this.gristDoc.isReadonly.get(); + const options = this._getColumnMenuOptions(copySelection); + if (selectedColIds.length > 1 && selectedColIds.includes(field.column().colId())) { - return MultiColumnMenu({isReadonly}); + return MultiColumnMenu(options); } else { return ColumnContextMenu({ - disableModify: Boolean(field.disableModify.peek()), filterOpenFunc: () => filterTriggerCtl.open(), - useNewUI: this.gristDoc.app.useNewUI, sortSpec: this.gristDoc.viewModel.activeSection.peek().activeSortSpec.peek(), colId: field.column.peek().id.peek(), - isReadonly + ...options, }); } }; +GridView.prototype._getColumnMenuOptions = function(copySelection) { + return { + numColumns: copySelection.fields.length, + disableModify: calcFieldsCondition(copySelection.fields, f => f.disableModify.peek()), + isReadonly: this.gristDoc.isReadonly.get(), + isFiltered: this.isFiltered(), + isFormula: calcFieldsCondition(copySelection.fields, f => f.column.peek().isRealFormula.peek()), + }; +} + GridView.prototype._columnFilterMenu = function(ctl, field) { this.ctxMenuHolder.autoDispose(ctl); return this.createFilterMenu(ctl, field); diff --git a/app/client/components/commandList.js b/app/client/components/commandList.js index 247b0055..b69b1fa6 100644 --- a/app/client/components/commandList.js +++ b/app/client/components/commandList.js @@ -335,6 +335,14 @@ exports.groups = [{ name: 'deleteFields', keys: ['Alt+-'], desc: 'Delete the currently selected columns' + }, { + name: 'clearColumns', + keys: [], + desc: 'Clear the selected columns' + }, { + name: 'convertFormulasToData', + keys: [], + desc: 'Convert the selected columns from formula to data' }, { name: 'addSection', keys: [], diff --git a/app/client/declarations.d.ts b/app/client/declarations.d.ts index 208c5fb7..3d485fec 100644 --- a/app/client/declarations.d.ts +++ b/app/client/declarations.d.ts @@ -245,6 +245,7 @@ declare module "app/client/models/TableModel" { constructor(docModel: DocModel, tableData: TableData); public fetch(force?: boolean): Promise; public getAllRows(): ReadonlyArray; + public getNumRows(): number; public getRowGrouping(groupByCol: string): RowGrouping; public sendTableActions(actions: UserAction[], optDesc?: string): Promise; public sendTableAction(action: UserAction, optDesc?: string): Promise | undefined; diff --git a/app/client/models/DataTableModelWithDiff.ts b/app/client/models/DataTableModelWithDiff.ts index 809e5d84..a1d0704c 100644 --- a/app/client/models/DataTableModelWithDiff.ts +++ b/app/client/models/DataTableModelWithDiff.ts @@ -133,6 +133,10 @@ export class DataTableModelWithDiff extends DisposableWithEvents implements Data return this.core.getAllRows(); } + public getNumRows(): number { + return this.core.getNumRows(); + } + public getRowGrouping(groupByCol: string): RowGrouping { return this.core.getRowGrouping(groupByCol); } diff --git a/app/client/models/QuerySet.ts b/app/client/models/QuerySet.ts index 6e4e90d2..e4da1fab 100644 --- a/app/client/models/QuerySet.ts +++ b/app/client/models/QuerySet.ts @@ -130,6 +130,10 @@ export class DynamicQuerySet extends RowSource { return this._querySet ? this._querySet.getAllRows() : []; } + public getNumRows(): number { + return this._querySet ? this._querySet.getNumRows() : 0; + } + /** * Tells whether the query's result got truncated, i.e. not all rows are included. */ diff --git a/app/client/models/TableModel.js b/app/client/models/TableModel.js index 3699799d..dc1c3bd0 100644 --- a/app/client/models/TableModel.js +++ b/app/client/models/TableModel.js @@ -35,6 +35,10 @@ TableModel.prototype.getAllRows = function() { return this.tableData.getRowIds(); }; +TableModel.prototype.getNumRows = function() { + return this.tableData.numRecords(); +}; + TableModel.prototype.getRowGrouping = function(groupByCol) { var grouping = this.rowGroupings[groupByCol]; if (!grouping) { diff --git a/app/client/models/rowset.ts b/app/client/models/rowset.ts index 8f4b93bc..4881bd6e 100644 --- a/app/client/models/rowset.ts +++ b/app/client/models/rowset.ts @@ -48,13 +48,16 @@ export type RowsChanged = RowList | typeof ALL; * and `rowNotify(rows, value)` event to notify listeners of a value associated with a row. * For the `rowNotify` event, rows may be the rowset.ALL constant. */ -export class RowSource extends DisposableWithEvents { +export abstract class RowSource extends DisposableWithEvents { /** * Returns an iterable over all rows in this RowSource. Should be implemented by derived classes. */ - public getAllRows(): RowList { - throw new Error("RowSource#getAllRows: Not implemented"); - } + public abstract getAllRows(): RowList; + + /** + * Returns the number of rows in this row source. + */ + public abstract getNumRows(): number; } // ---------------------------------------------------------------------- @@ -123,6 +126,15 @@ export class RowListener extends DisposableWithEvents { // MappedRowSource // ---------------------------------------------------------------------- +/** + * A trivial RowSource returning a fixed list of rows. + */ +export abstract class ArrayRowSource extends RowSource { + constructor(private _rows: RowId[]) { super(); } + public getAllRows(): RowList { return this._rows; } + public getNumRows(): number { return this._rows.length; } +} + /** * MappedRowSource wraps any other RowSource, and passes through all rows, replacing each row * identifier with the result of mapperFunc(row) call. @@ -155,6 +167,10 @@ export class MappedRowSource extends RowSource { public getAllRows(): RowList { return Array.from(this.parentRowSource.getAllRows(), this._mapperFunc); } + + public getNumRows(): number { + return this.parentRowSource.getNumRows(); + } } /** @@ -180,6 +196,10 @@ export class ExtendedRowSource extends RowSource { public getAllRows(): RowList { return [...this.parentRowSource.getAllRows()].concat(this.extras); } + + public getNumRows(): number { + return this.parentRowSource.getNumRows() + this.extras.length; + } } // ---------------------------------------------------------------------- @@ -209,6 +229,10 @@ export class BaseFilteredRowSource extends RowListener implements RowSource { return this._matchingRows.values(); } + public getNumRows(): number { + return this._matchingRows.size; + } + public onAddRows(rows: RowList) { const outputRows = []; for (const r of rows) { @@ -353,6 +377,10 @@ class RowGroupHelper extends RowSource { return this.rows.values(); } + public getNumRows(): number { + return this.rows.size; + } + public _addAll(rows: RowList) { for (const r of rows) { this.rows.add(r); } } diff --git a/app/client/ui/GridViewMenus.ts b/app/client/ui/GridViewMenus.ts index ab90f1cd..7f6bb9b2 100644 --- 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), ]; } diff --git a/app/client/ui/tooltips.ts b/app/client/ui/tooltips.ts index cd55b3b6..f903222a 100644 --- 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 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(); + +/** + * 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; `); diff --git a/app/client/widgets/BaseEditor.js b/app/client/widgets/BaseEditor.js index b508710b..7e204da9 100644 --- a/app/client/widgets/BaseEditor.js +++ b/app/client/widgets/BaseEditor.js @@ -19,6 +19,14 @@ BaseEditor.prototype.attach = function(cellElem) { // No-op by default. }; +/** + * Returns DOM container with the editor, typically present and attached after attach() has been + * called. + */ +BaseEditor.prototype.getDom = function() { + return null; +}; + /** * Called to get the value to save back to the cell. */ diff --git a/app/client/widgets/EditorTooltip.ts b/app/client/widgets/EditorTooltip.ts new file mode 100644 index 00000000..fa1029ff --- /dev/null +++ b/app/client/widgets/EditorTooltip.ts @@ -0,0 +1,51 @@ +import {ITooltipControl, showTooltip} from 'app/client/ui/tooltips'; +import {colors, testId} from 'app/client/ui2018/cssVars'; +import {icon} from 'app/client/ui2018/icons'; +import {cssLink} from 'app/client/ui2018/links'; +import {dom, styled} from 'grainjs'; + +export function showTooltipToCreateFormula(editorDom: HTMLElement, convert: () => void) { + function buildTooltip(ctl: ITooltipControl) { + return cssConvertTooltip(icon('Convert'), + cssLink('Convert column to formula', + dom.on('mousedown', (ev) => { ev.preventDefault(); convert(); }), + testId('editor-tooltip-convert'), + ), + cssCloseButton(icon('CrossSmall'), dom.on('click', ctl.close)), + ); + } + const offerCtl = showTooltip(editorDom, buildTooltip, {key: 'col-to-formula'}); + + dom.onDisposeElem(editorDom, offerCtl.close); + const lis = dom.onElem(editorDom, 'keydown', () => { + lis.dispose(); + offerCtl.close(); + }); +} + +const cssConvertTooltip = styled('div', ` + display: flex; + align-items: center; + --icon-color: ${colors.lightGreen}; + + & > .${cssLink.className} { + margin-left: 8px; + } +`); + +const cssCloseButton = styled('div', ` + cursor: pointer; + user-select: none; + width: 16px; + height: 16px; + line-height: 16px; + text-align: center; + margin: -4px -4px -4px 8px; + --icon-color: white; + border-radius: 16px; + + &:hover { + background-color: white; + --icon-color: black; + } +`); diff --git a/app/client/widgets/FieldEditor.ts b/app/client/widgets/FieldEditor.ts index 56d8858c..94ae14c9 100644 --- a/app/client/widgets/FieldEditor.ts +++ b/app/client/widgets/FieldEditor.ts @@ -5,6 +5,7 @@ import {UnsavedChange} from 'app/client/components/UnsavedChanges'; import {DataRowModel} from 'app/client/models/DataRowModel'; import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec'; import {reportError} from 'app/client/models/errors'; +import {showTooltipToCreateFormula} from 'app/client/widgets/EditorTooltip'; import {FormulaEditor} from 'app/client/widgets/FormulaEditor'; import {IEditorCommandGroup, NewBaseEditor} from 'app/client/widgets/NewBaseEditor'; import {CellValue} from "app/common/DocActions"; @@ -72,18 +73,21 @@ export class FieldEditor extends Disposable { this._cellElem = options.cellElem; const startVal = options.startVal; + let offerToMakeFormula = false; const column = this._field.column(); - let isFormula: boolean, editValue: string|undefined; + let isFormula: boolean = column.isRealFormula.peek(); + let editValue: string|undefined = startVal; if (startVal && gutil.startsWith(startVal, '=')) { - // If we entered the cell by typing '=', we immediately convert to formula. - isFormula = true; - editValue = gutil.removePrefix(startVal, '=') as string; - } else { - // Initially, we mark the field as editing formula if it's a non-empty formula field. This can - // be changed by typing "=", but the field won't be an actual formula field until saved. - isFormula = column.isRealFormula.peek(); - editValue = startVal; + if (isFormula || this._field.column().isEmpty()) { + // If we typed '=' on an empty column, convert it to a formula. If on a formula column, + // start editing ignoring the initial '='. + isFormula = true; + editValue = gutil.removePrefix(startVal, '=') as string; + } else { + // If we typed '=' on a non-empty column, only suggest to convert it to a formula. + offerToMakeFormula = true; + } } // These are the commands for while the editor is active. @@ -108,6 +112,10 @@ export class FieldEditor extends Disposable { this.rebuildEditor(isFormula, editValue, Number.POSITIVE_INFINITY); + if (offerToMakeFormula) { + this._offerToMakeFormula(); + } + // Whenever focus returns to the Clipboard component, close the editor by saving the value. this._gristDoc.app.on('clipboard_focus', this._saveEdit, this); @@ -161,10 +169,16 @@ export class FieldEditor extends Disposable { private _makeFormula() { const editor = this._editorHolder.get(); - // On keyPress of "=" on textInput, turn the value into a formula. + // On keyPress of "=" on textInput, consider turning the column into a formula. if (editor && !this._field.editingFormula.peek() && editor.getCursorPos() === 0) { - this.rebuildEditor(true, editor.getTextValue(), 0); - return false; + if (this._field.column().isEmpty()) { + // If we typed '=' an empty column, convert it to a formula. + this.rebuildEditor(true, editor.getTextValue(), 0); + return false; + } else { + // If we typed '=' on a non-empty column, only suggest to convert it to a formula. + this._offerToMakeFormula(); + } } return true; // don't stop propagation. } @@ -172,7 +186,7 @@ export class FieldEditor extends Disposable { private _unmakeFormula() { const editor = this._editorHolder.get(); // Only convert to data if we are undoing a to-formula conversion. To convert formula to - // data, delete the formula first (which makes the column "empty"). + // data, use column menu option, or delete the formula first (which makes the column "empty"). if (editor && this._field.editingFormula.peek() && editor.getCursorPos() === 0 && !this._field.column().isRealFormula()) { // Restore a plain '=' character. This gives a way to enter "=" at the start if line. The @@ -183,11 +197,27 @@ export class FieldEditor extends Disposable { return true; // don't stop propagation. } + private _offerToMakeFormula() { + const editorDom = this._editorHolder.get()?.getDom(); + if (!editorDom) { return; } + showTooltipToCreateFormula(editorDom, () => this._convertEditorToFormula()); + } + + private _convertEditorToFormula() { + const editor = this._editorHolder.get(); + if (editor) { + const editValue = editor.getTextValue(); + const formulaValue = editValue.startsWith('=') ? editValue.slice(1) : editValue; + this.rebuildEditor(true, formulaValue, 0); + } + } + private async _saveEdit() { return this._saveEditPromise || (this._saveEditPromise = this._doSaveEdit()); } - // Returns whether the cursor jumped, i.e. current record got reordered. + // Returns true if Enter/Shift+Enter should NOT move the cursor, for instance if the current + // record got reordered (i.e. the cursor jumped), or when editing a formula. private async _doSaveEdit(): Promise { const editor = this._editorHolder.get(); if (!editor) { return false; } @@ -205,19 +235,12 @@ export class FieldEditor extends Disposable { // editingFormula() is used for toggling column headers, and this is deferred to start of // typing (a double-click or Enter) does not immediately set it. (This can cause a // console.warn below, although harmless.) - let isFormula = this._field.editingFormula(); + const isFormula = this._field.editingFormula(); const col = this._field.column(); let waitPromise: Promise|null = null; if (isFormula) { const formula = editor.getCellValue(); - if (col.isRealFormula() && formula === "") { - // A somewhat surprising feature: deleting the formula converts the column to data, keeping - // the values. To clear the column, enter an empty formula again (now into a data column). - // TODO: this should probably be made more intuitive. - isFormula = false; - } - // Bundle multiple changes so that we can undo them in one step. if (isFormula !== col.isFormula.peek() || formula !== col.formula.peek()) { waitPromise = this._gristDoc.docData.bundleActions(null, () => Promise.all([ @@ -243,6 +266,6 @@ export class FieldEditor extends Disposable { // Deactivate the editor. We are careful to avoid using `this` afterwards. this.dispose(); await waitPromise; - return (saveIndex !== cursor.rowIndex()); + return isFormula || (saveIndex !== cursor.rowIndex()); } } diff --git a/app/client/widgets/NTextEditor.ts b/app/client/widgets/NTextEditor.ts index 5e3d814f..874adff0 100644 --- a/app/client/widgets/NTextEditor.ts +++ b/app/client/widgets/NTextEditor.ts @@ -59,6 +59,10 @@ export class NTextEditor extends NewBaseEditor { this.textInput.setSelectionRange(pos, pos); } + public getDom(): HTMLElement { + return this._dom; + } + public getCellValue(): CellValue { return this.textInput.value; } diff --git a/app/client/widgets/NewBaseEditor.ts b/app/client/widgets/NewBaseEditor.ts index a3ddc953..fb8a49fb 100644 --- a/app/client/widgets/NewBaseEditor.ts +++ b/app/client/widgets/NewBaseEditor.ts @@ -65,6 +65,12 @@ export abstract class NewBaseEditor extends Disposable { */ public abstract attach(cellElem: Element): void; + /** + * Returns DOM container with the editor, typically present and attached after attach() has been + * called. + */ + public getDom(): HTMLElement|null { return null; } + /** * Called to get the value to save back to the cell. */ diff --git a/app/client/widgets/TextEditor.js b/app/client/widgets/TextEditor.js index 05937949..d261d7cd 100644 --- a/app/client/widgets/TextEditor.js +++ b/app/client/widgets/TextEditor.js @@ -69,6 +69,10 @@ TextEditor.prototype.attach = function(cellElem) { this.textInput.setSelectionRange(pos, pos); }; +TextEditor.prototype.getDom = function() { + return this.dom; +}; + TextEditor.prototype.setSizerLimits = function() { // Set the max width of the sizer to the max we could possibly grow to, so that it knows to wrap // once we reach it.