diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index 0c35ac0d..0099c68e 100644 --- a/app/client/components/BaseView.js +++ b/app/client/components/BaseView.js @@ -1,21 +1,23 @@ -var _ = require('underscore'); -var ko = require('knockout'); -var moment = require('moment-timezone'); -var {getSelectionDesc} = require('app/common/DocActions'); -var {nativeCompare, roundDownToMultiple, waitObs} = require('app/common/gutil'); -var gutil = require('app/common/gutil'); +/* globals KeyboardEvent */ + +const _ = require('underscore'); +const ko = require('knockout'); +const moment = require('moment-timezone'); +const {getSelectionDesc} = require('app/common/DocActions'); +const {nativeCompare, roundDownToMultiple, waitObs} = require('app/common/gutil'); +const gutil = require('app/common/gutil'); const MANUALSORT = require('app/common/gristTypes').MANUALSORT; -var gristTypes = require('app/common/gristTypes'); -var tableUtil = require('../lib/tableUtil'); -var {DataRowModel} = require('../models/DataRowModel'); -var {DynamicQuerySet} = require('../models/QuerySet'); -var {SortFunc} = require('app/common/SortFunc'); -var rowset = require('../models/rowset'); -var Base = require('./Base'); -var {Cursor} = require('./Cursor'); -var FieldBuilder = require('../widgets/FieldBuilder'); -var commands = require('./commands'); -var BackboneEvents = require('backbone').Events; +const gristTypes = require('app/common/gristTypes'); +const tableUtil = require('../lib/tableUtil'); +const {DataRowModel} = require('../models/DataRowModel'); +const {DynamicQuerySet} = require('../models/QuerySet'); +const {SortFunc} = require('app/common/SortFunc'); +const rowset = require('../models/rowset'); +const Base = require('./Base'); +const {Cursor} = require('./Cursor'); +const FieldBuilder = require('../widgets/FieldBuilder'); +const commands = require('./commands'); +const BackboneEvents = require('backbone').Events; const {ClientColumnGetters} = require('app/client/models/ClientColumnGetters'); const {reportError, reportSuccess} = require('app/client/models/errors'); const {urlState} = require('app/client/models/gristUrlState'); @@ -26,7 +28,9 @@ const {ExtraRows} = require('app/client/models/DataTableModelWithDiff'); const {createFilterMenu} = require('app/client/ui/ColumnFilterMenu'); const {closeRegisteredMenu} = require('app/client/ui2018/menus'); const {COMMENTS} = require('app/client/models/features'); - +const {DismissedPopup} = require('app/common/Prefs'); +const {markAsSeen} = require('app/client/models/UserPrefs'); +const {buildConfirmDelete, reportUndo} = require('app/client/components/modals'); /** * BaseView forms the basis for ViewSection classes. @@ -241,11 +245,48 @@ BaseView.commonCommands = { showRawData: function() { this.showRawData().catch(reportError); }, + deleteRecords: function(source) { this.deleteRecords(source); }, + filterByThisCellValue: function() { this.filterByThisCellValue(); }, duplicateRows: function() { this._duplicateRows().catch(reportError); }, openDiscussion: function() { this.openDiscussionAtCursor(); }, }; +BaseView.prototype.selectedRows = function() { + return []; +}; + +BaseView.prototype.deleteRows = function(rowIds) { + return this.tableModel.sendTableAction(['BulkRemoveRecord', rowIds]); +}; + +BaseView.prototype.deleteRecords = function(source) { + const rowIds = this.selectedRows(); + if (this.viewSection.disableAddRemoveRows() || rowIds.length === 0){ + return; + } + const isKeyboard = source instanceof KeyboardEvent; + const popups = this.gristDoc.docPageModel.appModel.dismissedPopups; + const popupName = DismissedPopup.check('deleteRecords'); + const onSave = async (remember) => { + if (remember) { + markAsSeen(popups, popupName); + } + return this.deleteRows(rowIds); + }; + if (isKeyboard && !popups.get().includes(popupName)) { + // If we can't find it, use viewPane itself + this.scrollToCursor(); + const selectedCell = this.viewPane.querySelector(".selected_cursor") || this.viewPane; + buildConfirmDelete(selectedCell, onSave, rowIds.length <= 1); + } else { + onSave().then(() => { + reportUndo(this.gristDoc, `You deleted ${rowIds.length} row${rowIds.length > 1 ? 's' : ''}.`); + return true; + }); + } +}; + /** * Sets the cursor to the given position, deferring if necessary until the current query finishes * loading. diff --git a/app/client/components/DeprecatedCommands.ts b/app/client/components/DeprecatedCommands.ts new file mode 100644 index 00000000..a66baa55 --- /dev/null +++ b/app/client/components/DeprecatedCommands.ts @@ -0,0 +1,118 @@ +import * as commands from 'app/client/components/commands'; +import {Command} from 'app/client/components/commands'; +import {markAsSeen} from 'app/client/models/UserPrefs'; +import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals'; +import {reportMessage} from 'app/client/models/errors'; +import {DeprecationWarning} from 'app/common/Prefs'; +import {GristDoc} from 'app/client/components/GristDoc'; +import {showDeprecatedWarning} from 'app/client/components/modals'; +import {Disposable, dom, Holder, styled} from 'grainjs'; +import intersection from "lodash/intersection"; + +const G = getBrowserGlobals('document', 'window'); + +/** + * Manages deprecated commands and keyboard shortcuts. It subscribes itself to all commands and + * keyboard shortcuts, and shows a warning when a deprecated command is used. + */ +export class DeprecatedCommands extends Disposable { + // Holds the commands created by this class, so they can be disposed, + // when this class is disposed or reattached. + private _holder = Holder.create(this); + + constructor(private _gristDoc: GristDoc) { + super(); + G.window.resetSeenWarnings = () => {}; + } + + public attach() { + // We can be attached multiple times, so first clear previous commands. + this._holder.clear(); + + // Get all the warnings from the app model and expose reset function (used in tests only). + // When we reset the warnings, we also need to reattach ourselves. + const seenWarnings = this._gristDoc.docPageModel.appModel.deprecatedWarnings; + G.window.resetSeenWarnings = () => { + if (!this._gristDoc.isDisposed()) { + seenWarnings.set([]); + this.attach(); + } + }; + + // We wan't do anything for anonymous users. + if (!this._gristDoc.docPageModel.appModel.currentValidUser) { + return; + } + + // If user has seen all keyboard warnings, don't need to do anything. + const commandList = Object.values(commands.allCommands as Record); + const deprecatedCommands = commandList.filter((command) => command.deprecated); + const deprecatedNames = deprecatedCommands.map((command) => command.name); + if (intersection(seenWarnings.get(), deprecatedNames).length === deprecatedNames.length) { + return; + } + + // Now subscribe to all the commands and handle them. + const group: any = {}; + for (const c of deprecatedCommands) { + group[c.name] = this._handleCommand.bind(this, c); + } + if (Object.keys(group).length) { + this._holder.autoDispose(commands.createGroup(group, this, true)); + } + } + + private _handleCommand(c: Command) { + const seenWarnings = this._gristDoc.docPageModel.appModel.deprecatedWarnings; + if (!this._hasSeenWarning(c.name)) { + markAsSeen(seenWarnings, c.name); + this._showWarning(c.desc); + return false; // Stop processing. + } else { + return true; // Continue processing. + } + } + + private _showWarning(desc: string) { + // Try to figure out where to show the message. If we have active view, we can try + // to find the selected cell and show the message there. Otherwise, we show it in the + // bottom right corner as a warning. + const selectedCell = this._gristDoc.currentView.get()?.viewPane.querySelector(".selected_cursor"); + if (!selectedCell) { + reportMessage(() => dom('div', this._createMessage(desc)), { + level: 'info', + key: 'deprecated-command', + }); + } else { + showDeprecatedWarning(selectedCell, this._createMessage(desc)); + } + } + + private _hasSeenWarning(name: string) { + const seenWarnings = this._gristDoc.docPageModel.appModel.deprecatedWarnings; + const preference = seenWarnings.get() ?? []; + return preference.includes(DeprecationWarning.check(name)); + } + + private _createMessage(description: string) { + const elements: Node[] = []; + // Description can have embedded commands in the form of {commandName}. + // To construct message we need to replace all {name} to key strokes dom. + for (const part of description.split(/({\w+})/g)) { + // If it starts with { + if (part[0] === '{') { + const otherCommand = commands.allCommands[part.slice(1, -1)]; + if (otherCommand) { + elements.push(otherCommand.getKeysDom()); + } + } else { + elements.push(cssTallerText(part)); + } + } + return elements; + } +} + +const cssTallerText = styled('span', ` + line-height: 24px; +`); diff --git a/app/client/components/DetailView.js b/app/client/components/DetailView.js index 1a3db0c2..6054a53d 100644 --- a/app/client/components/DetailView.js +++ b/app/client/components/DetailView.js @@ -1,18 +1,18 @@ -var _ = require('underscore'); -var ko = require('knockout'); +const _ = require('underscore'); +const ko = require('knockout'); -var dom = require('app/client/lib/dom'); -var kd = require('app/client/lib/koDom'); -var koDomScrolly = require('app/client/lib/koDomScrolly'); +const dom = require('app/client/lib/dom'); +const kd = require('app/client/lib/koDom'); +const koDomScrolly = require('app/client/lib/koDomScrolly'); const {renderAllRows} = require('app/client/components/Printing'); require('app/client/lib/koUtil'); // Needed for subscribeInit. -var Base = require('./Base'); -var BaseView = require('./BaseView'); -var {CopySelection} = require('./CopySelection'); -var RecordLayout = require('./RecordLayout'); -var commands = require('./commands'); +const Base = require('./Base'); +const BaseView = require('./BaseView'); +const {CopySelection} = require('./CopySelection'); +const RecordLayout = require('./RecordLayout'); +const commands = require('./commands'); const {RowContextMenu} = require('../ui/RowContextMenu'); const {parsePasteForView} = require("./BaseView2"); @@ -122,14 +122,6 @@ DetailView.generalCommands = { cursorDown: function() { this.cursor.fieldIndex(this.cursor.fieldIndex() + 1); }, pageUp: function() { this.cursor.rowIndex(this.cursor.rowIndex() - 1); }, pageDown: function() { this.cursor.rowIndex(this.cursor.rowIndex() + 1); }, - - deleteRecords: function() { - // Do not allow deleting the add record row. - if (!this._isAddRow()) { - this.deleteRow(this.cursor.rowIndex()); - } - }, - copy: function() { return this.copy(this.getSelection()); }, cut: function() { return this.cut(this.getSelection()); }, paste: function(pasteObj, cutCallback) { @@ -146,18 +138,21 @@ DetailView.generalCommands = { //---------------------------------------------------------------------- -// TODO: Factor code duplicated with GridView for deleteRow, deleteColumn, -// insertDetailField out of the view modules -DetailView.prototype.deleteRow = function(index) { - if (this.viewSection.disableAddRemoveRows()) { - return; +DetailView.prototype.selectedRows = function() { + if (!this._isAddRow()) { + return [this.viewData.getRowId(this.cursor.rowIndex())]; } - var action = ['RemoveRecord', this.viewData.getRowId(index)]; - return this.tableModel.sendTableAction(action) - .bind(this).then(function() { + return []; +}; + +DetailView.prototype.deleteRows = async function(rowIds) { + const index = this.cursor.rowIndex(); + try { + await BaseView.prototype.deleteRows.call(this, rowIds); + } finally { this.cursor.rowIndex(index); - }); + } }; /** diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index d09bdccf..2f1e745d 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -1,4 +1,4 @@ -/* globals alert, $ */ +/* globals $ */ const _ = require('underscore'); const ko = require('knockout'); @@ -26,7 +26,9 @@ const koUtil = require('app/client/lib/koUtil'); const convert = require('color-convert'); const {renderAllRows} = require('app/client/components/Printing'); -const {reportError} = require('app/client/models/AppModel'); +const {reportWarning} = require('app/client/models/errors'); +const {reportUndo} = require('app/client/components/modals'); + const {onDblClickMatchElem} = require('app/client/lib/dblclick'); // Grist UI Components @@ -46,7 +48,6 @@ const {showTooltip} = require('app/client/ui/tooltips'); const {parsePasteForView} = require("./BaseView2"); const {CombinedStyle} = require("app/client/models/Styles"); - // A threshold for interpreting a motionless click as a click rather than a drag. // Anything longer than this time (in milliseconds) should be interpreted as a drag // even if there is no movement. @@ -299,25 +300,19 @@ GridView.gridCommands = { // Re-define editField after fieldEditSave to make it take precedence for the Enter key. editField: function() { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor(); }, - deleteRecords: function() { - const saved = this.cursor.getCursorPos(); - this.cursor.setLive(false); - - // Don't return a promise. Nothing will use it, and the Command implementation will not - // prevent default browser behavior if we return a truthy value. - this.deleteRows(this.getSelection()) - .finally(() => { - this.cursor.setCursorPos(saved); - this.cursor.setLive(true); - this.clearSelection(); - }) - .catch(reportError); - }, insertFieldBefore: function() { this.insertColumn(this.cursor.fieldIndex()); }, insertFieldAfter: function() { this.insertColumn(this.cursor.fieldIndex() + 1); }, renameField: function() { this.currentEditingColumnIndex(this.cursor.fieldIndex()); }, hideFields: function() { this.hideFields(this.getSelection()); }, - deleteFields: function() { this.deleteColumns(this.getSelection()); }, + deleteFields: function() { + const selection = this.getSelection(); + const count = selection.colIds.length; + this.deleteColumns(selection).then((result) => { + if (result !== false) { + reportUndo(this.gristDoc, `You deleted ${count} column${count > 1 ? 's' : ''}.`); + } + }); + }, clearValues: function() { this.clearValues(this.getSelection()); }, clearColumns: function() { this._clearColumns(this.getSelection()); }, convertFormulasToData: function() { this._convertFormulasToData(this.getSelection()); }, @@ -670,14 +665,21 @@ GridView.prototype.preventAssignCursor = function() { this._assignCursorTimeoutId = null; } -GridView.prototype.deleteRows = function(selection) { - if (!this.viewSection.disableAddRemoveRows()) { - var rowIds = _.without(selection.rowIds, 'new'); - if (rowIds.length > 0) { - return this.tableModel.sendTableAction(['BulkRemoveRecord', rowIds]); - } +GridView.prototype.selectedRows = function() { + const selection = this.getSelection(); + return _.without(selection.rowIds, 'new'); +}; + +GridView.prototype.deleteRows = async function(rowIds) { + const saved = this.cursor.getCursorPos(); + this.cursor.setLive(false); + try { + await BaseView.prototype.deleteRows.call(this, rowIds); + } finally { + this.cursor.setCursorPos(saved); + this.cursor.setLive(true); + this.clearSelection(); } - return Promise.resolve(); }; GridView.prototype.addNewColumn = function() { @@ -728,14 +730,17 @@ GridView.prototype.showColumn = function(colId, index) { GridView.prototype.deleteColumns = function(selection) { var fields = selection.fields; if (fields.length === this.viewSection.viewFields().peekLength) { - alert("You can't delete all the columns on the grid."); - return; + reportWarning("You can't delete all the columns on the grid.", { + key: 'delete-all-columns', + }); + return Promise.resolve(false); } let actions = fields.filter(col => !col.disableModify()).map(col => ['RemoveColumn', col.colId()]); if (actions.length > 0) { - this.tableModel.sendTableActions(actions, `Removed columns ${actions.map(a => a[1]).join(', ')} ` + + return this.tableModel.sendTableActions(actions, `Removed columns ${actions.map(a => a[1]).join(', ')} ` + `from ${this.tableModel.tableData.tableId}.`).then(() => this.clearSelection()); } + return Promise.resolve(false); }; GridView.prototype.hideFields = function(selection) { diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 5ded57a8..37bb0081 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -11,6 +11,7 @@ import {CodeEditorPanel} from 'app/client/components/CodeEditorPanel'; import * as commands from 'app/client/components/commands'; import {CursorPos} from 'app/client/components/Cursor'; import {CursorMonitor, ViewCursorPos} from "app/client/components/CursorMonitor"; +import {DeprecatedCommands} from 'app/client/components/DeprecatedCommands'; import {DocComm} from 'app/client/components/DocComm'; import * as DocConfigTab from 'app/client/components/DocConfigTab'; import {Drafts} from "app/client/components/Drafts"; @@ -60,6 +61,7 @@ import {isList, isListType, isRefListType, RecalcWhen} from 'app/common/gristTyp import {HashLink, IDocPage, isViewDocPage, SpecialDocPage, ViewDocPage} from 'app/common/gristUrls'; import {undef, waitObs} from 'app/common/gutil'; import {LocalPlugin} from "app/common/plugin"; +import {DismissedPopup} from 'app/common/Prefs'; import {StringUnion} from 'app/common/StringUnion'; import {TableData} from 'app/common/TableData'; import {DocStateComparison} from 'app/common/UserAPI'; @@ -329,8 +331,8 @@ export class GristDoc extends DisposableWithEvents { /* Command binding */ this.autoDispose(commands.createGroup({ - undo() { this._undoStack.sendUndoAction().catch(reportError); }, - redo() { this._undoStack.sendRedoAction().catch(reportError); }, + undo(this: GristDoc) { this._undoStack.sendUndoAction().catch(reportError); }, + redo(this: GristDoc) { this._undoStack.sendRedoAction().catch(reportError); }, reloadPlugins() { this.docComm.reloadPlugins().then(() => G.window.location.reload(false)); }, }, this, true)); @@ -399,6 +401,12 @@ export class GristDoc extends DisposableWithEvents { this.draftMonitor = Drafts.create(this, this); this.cursorMonitor = CursorMonitor.create(this, this); this.editorMonitor = EditorMonitor.create(this, this); + + DeprecatedCommands.create(this, this).attach(); + + G.window.resetSeenPopups = (seen = false) => { + this.docPageModel.appModel.dismissedPopups.set(seen ? DismissedPopup.values : []); + }; } /** @@ -530,6 +538,7 @@ export class GristDoc extends DisposableWithEvents { this.trigger('schemaUpdateAction', docActions); } this.docPageModel.updateCurrentDocUsage(message.data.docUsage); + this.trigger('onDocUserAction', docActions); } } diff --git a/app/client/components/commandList.js b/app/client/components/commandList.js index 867d565d..3d07f1e5 100644 --- a/app/client/components/commandList.js +++ b/app/client/components/commandList.js @@ -1,3 +1,6 @@ +const { DeprecationWarning } = require('app/common/Prefs'); + + // The top-level groups, and the ordering within them are for user-facing documentation. exports.groups = [{ group: 'General', @@ -329,15 +332,15 @@ exports.groups = [{ commands: [ { name: 'insertRecordBefore', - keys: ['Mod+Shift+='], + keys: ['Mod+Shift+Enter'], desc: 'Insert a new record, before the currently selected one in an unsorted table' }, { name: 'insertRecordAfter', - keys: ['Mod+='], + keys: ['Mod+Enter'], desc: 'Insert a new record, after the currently selected one in an unsorted table', }, { name: 'deleteRecords', - keys: ['Mod+-'], + keys: ['Mod+Del', 'Mod+Backspace'], desc: 'Delete the currently selected record' }, { name: 'insertFieldBefore', @@ -383,7 +386,22 @@ exports.groups = [{ name: 'duplicateRows', keys: ['Mod+Shift+d'], desc: 'Duplicate selected rows' - } + }, { + name: DeprecationWarning.parse('deprecatedInsertRowBefore'), + keys: ['Mod+Shift+='], + desc: 'Shortcuts to remove or insert a record have changed, to avoid interfering with page zoom. In the future, to delete a record use {deleteRecords}, and to insert a record use {insertRecordAfter}.', + deprecated: true, + }, { + name: DeprecationWarning.parse('deprecatedInsertRecordAfter'), + keys: ['Mod+='], + desc: 'Shortcuts to remove or insert a record have changed, to avoid interfering with page zoom. In the future, to delete a record use {deleteRecords}, and to insert a record use {insertRecordAfter}.', + deprecated: true, + }, { + name: DeprecationWarning.parse('deprecatedDeleteRecords'), + keys: ['Mod+-'], + desc: 'Shortcuts to remove or insert a record have changed, to avoid interfering with page zoom. In the future, to delete a record use {deleteRecords}, and to insert a record use {insertRecordAfter}.', + deprecated: true, + }, ], }, { group: 'Sorting', diff --git a/app/client/components/commands.js b/app/client/components/commands.js index e846894b..38a91ec2 100644 --- a/app/client/components/commands.js +++ b/app/client/components/commands.js @@ -63,7 +63,7 @@ function init(optCommandGroups) { if (allCommands[c.name]) { console.error("Ignoring duplicate command %s in commandList", c.name); } else { - allCommands[c.name] = new Command(c.name, c.desc, c.keys); + allCommands[c.name] = new Command(c.name, c.desc, c.keys, c.deprecated); } }); }); @@ -116,7 +116,7 @@ function getHumanKey(key, isMac) { * @property {Function} run: A bound function that will run the currently active implementation. * @property {Observable} isActive: Knockout observable for whether this command is active. */ -function Command(name, desc, keys) { +function Command(name, desc, keys, deprecated) { this.name = name; this.desc = desc; this.humanKeys = keys.map(key => getHumanKey(key, isMac)); @@ -124,7 +124,7 @@ function Command(name, desc, keys) { this.isActive = ko.observable(false); this._implGroupStack = []; this._activeFunc = _.noop; // The function to run when this command is invoked. - + this.deprecated = deprecated || false; // Let .run bind the Command object, so that it can be used as a stand-alone callback. this.run = this._run.bind(this); } diff --git a/app/client/components/modals.ts b/app/client/components/modals.ts new file mode 100644 index 00000000..94b683f8 --- /dev/null +++ b/app/client/components/modals.ts @@ -0,0 +1,141 @@ +import * as commands from 'app/client/components/commands'; +import {GristDoc} from 'app/client/components/GristDoc'; +import {FocusLayer} from 'app/client/lib/FocusLayer'; +import {reportSuccess} from 'app/client/models/errors'; +import {basicButton, primaryButton} from 'app/client/ui2018/buttons'; +import {labeledSquareCheckbox} from 'app/client/ui2018/checkbox'; +import {testId, theme} from 'app/client/ui2018/cssVars'; +import {modalTooltip} from 'app/client/ui2018/modals'; +import {dom, DomContents, observable, styled} from 'grainjs'; + +/** + * This is a file for all custom and pre-configured popups, modals, toasts and tooltips, used + * in more then one component. + */ + +/** + * Tooltip or popup to confirm row deletion. + */ +export function buildConfirmDelete( + refElement: Element, + onSave: (remember: boolean) => void, + single = true, +) { + const remember = observable(false); + const tooltip = modalTooltip(refElement, (ctl) => + cssContainer( + dom.autoDispose(remember), + testId('confirm-deleteRows'), + testId('confirm-popup'), + elem => { FocusLayer.create(ctl, {defaultFocusElem: elem, pauseMousetrap: true}); }, + dom.onKeyDown({ + Escape: () => ctl.close(), + Enter: () => { onSave(remember.get()); ctl.close(); }, + }), + dom('div', `Are you sure you want to delete ${single ? 'this' : 'these'} record${single ? '' : 's'}?`, + dom.style('margin-bottom', '10px'), + ), + cssButtons( + dom.style('margin-bottom', '12px'), + primaryButton('Delete', testId('confirm-save'), dom.on('click', () => { + onSave(remember.get()); + ctl.close(); + })), + basicButton('Cancel', testId('confirm-cancel'), dom.on('click', () => ctl.close())) + ), + dom('div', + labeledSquareCheckbox(remember, "Don't ask again.", testId('confirm-remember')), + ) + ), {} + ); + // Attach this tooltip to a cell so that it is automatically closed when the cell is disposed. + // or scrolled out of view (and then disposed). + dom.onDisposeElem(refElement, () => { + if (!tooltip.isDisposed()) { + tooltip.close(); + } + }); + return tooltip; +} + +export function showDeprecatedWarning( + refElement: Element, + content: DomContents +) { + const tooltip = modalTooltip(refElement, (ctl) => + cssWideContainer( + testId('popup-warning-deprecated'), + elem => { FocusLayer.create(ctl, {defaultFocusElem: elem, pauseMousetrap: true}); }, + dom.onKeyDown({ + Escape: () => ctl.close(), + Enter: () => ctl.close(), + }), + content, + cssButtons( + dom.style('margin-top', '12px'), + dom.style('justify-content', 'right'), + basicButton('Close', testId('confirm-cancel'), dom.on('click', () => ctl.close())) + ), + ) + ); + // Attach this warning to a cell so that it is automatically closed when the cell is disposed. + // or scrolled out of view (and then disposed). + dom.onDisposeElem(refElement, () => { + if (!tooltip.isDisposed()) { + tooltip.close(); + } + }); + return tooltip; +} + +/** + * Shows notification with a single button 'Undo' delete. + */ +export function reportUndo( + doc: GristDoc, + messageLabel: string, + buttonLabel = 'Undo to restore' +) { + // First create a notification with a button to undo the delete. + let notification = reportSuccess(messageLabel, { + key: 'undo', + actions: [{ + label: buttonLabel, + action: () => { + // When user clicks on the button, undo the last action. + commands.allCommands.undo.run(); + // And remove this notification. + close(); + }, + }] + }); + + // When we received some actions from the server, cancel this popup, + // as the undo might do something else. + doc.on('onDocUserAction', close); + notification?.onDispose(() => doc.off('onDocUserAction', close)); + + function close() { + if (notification && !notification?.isDisposed()) { + notification.dispose(); + notification = undefined; + } + } +} + +const cssTheme = styled('div', ` + color: ${theme.text}; +`); + +const cssButtons = styled('div', ` + display: flex; + gap: 6px; +`); + +const cssContainer = styled(cssTheme, ` + max-width: 210px; +`); + +const cssWideContainer = styled(cssTheme, ` + max-width: 340px; +`); diff --git a/app/client/declarations.d.ts b/app/client/declarations.d.ts index f9c4aa02..3be3229e 100644 --- a/app/client/declarations.d.ts +++ b/app/client/declarations.d.ts @@ -110,6 +110,7 @@ declare module "app/client/components/ViewConfigTab" { declare module "app/client/components/commands" { export class Command { public name: string; + public deprecated: boolean; public desc: string; public humanKeys: string[]; public keys: string[]; diff --git a/app/client/models/AppModel.ts b/app/client/models/AppModel.ts index 3e668a95..40e9ad2a 100644 --- a/app/client/models/AppModel.ts +++ b/app/client/models/AppModel.ts @@ -11,7 +11,7 @@ import {Features, isLegacyPlan, Product} from 'app/common/Features'; import {GristLoadConfig} from 'app/common/gristUrls'; import {FullUser} from 'app/common/LoginSessionAPI'; import {LocalPlugin} from 'app/common/plugin'; -import {UserPrefs} from 'app/common/Prefs'; +import {DeprecationWarning, DismissedPopup, UserPrefs} from 'app/common/Prefs'; import {isOwner} from 'app/common/roles'; import {getTagManagerScript} from 'app/common/tagManager'; import {getDefaultThemePrefs, Theme, ThemeAppearance, ThemeColors, ThemePrefs, @@ -22,6 +22,7 @@ import {getOrgName, Organization, OrgError, UserAPI, UserAPIImpl} from 'app/comm import {getUserPrefObs, getUserPrefsObs} from 'app/client/models/UserPrefs'; import {bundleChanges, Computed, Disposable, Observable, subscribe} from 'grainjs'; +// Reexported for convenience. export {reportError} from 'app/client/models/errors'; export type PageType = "doc" | "home" | "billing" | "welcome"; @@ -60,8 +61,10 @@ export interface TopAppModel { fetchUsersAndOrgs(): Promise; } -// AppModel is specific to the currently loaded organization and active user. It gets rebuilt when -// we switch the current organization or the current user. +/** + * AppModel is specific to the currently loaded organization and active user. It gets rebuilt when + * we switch the current organization or the current user. + */ export interface AppModel { topAppModel: TopAppModel; api: UserAPI; @@ -83,6 +86,14 @@ export interface AppModel { userPrefsObs: Observable; themePrefs: Observable; currentTheme: Computed; + /** + * Popups that user has seen. + */ + dismissedPopups: Observable; + /** + * Deprecation messages that user has seen. + */ + deprecatedWarnings: Observable; pageType: Observable; @@ -222,6 +233,12 @@ export class AppModelImpl extends Disposable implements AppModel { }) as Observable; public readonly currentTheme = this._getCurrentThemeObs(); + public readonly dismissedPopups = + getUserPrefObs(this.userPrefsObs, 'dismissedPopups', { defaultValue: [] }) as Observable; + public readonly deprecatedWarnings = getUserPrefObs(this.userPrefsObs, 'seenDeprecatedWarnings', + { defaultValue: []}) as Observable; + + // Get the current PageType from the URL. public readonly pageType: Observable = Computed.create(this, urlState().state, (use, state) => (state.doc ? "doc" : (state.billing ? "billing" : (state.welcome ? "welcome" : "home")))); diff --git a/app/client/models/NotifyModel.ts b/app/client/models/NotifyModel.ts index 22eb5dd9..81f8b56b 100644 --- a/app/client/models/NotifyModel.ts +++ b/app/client/models/NotifyModel.ts @@ -26,7 +26,7 @@ interface INotifier { getFullAppErrors(): IAppError[]; } -interface INotification extends Expirable { +export interface INotification extends Expirable { expire(): Promise; } @@ -34,11 +34,19 @@ export interface IProgress extends Expirable { setProgress(percent: number): void; } +/** + * Custom action to be shown as a notification with a handler. + */ +export interface CustomAction { label: string, action: () => void } +/** + * A string, or a function that builds dom. + */ +export type MessageType = string | (() => DomElementArg); // Identifies supported actions. These are implemented in NotifyUI. -export type NotifyAction = 'upgrade' | 'renew' | 'personal' | 'report-problem' | 'ask-for-help'; +export type NotifyAction = 'upgrade' | 'renew' | 'personal' | 'report-problem' | 'ask-for-help' | CustomAction; export interface INotifyOptions { - message: string | (() => DomElementArg); // A string, or a function that builds dom. + message: MessageType; // A string, or a function that builds dom. timestamp?: number; title?: string; canUserClose?: boolean; @@ -224,7 +232,7 @@ export class Notifier extends Disposable implements INotifier { * Additional option level, can be used to style the notification to like a success, warning, * info or error message. */ - public createUserMessage(message: string, options: Partial = {}): INotification { + public createUserMessage(message: MessageType, options: Partial = {}): INotification { const timestamp = Date.now(); if (options.actions && options.actions.includes('ask-for-help')) { // If user should be able to ask for help, add this error to the notifier dropdown too for a diff --git a/app/client/models/errors.ts b/app/client/models/errors.ts index 7def3c36..79ce5ad0 100644 --- a/app/client/models/errors.ts +++ b/app/client/models/errors.ts @@ -1,6 +1,6 @@ import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals'; import * as log from 'app/client/lib/log'; -import {INotifyOptions, Notifier} from 'app/client/models/NotifyModel'; +import {INotification, INotifyOptions, MessageType, Notifier} from 'app/client/models/NotifyModel'; import {ApiErrorDetails} from 'app/common/ApiError'; import {fetchFromHome, pageHasHome} from 'app/common/urlUtils'; import isError = require('lodash/isError'); @@ -44,9 +44,9 @@ export function getAppErrors(): string[] { /** * Shows normal notification without any styling or icon. */ -export function reportMessage(msg: string, options?: Partial) { +export function reportMessage(msg: MessageType, options?: Partial): INotification|undefined { if (_notifier && !_notifier.isDisposed()) { - _notifier.createUserMessage(msg, { + return _notifier.createUserMessage(msg, { ...options }); } @@ -60,14 +60,14 @@ export function reportWarning(msg: string, options?: Partial) { options = {level: 'warning', ...options}; log.warn(`${options.level}: `, msg); _logError(msg); - reportMessage(msg, options); + return reportMessage(msg, options); } /** * Shows success toast notification (with green styling). */ -export function reportSuccess(msg: string, options?: Partial) { - reportMessage(msg, {level: 'success', ...options}); +export function reportSuccess(msg: MessageType, options?: Partial) { + return reportMessage(msg, {level: 'success', ...options}); } /** diff --git a/app/client/ui/App.ts b/app/client/ui/App.ts index 06d9afad..603a4c6f 100644 --- a/app/client/ui/App.ts +++ b/app/client/ui/App.ts @@ -95,7 +95,7 @@ export class App extends DisposableWithEvents { ) ), dom.forEach(commandList.groups, (group: any) => { - const cmds = group.commands.filter((cmd: any) => Boolean(cmd.desc && cmd.keys.length)); + const cmds = group.commands.filter((cmd: any) => Boolean(cmd.desc && cmd.keys.length && !cmd.deprecated)); return cmds.length > 0 ? dom('tbody', dom('tr', diff --git a/app/client/ui/NotifyUI.ts b/app/client/ui/NotifyUI.ts index 8d23dd51..c56a0630 100644 --- a/app/client/ui/NotifyUI.ts +++ b/app/client/ui/NotifyUI.ts @@ -60,6 +60,10 @@ function buildAction(action: NotifyAction, item: Notification, options: IBeaconO return cssToastAction('Ask for help', dom.on('click', () => beaconOpenMessage({...options, includeAppErrors: true, errors}))); } + + default: + return cssToastAction(action.label, testId('toast-custom-action'), + dom.on('click', action.action)); } } diff --git a/app/client/ui2018/modals.ts b/app/client/ui2018/modals.ts index 3b5caf30..b2d8f723 100644 --- a/app/client/ui2018/modals.ts +++ b/app/client/ui2018/modals.ts @@ -6,6 +6,7 @@ import {bigBasicButton, bigPrimaryButton, cssButton} from 'app/client/ui2018/but import {mediaSmall, testId, theme, vars} from 'app/client/ui2018/cssVars'; import {loadingSpinner} from 'app/client/ui2018/loaders'; import {waitGrainObs} from 'app/common/gutil'; +import {IOpenController, IPopupDomCreator, IPopupOptions, PopupControl, popupOpen} from 'popweasel'; import {Computed, Disposable, dom, DomContents, DomElementArg, input, keyframes, MultiHolder, Observable, styled} from 'grainjs'; @@ -468,8 +469,40 @@ export function cssModalWidth(style: ModalWidth) { return cssModalDialog.cls('-' + style); } +/** + * Shows a little modal as a tooltip. + * + * Example: + * dom.on('click', (_, element) => modalTooltip(element, (ctl) => { + * return dom('div', 'Hello world', dom.on('click', () => ctl.close())); + * })) + */ +export function modalTooltip( + reference: Element, + domCreator: IPopupDomCreator, + options: IPopupOptions = {} +): PopupControl { + return popupOpen(reference, (ctl: IOpenController) => { + const element = cssModalTooltip( + domCreator(ctl) + ); + return element; + }, options); +} + /* CSS styled components */ +const cssModalTooltip = styled('div', ` + padding: 16px; + background: ${theme.modalBg}; + border-radius: 3px; + box-shadow: 0 2px 18px 0 ${theme.modalInnerShadow}, 0 0 1px 0 ${theme.modalOuterShadow}; + outline: none; + & > div { + outline: none; + } +`); + // For centering, we use 'margin: auto' on the flex item instead of 'justify-content: center' on // the flex container, to ensure the full item can be scrolled in case of overflow. // See https://stackoverflow.com/a/33455342/328565 diff --git a/app/common/Prefs.ts b/app/common/Prefs.ts index be53e29c..dacf22b2 100644 --- a/app/common/Prefs.ts +++ b/app/common/Prefs.ts @@ -23,6 +23,10 @@ export interface UserPrefs extends Prefs { recordSignUpEvent?: boolean; // Theme-related preferences. theme?: ThemePrefs; + // List of deprecated warnings user have seen. + seenDeprecatedWarnings?: DeprecationWarning[]; + // List of dismissedPopups user have seen. + dismissedPopups?: DismissedPopup[]; } // A collection of preferences related to a combination of user and org. @@ -45,3 +49,25 @@ export interface UserOrgPrefs extends Prefs { } export type OrgPrefs = Prefs; + +/** + * List of all deprecated warnings that user can see and dismiss. + * All of them are marked as seen for new users in FlexServer.ts (welcomeNewUser handler). + * For now we use then to mark which keyboard shortcuts are deprecated, so those keys + * are also used in commandList.js. + */ +export const DeprecationWarning = StringUnion( + 'deprecatedInsertRowBefore', + 'deprecatedInsertRecordAfter', + 'deprecatedDeleteRecords', +); +export type DeprecationWarning = typeof DeprecationWarning.type; + +/** + * List of all popups that user can see and dismiss + */ +export const DismissedPopup = StringUnion( + 'deleteRecords', // confirmation for deleting records keyboard shortcut + 'deleteFields' // confirmation for deleting columns keyboard shortcut +); +export type DismissedPopup = typeof DismissedPopup.type; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 9da2fb2e..6c3f3be4 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -5,6 +5,7 @@ import {encodeUrl, getSlugIfNeeded, GristLoadConfig, IGristUrlState, isOrgInPath parseSubdomain, sanitizePathTail} from 'app/common/gristUrls'; import {getOrgUrlInfo} from 'app/common/gristUrls'; import {UserProfile} from 'app/common/LoginSessionAPI'; +import {DeprecationWarning} from 'app/common/Prefs'; import {tbind} from 'app/common/tbind'; import * as version from 'app/common/version'; import {ApiServer} from 'app/gen-server/ApiServer'; @@ -812,6 +813,8 @@ export class FlexServer implements GristServer { await this._dbManager.updateOrg(getScope(req), 0, {userPrefs: { showNewUserQuestions: true, recordSignUpEvent: true, + // Mark all deprecated warnings as seen. + seenDeprecatedWarnings: DeprecationWarning.values }}); const domain = mreq.org ?? null; diff --git a/test/nbrowser/ReferenceColumns.ts b/test/nbrowser/ReferenceColumns.ts index b799d193..205d247f 100644 --- a/test/nbrowser/ReferenceColumns.ts +++ b/test/nbrowser/ReferenceColumns.ts @@ -5,7 +5,6 @@ import {server, setupTestSuite} from 'test/nbrowser/testUtils'; describe('ReferenceColumns', function() { this.timeout(20000); - setupTestSuite(); let session: Session; const cleanup = setupTestSuite({team: true}); @@ -543,7 +542,8 @@ describe('ReferenceColumns', function() { // Delete a row. await gu.getCell({section: 'Colors', col: 'Color Name', rowNum: 1}).doClick(); - await driver.find('body').sendKeys(Key.chord(await gu.modKey(), '-')); + await driver.find('body').sendKeys(Key.chord(await gu.modKey(), Key.DELETE)); + await gu.confirm(true, true); await gu.waitForServer(); // See that the value is gone from the autocomplete. @@ -554,7 +554,7 @@ describe('ReferenceColumns', function() { // Add a row. await gu.getCell({section: 'Colors', col: 'Color Name', rowNum: 1}).doClick(); - await driver.find('body').sendKeys(Key.chord(await gu.modKey(), '=')); + await driver.find('body').sendKeys(Key.chord(await gu.modKey(), Key.ENTER)); await gu.waitForServer(); await driver.sendKeys('HELIOTROPE', Key.ENTER); await gu.waitForServer(); diff --git a/test/nbrowser/ReferenceList.ts b/test/nbrowser/ReferenceList.ts index 40c15614..4ab1e1e6 100644 --- a/test/nbrowser/ReferenceList.ts +++ b/test/nbrowser/ReferenceList.ts @@ -154,8 +154,7 @@ describe('ReferenceList', function() { // Now delete the row containing Avatar. await gu.getCell('Title', 4, 'Films record').doClick(); - await gu.sendKeys(Key.chord(await gu.modKey(), '-')); - await gu.waitForServer(); + await gu.removeRow(4); // Check that all references to Avatar are deleted. assert.deepEqual( @@ -840,8 +839,7 @@ describe('ReferenceList', function() { // Delete a row. await gu.getCell({section: 'Colors', col: 'Color Name', rowNum: 1}).doClick(); - await driver.find('body').sendKeys(Key.chord(await gu.modKey(), '-')); - await gu.waitForServer(); + await gu.removeRow(1); // See that the value is gone from the autocomplete. await cell.click(); @@ -851,7 +849,7 @@ describe('ReferenceList', function() { // Add a row. await gu.getCell({section: 'Colors', col: 'Color Name', rowNum: 1}).doClick(); - await driver.find('body').sendKeys(Key.chord(await gu.modKey(), '=')); + await driver.find('body').sendKeys(Key.chord(await gu.modKey(), Key.ENTER)); await gu.waitForServer(); await driver.sendKeys('HELIOTROPE', Key.ENTER); await gu.waitForServer(); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 530ce88a..4db44f3b 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -411,6 +411,30 @@ export async function getGridRowCount(): Promise { return parseInt(rowNum, 10); } +/** + * Returns the total row count in the card list that is the active section by scrolling to the bottom + * and examining the last row number. The count includes the special "Add Row". + */ +export async function getCardListCount(): Promise { + await sendKeys(Key.chord(await modKey(), Key.DOWN)); + const rowNum = await driver.find('.active.detailview_record_detail .detail_row_num').getText(); + return parseInt(rowNum, 10); +} + +/** + * Returns the total row count in the card widget that is the active section by looking + * at the displayed count in the section header. The count includes the special "Add Row". + */ +export async function getCardCount(): Promise { + const section = await driver.findWait('.active_section', 4000); + const counter = await section.findAll(".grist-single-record__menu__count"); + if (counter.length) { + const cardRow = (await counter[0].getText()).split(' OF ')[1]; + return parseInt(cardRow) + 1; + } + return 1; +} + /** * Return the .column-name element for the specified column, which may be specified by full name * or index, and may include a section (or will use the active section by default). @@ -856,6 +880,22 @@ export async function sendActions(actions: UserAction[]) { await waitForServer(); } +/** + * Confirms dialog for removing rows. In the future, can be used for other dialogs. + */ +export async function confirm(save = true, remember = false) { + if (await driver.find(".test-confirm-save").isPresent()) { + if (remember) { + await driver.find(".test-confirm-remember").click(); + } + if (save) { + await driver.find(".test-confirm-save").click(); + } else { + await driver.find(".test-confirm-cancel").click(); + } + } +} + /** * Returns the left-panel item for the given page, given by a full string name, or a RegExp. * You may simply click it to switch to that page. @@ -2472,6 +2512,15 @@ export async function scrollActiveView(x: number, y: number) { await driver.sleep(10); // wait a bit for the scroll to happen (this is async operation in Grist). } +export async function scrollActiveViewTop() { + await driver.executeScript(function() { + const view = document.querySelector(".active_section .grid_view_data") || + document.querySelector(".active_section .detailview_scroll_pane"); + view!.scrollTop = 0; + }); + await driver.sleep(10); // wait a bit for the scroll to happen (this is async operation in Grist). +} + /** * Filters a column in a Grid using the filter menu. */ diff --git a/test/nbrowser/testUtils.ts b/test/nbrowser/testUtils.ts index 37caa780..1b5a0ff9 100644 --- a/test/nbrowser/testUtils.ts +++ b/test/nbrowser/testUtils.ts @@ -88,7 +88,18 @@ export function setupTestSuite(options?: TestSuiteOptions) { // Also, log out, to avoid logins interacting, unless NO_CLEANUP is requested (useful for // debugging tests). if (!process.env.NO_CLEANUP) { - after(() => server.removeLogin()); + after(async () => { + try { + await server.removeLogin(); + } catch(err) { + // If there are any alerts open, close them as it might be blocking other tests. + if (err.name && err.name === 'UnexpectedAlertOpenError') { + await driver.switchTo().alert().accept(); + assert.fail("Unexpected alert open"); + } + throw err; + } + }); } // If requested, clear user preferences for all test users after this suite.