From 7cc3092e1b04ccd0509195251ddd5c3d8cb60e03 Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Tue, 8 Feb 2022 10:26:29 +0100 Subject: [PATCH] (core) Restore default context menu out of std views Summary: Also fix few clode glitches - Attach events handler to the scrollPane using `onMatch` instead of the cell itself. This streamline cell dom creation a bit while scrolling. - Fix memory leaks on contextMenu. - Also fix Importer and ColumnOps nbrowser test see: https://phab.getgrist.com/D3237#inline-36376 https://phab.getgrist.com/D3237#inline-36375 Restore default context menu for where there's no custom one It appears that default context menu adds some value, in particular for links and for editing text. This diff restores it. Test Plan: Should not break anything. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3256 --- app/client/components/GridView.js | 32 ++++++++++++++++--------------- app/client/ui/AppUI.ts | 5 +---- app/client/ui/contextMenu.ts | 13 ++++--------- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index ba9a522f..11c6babc 100644 --- a/app/client/components/GridView.js +++ b/app/client/components/GridView.js @@ -224,6 +224,9 @@ function GridView(gristDoc, viewSectionModel, isPreview = false) { //-------------------------------------------------- // Set up DOM event handling. onDblClickMatchElem(this.scrollPane, '.field', () => this.activateEditorAtCursor()); + if (!this.isPreview) { + grainjsDom.onMatchElem(this.scrollPane, '.field:not(.column_name)', 'contextmenu', (ev, elem) => this.onCellContextMenu(ev, elem), {useCapture: true}); + } this.onEvent(this.scrollPane, 'scroll', this.onScroll); //-------------------------------------------------- @@ -1117,7 +1120,7 @@ GridView.prototype.buildDom = function() { self.changeHover(-1); } }), - contextMenu((ctx) => { + self.isPreview ? null : 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. @@ -1169,20 +1172,6 @@ GridView.prototype.buildDom = function() { kd.toggleClass('selected', isSelected), 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); - } - }) - ); }) ) @@ -1215,6 +1204,19 @@ GridView.prototype.onLinkFilterChange = function(rowId) { this.clearSelection(); }; +GridView.prototype.onCellContextMenu = function(ev, elem) { + let row = this.domToRowModel(elem, selector.CELL); + let col = this.domToColModel(elem, selector.CELL); + + if (this.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. + this.preventAssignCursor(); + } else { + this.assignCursor(elem, selector.NONE); + } +} + // ====================================================================================== // SELECTOR STUFF diff --git a/app/client/ui/AppUI.ts b/app/client/ui/AppUI.ts index 669161ee..dc03d873 100644 --- a/app/client/ui/AppUI.ts +++ b/app/client/ui/AppUI.ts @@ -30,10 +30,7 @@ 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/contextMenu.ts b/app/client/ui/contextMenu.ts index 5717bb18..cf27403b 100644 --- a/app/client/ui/contextMenu.ts +++ b/app/client/ui/contextMenu.ts @@ -7,7 +7,7 @@ * 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 { Disposable, dom, DomArg, DomContents, Holder } from "grainjs"; import { cssMenuElem } from 'app/client/ui2018/menus'; import { IOpenController, Menu } from 'popweasel'; @@ -77,22 +77,17 @@ class ContextMenuController extends Disposable implements IOpenController { } } -/** - * 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) => { + const holder = Holder.create(null); + dom.autoDisposeElem(elem, holder); dom.onElem(elem, 'contextmenu', (ev) => { ev.preventDefault(); ev.stopPropagation(); - dom.autoDisposeElem(elem, showContextMenu(ev, contentFunc)); + ContextMenuController.create(holder, ev, contentFunc); }); }; }