From 196ab6c473a46a4a404e4125eb102ffe00ec008f Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Mon, 7 Feb 2022 15:09:23 +0100 Subject: [PATCH] (core) Adds cell context menu Summary: - Brings in a new utility `contextMenu` to open context menu next to the mouse position - Use this utility to show a CellContextMenu, that sort of merge cell context menu and column context menu together. - Show cell context menu on context click on any grid's cell. - Also takes care of showing the row context menu for detail view on a context click that occurs on cells and not only on the row num header as it was the case prior to this diff. - task: https://gristlabs.getgrist.com/doc/check-ins/p/5#a1.s9.r1529.c31 - discussion: https://grist.quip.com/ETGkAroLnc0Y/Cell-Context-Menu {F40092} Test Plan: - Adds project test and nbrowser for cell context menu and new cases for the detail row context menu. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D3237 --- app/client/components/GridView.js | 79 ++++++++++++++++++--- app/client/components/RecordLayout.js | 5 ++ app/client/ui/AppUI.ts | 5 +- app/client/ui/CellContextMenu.ts | 85 +++++++++++++++++++++++ app/client/ui/GridViewMenus.ts | 2 +- app/client/ui/RowContextMenu.ts | 1 + app/client/ui/contextMenu.ts | 98 +++++++++++++++++++++++++++ app/client/ui2018/menus.ts | 2 +- test/nbrowser/gristUtils.ts | 6 +- 9 files changed, 271 insertions(+), 12 deletions(-) create mode 100644 app/client/ui/CellContextMenu.ts create mode 100644 app/client/ui/contextMenu.ts diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index 14852f94..bce11e1c 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -28,14 +28,16 @@ const {reportError} = require('app/client/models/AppModel'); const {onDblClickMatchElem} = require('app/client/lib/dblclick'); // Grist UI Components -const {Holder, Computed} = require('grainjs'); +const {dom: grainjsDom, Holder, Computed} = require('grainjs'); const {menu} = require('../ui2018/menus'); const {calcFieldsCondition} = require('../ui/GridViewMenus'); const {ColumnAddMenu, ColumnContextMenu, MultiColumnMenu, freezeAction} = require('../ui/GridViewMenus'); const {RowContextMenu} = require('../ui/RowContextMenu'); const {setPopupToCreateDom} = require('popweasel'); +const {CellContextMenu} = require('app/client/ui/CellContextMenu'); const {testId} = require('app/client/ui2018/cssVars'); +const {contextMenu} = require('app/client/ui/contextMenu'); const {menuToggle} = require('app/client/ui/MenuToggle'); const {showTooltip} = require('app/client/ui/tooltips'); @@ -76,6 +78,7 @@ function GridView(gristDoc, viewSectionModel, isPreview = false) { this.cellSelector = this.autoDispose(selector.CellSelector.create(this, { // This is a bit of a hack to prevent dragging when there's an open column menu + // TODO: disable dragging when there is an open cell context menu as well isDisabled: () => Boolean(!this.ctxMenuHolder.isEmpty()) })); this.colMenuTargets = {}; // Reference from column ref to its menu target dom @@ -603,6 +606,26 @@ GridView.prototype.assignCursor = function(elem, elemType) { this.cellSelector.currentSelectType(elemType); }; +/** + * Schedules cursor assignement to happen at end of tick. Calling `preventAssignCursor()` before + * prevents assignment to happen. This was added to prevent cursor assignment on a `context click` + * on a cell that is already selected. + */ +GridView.prototype.scheduleAssignCursor = function(elem, elemType) { + this._assignCursorTimeoutId = setTimeout(() => { + this.assignCursor(elem, elemType); + this._assignCursorTimeoutId = null; + }, 0); +} + +/** + * See `scheduleAssignCursor()` for doc. + */ +GridView.prototype.preventAssignCursor = function() { + clearTimeout(this._assignCursorTimeoutId); + this._assignCursorTimeoutId = null; +} + GridView.prototype.deleteRows = function(selection) { if (!this.viewSection.disableAddRemoveRows()) { var rowIds = _.without(selection.rowIds, 'new'); @@ -1067,11 +1090,10 @@ GridView.prototype.buildDom = function() { }), self.isPreview ? null : menuToggle(null, dom.on('click', ev => self.maybeSelectRow(ev.currentTarget.parentNode, row.getRowId())), - menu(() => RowContextMenu({ - disableInsert: Boolean(self.gristDoc.isReadonly.get() || self.viewSection.disableAddRemoveRows() || self.tableModel.tableMetaRow.onDemand()), - disableDelete: Boolean(self.gristDoc.isReadonly.get() || self.viewSection.disableAddRemoveRows() || self.getSelection().onlyAddRowSelected()), - isViewSorted: self.viewSection.activeSortSpec.peek().length > 0, - }), { trigger: ['click'] }), + menu((ctx) => { + ctx.autoDispose(isRowActive.subscribe(() => ctx.close())); + return self.rowContextMenu(); + }, { trigger: ['click'] }), // Prevent mousedown on the dropdown triangle from initiating row drag. dom.on('mousedown', () => false), testId('row-menu-trigger'), @@ -1095,6 +1117,13 @@ GridView.prototype.buildDom = function() { self.changeHover(-1); } }), + contextMenu((ctx) => { + // We need to close the menu when the row is removed, but the dom of the row is not + // disposed when the record is removed (this is probably due to how scrolly work). Hence, + // we need to subscribe to `isRowActive` to close the menu. + ctx.autoDispose(isRowActive.subscribe(() => ctx.close())); + return self.cellContextMenu(); + }), self.comparison ? kd.cssClass(() => { const rowType = self.extraRows.getRowType(row.id()); return rowType && `diff-${rowType}` || ''; @@ -1139,7 +1168,21 @@ GridView.prototype.buildDom = function() { kd.style('borderRightWidth', v.borderWidthPx), kd.toggleClass('selected', isSelected), - fieldBuilder.buildDomWithCursor(row, isCellActive, isCellSelected) + fieldBuilder.buildDomWithCursor(row, isCellActive, isCellSelected), + + grainjsDom.on('contextmenu', (ev, elem) => { + let row = self.domToRowModel(elem, selector.CELL); + let col = self.domToColModel(elem, selector.CELL); + + if (self.cellSelector.containsCell(row._index(), col._index())) { + // contextmenu event could be preceded by a mousedown event (ie: when ctrl+click on + // mac) which triggers a cursor assignment that we need to prevent. + self.preventAssignCursor(); + } else { + self.assignCursor(elem, selector.NONE); + } + }) + ); }) ) @@ -1283,7 +1326,7 @@ GridView.prototype.attachSelectorHandlers = function () { }; var cellCallbacks = { 'mousedown': { 'select': this.cellMouseDown, - 'drag' : function(elem) { this.assignCursor(elem, selector.NONE); }, + 'drag' : function(elem) { this.scheduleAssignCursor(elem, selector.NONE); }, 'elemName': '.field:not(.column_name)', 'source': this.scrollPane }, @@ -1483,6 +1526,26 @@ GridView.prototype.maybeSelectRow = function(elem, rowId) { } }; +GridView.prototype.rowContextMenu = function() { + return RowContextMenu(this._getRowContextMenuOptions()); +}; + +GridView.prototype._getRowContextMenuOptions = function() { + return { + disableInsert: Boolean(this.gristDoc.isReadonly.get() || this.viewSection.disableAddRemoveRows() || this.tableModel.tableMetaRow.onDemand()), + disableDelete: Boolean(this.gristDoc.isReadonly.get() || this.viewSection.disableAddRemoveRows() || this.getSelection().onlyAddRowSelected()), + isViewSorted: this.viewSection.activeSortSpec.peek().length > 0, + numRows: this.getSelection().rowIds.length + }; +}; + +GridView.prototype.cellContextMenu = function() { + return CellContextMenu( + this._getRowContextMenuOptions(), + this._getColumnMenuOptions(this.getSelection()) + ); +}; + // End Context Menus GridView.prototype.scrollToCursor = function(sync = true) { diff --git a/app/client/components/RecordLayout.js b/app/client/components/RecordLayout.js index 49a03581..4fe92c1e 100644 --- a/app/client/components/RecordLayout.js +++ b/app/client/components/RecordLayout.js @@ -38,6 +38,7 @@ var commands = require('./commands'); var {menuToggle} = require('app/client/ui/MenuToggle'); var {menu} = require('../ui2018/menus'); var {testId} = require('app/client/ui2018/cssVars'); +var {contextMenu} = require('app/client/ui/contextMenu'); /** * Construct a RecordLayout. @@ -334,12 +335,16 @@ RecordLayout.prototype.buildLayoutDom = function(row, optCreateEditor) { this.layoutEditor.peek().dispose(); this.layoutEditor(null); }) : null, + // enables row context menu anywhere on the card + contextMenu(() => this.buildContextMenu(row)), dom('div.detail_row_num', kd.text(() => (row._index() + 1)), dom.on('contextmenu', ev => { // This is a little hack to position the menu the same way as with a click, // the same hack as on a column menu. ev.preventDefault(); + // prevent 2nd context menu to show up + ev.stopPropagation(); ev.currentTarget.querySelector('.menu_toggle').click(); }), menuToggle(null, diff --git a/app/client/ui/AppUI.ts b/app/client/ui/AppUI.ts index dc03d873..669161ee 100644 --- a/app/client/ui/AppUI.ts +++ b/app/client/ui/AppUI.ts @@ -30,7 +30,10 @@ export function createAppUI(topAppModel: TopAppModel, appObj: App): IDisposable dom.update(document.body, content, { // Cancel out bootstrap's overrides. style: 'font-family: inherit; font-size: inherit; line-height: inherit;' - }); + }, + // prevent default context menu to show + dom.on('contextmenu', (ev) => ev.preventDefault()) + ); function dispose() { // Return value of dom.maybe() / dom.domComputed() is a pair of markers with a function that diff --git a/app/client/ui/CellContextMenu.ts b/app/client/ui/CellContextMenu.ts new file mode 100644 index 00000000..63359777 --- /dev/null +++ b/app/client/ui/CellContextMenu.ts @@ -0,0 +1,85 @@ +import { allCommands } from 'app/client/components/commands'; +import { menuDivider, menuItemCmd } from 'app/client/ui2018/menus'; +import { dom } from 'grainjs'; +import { IMultiColumnContextMenu } from 'app/client/ui/GridViewMenus'; + +interface IRowContextMenu { + disableInsert: boolean; + disableDelete: boolean; + isViewSorted: boolean; + numRows: number; +} + +export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiColumnContextMenu) { + + const { disableInsert, disableDelete, isViewSorted } = rowOptions; + const { disableModify, isReadonly } = colOptions; + + const disableForReadonlyColumn = dom.cls('disabled', Boolean(disableModify) || isReadonly); + const disableForReadonlyView = dom.cls('disabled', isReadonly); + + const numCols: number = colOptions.numColumns; + const nameClearColumns = colOptions.isFiltered ? + (numCols > 1 ? `Clear ${numCols} entire columns` : 'Clear entire column') : + (numCols > 1 ? `Clear ${numCols} columns` : 'Clear column'); + const nameDeleteColumns = numCols > 1 ? `Delete ${numCols} columns` : 'Delete column'; + + const numRows: number = rowOptions.numRows; + const nameDeleteRows = numRows > 1 ? `Delete ${numRows} rows` : 'Delete row'; + + const nameClearCells = (numRows > 1 || numCols > 1) ? 'Clear values' : 'Clear cell'; + + const result: Array = []; + + result.push( + + // TODO: implement copy/paste actions + + colOptions.isFormula ? + null : + menuItemCmd(allCommands.clearValues, nameClearCells, disableForReadonlyColumn), + menuItemCmd(allCommands.clearColumns, nameClearColumns, disableForReadonlyColumn), + + ...( + (numCols > 1 || numRows > 1) ? [] : [ + menuDivider(), + menuItemCmd(allCommands.copyLink, 'Copy anchor link') + ] + ), + + menuDivider(), + + // inserts + ...( + isViewSorted ? + // When the view is sorted, any newly added records get shifts instantly at the top or + // bottom. It could be very confusing for users who might expect the record to stay above or + // below the active row. Thus in this case we show a single `insert row` command. + [menuItemCmd(allCommands.insertRecordAfter, 'Insert row', + dom.cls('disabled', disableInsert))] : + + [menuItemCmd(allCommands.insertRecordBefore, 'Insert row above', + dom.cls('disabled', disableInsert)), + menuItemCmd(allCommands.insertRecordAfter, 'Insert row below', + dom.cls('disabled', disableInsert))] + ), + + menuItemCmd(allCommands.insertFieldBefore, 'Insert column to the left', + disableForReadonlyView), + menuItemCmd(allCommands.insertFieldAfter, 'Insert column to the right', + disableForReadonlyView), + + + menuDivider(), + + // deletes + menuItemCmd(allCommands.deleteRecords, nameDeleteRows, + dom.cls('disabled', disableDelete)), + + menuItemCmd(allCommands.deleteFields, nameDeleteColumns, disableForReadonlyColumn), + + // todo: add "hide N columns" + ); + + return result; +} diff --git a/app/client/ui/GridViewMenus.ts b/app/client/ui/GridViewMenus.ts index 3554bb9a..2a416d07 100644 --- a/app/client/ui/GridViewMenus.ts +++ b/app/client/ui/GridViewMenus.ts @@ -32,7 +32,7 @@ export function ColumnAddMenu(gridView: IView, viewSection: IViewSection) { }, `Show column ${col.label()}`)) ]; } -interface IMultiColumnContextMenu { +export 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; diff --git a/app/client/ui/RowContextMenu.ts b/app/client/ui/RowContextMenu.ts index 97fb33f3..84620327 100644 --- a/app/client/ui/RowContextMenu.ts +++ b/app/client/ui/RowContextMenu.ts @@ -28,6 +28,7 @@ export function RowContextMenu({ disableInsert, disableDelete, isViewSorted }: I } result.push( menuDivider(), + // TODO: should show `Delete ${num} rows` when multiple are selected menuItemCmd(allCommands.deleteRecords, 'Delete', dom.cls('disabled', disableDelete)), ); diff --git a/app/client/ui/contextMenu.ts b/app/client/ui/contextMenu.ts new file mode 100644 index 00000000..5717bb18 --- /dev/null +++ b/app/client/ui/contextMenu.ts @@ -0,0 +1,98 @@ +/** + * This module implements context menu to be shown on contextmenu event (most commonly associated + * with right+click, but could varies slightly depending on platform, ie: mac support ctrl+click as + * well). + * + * To prevent the default context menu to show everywhere else (including on the top of your custom + * context menu) dont forget to prevent it by including below line at the root of the dom: + * `dom.on('contextmenu', ev => ev.preventDefault())` + */ +import { Disposable, dom, DomArg, DomContents } from "grainjs"; +import { cssMenuElem } from 'app/client/ui2018/menus'; +import { IOpenController, Menu } from 'popweasel'; + +export type IContextMenuContentFunc = (ctx: ContextMenuController) => DomContents; + +class ContextMenuController extends Disposable implements IOpenController { + private _content: HTMLElement; + constructor(private _event: MouseEvent, contentFunc: IContextMenuContentFunc) { + super(); + + setTimeout(() => this._updatePosition(), 0); + + // Create content and add to the dom but keep hidden until menu gets positioned + const menu = Menu.create(null, this, [contentFunc(this)], { + menuCssClass: cssMenuElem.className + ' grist-floating-menu' + }); + const content = this._content = menu.content; + content.style.visibility = 'hidden'; + document.body.appendChild(content); + + // Prevents arrow to move the cursor while menu is open. + dom.onKeyElem(content, 'keydown', { + ArrowLeft: (ev) => ev.stopPropagation(), + ArrowRight: (ev) => ev.stopPropagation() + // UP and DOWN are already handle by the menu to navigate the menu) + }); + + // On click anywhere on the page (outside popup content), close it. + const onClick = (evt: MouseEvent) => { + const target: Node|null = evt.target as Node; + if (target && !content.contains(target)) { + this.close(); + } + }; + this.autoDispose(dom.onElem(document, 'contextmenu', onClick, {useCapture: true})); + this.autoDispose(dom.onElem(document, 'click', onClick, {useCapture: true})); + + // Cleanup involves removing the element. + this.onDispose(() => { + dom.domDispose(content); + content.remove(); + }); + } + + public close() { + this.dispose(); + } + public setOpenClass() {} + + // IOpenController expects a trigger elem but context menu has no trigger. Let's return body for + // now. As of time of writing the trigger elem is only used by popweasel when certain options are + // enabled, ie: strectToSelector, parentSelectoToMark. + // TODO: make a PR on popweasel to support using Menu with no trigger element. + public getTriggerElem() { return document.body; } + public update() {} + + private _updatePosition() { + const content = this._content; + const ev = this._event; + const rect = content.getBoundingClientRect(); + // position menu on the right of the cursor if it can fit, on the left otherwise + content.style.left = ((ev.pageX + rect.width < window.innerWidth) ? ev.pageX : ev.pageX - rect.width) + 'px'; + // position menu below the cursor if it can fit, otherwise fit at the bottom of the screen + content.style.bottom = Math.max(window.innerHeight - (ev.pageY + rect.height), 0) + 'px'; + // show content + content.style.visibility = ''; + } +} + +/** + * Show the return value of contentFunc() in a context menu next to the mouse. + */ +function showContextMenu(ev: MouseEvent, contentFunc: IContextMenuContentFunc) { + return ContextMenuController.create(null, ev, contentFunc); +} + +/** + * Show a context menu on contextmenu. + */ +export function contextMenu(contentFunc: IContextMenuContentFunc): DomArg { + return (elem) => { + dom.onElem(elem, 'contextmenu', (ev) => { + ev.preventDefault(); + ev.stopPropagation(); + dom.autoDisposeElem(elem, showContextMenu(ev, contentFunc)); + }); + }; +} diff --git a/app/client/ui2018/menus.ts b/app/client/ui2018/menus.ts index 76a6c863..c5586f90 100644 --- a/app/client/ui2018/menus.ts +++ b/app/client/ui2018/menus.ts @@ -35,7 +35,7 @@ export function menuItemSubmenu( return weasel.menuItemSubmenu(submenu, {...defaults, ...options}, ...args); } -const cssMenuElem = styled('div', ` +export const cssMenuElem = styled('div', ` font-family: ${vars.fontFamily}; font-size: ${vars.mediumFontSize}; line-height: initial; diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index fab2813a..3634d7d5 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -46,7 +46,7 @@ export const checkLoginPage = homeUtil.checkLoginPage.bind(homeUtil); export const fixturesRoot: string = testUtils.fixturesRoot; // it is sometimes useful in debugging to turn off automatic cleanup of docs and workspaces. -const noCleanup = Boolean(process.env.NO_CLEANUP); +export const noCleanup = Boolean(process.env.NO_CLEANUP); // Most test code uses simulateLogin through the server reference. Keep them to reduce unnecessary // code changes. @@ -372,6 +372,10 @@ export async function dbClick(cell: WebElement) { await driver.withActions(a => a.doubleClick(cell)); } +export async function rightClick(cell: WebElement) { + await driver.withActions((actions) => actions.contextClick(cell)); +} + /** * Returns {rowNum, col} object representing the position of the cursor in the active view * section. RowNum is a 1-based number as in the row headers, and col is a 0-based index for