(kb) differenciate global view commands from focused-only commands

sometimes, we want to run "current view"-related commands via keyboard
shortcuts, while we are focusing ie. the creator panel.

this commit makes it possible, by distinguishing "globally active"
view commands from "only user-focused" view commands.

in order to have the "globally active" commands more reliably work, we
make sure the active section is focused by the region switcher
automatically. it's not an ideal behavior for the user, as he will then
need to manually go focus back the panel he was on ; but this is more
done as a safety measure to not end up in a state where `body` is
focused while we'd expect otherwise.

Author: manuhabitela

app/client/components/BaseView.js

@@ -34,6 +34,7 @@ const {markAsSeen} = require('app/client/models/UserPrefs');
 const {buildConfirmDelete, reportUndo} = require('app/client/components/modals');
 const {buildReassignModal} = require('app/client/ui/buildReassignModal');
 const {MutedError} = require('app/client/models/errors');
+const {viewCommands} = require('app/client/components/RegionFocusSwitcher');
 
 
 /**
@@ -128,7 +129,8 @@ function BaseView(gristDoc, viewSectionModel, options) {
   this.listenTo(this.viewSection.events, 'rowHeightChange', this.onResize );
 
   // Create a command group for keyboard shortcuts common to all views.
-  this.autoDispose(commands.createGroup(BaseView.commonCommands, this, this.viewSection.enableCommands));
+  this.autoDispose(commands.createGroup(viewCommands(BaseView.commonCommands, this), this, this.viewSection.hasFocus));
+  this.autoDispose(commands.createGroup(BaseView.commonFocusedCommands, this, this.viewSection.hasRegionFocus));
 
   //--------------------------------------------------
   // Prepare logic for linking with other sections.
@@ -233,34 +235,43 @@ _.extend(Base.prototype, BackboneEvents);
 
 /**
  * These commands are common to GridView and DetailView.
+ *
+ * They work when the view is the currently active one.
  */
 BaseView.commonCommands = {
   input: function(init) {
     this.scrollToCursor(true).catch(reportError);
     this.activateEditorAtCursor({init});
   },
-  editField: function(event) { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor({event}); },
-
+  copyLink: function() { this.copyLink().catch(reportError); },
+  filterByThisCellValue: function() { this.filterByThisCellValue(); },
+  duplicateRows: function() { this._duplicateRows().catch(reportError); },
+  openDiscussion: function() { this.openDiscussionAtCursor(); },
   insertRecordBefore: function() { this.insertRow(this.cursor.rowIndex()); },
   insertRecordAfter: function() { this.insertRow(this.cursor.rowIndex() + 1); },
+};
+
+/**
+ * These commands are common to GridView and DetailView.
+ *
+ * They are enabled only when the user is actually focusing the view, meaning
+ * they don't work when the view is the active one but the user is focused on the creator panel.
+ */
+BaseView.commonFocusedCommands = {
+  editField: function(event) { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor({event}); },
 
   insertCurrentDate: function() { this.insertCurrentDate(false); },
   insertCurrentDateTime: function() { this.insertCurrentDate(true); },
 
-  copyLink: function() { this.copyLink().catch(reportError); },
-
   deleteRecords: function(source) { this.deleteRecords(source); },
 
-  filterByThisCellValue: function() { this.filterByThisCellValue(); },
-  duplicateRows: function() { this._duplicateRows().catch(reportError); },
-  openDiscussion: function() { this.openDiscussionAtCursor(); },
   viewAsCard: function() {
     /* Overridden by subclasses.
      *
      * This is still needed so that <space> doesn't trigger the `input` command
      * if a subclass doesn't support opening the current record as a card. */
   },
-};
+}
 
 BaseView.prototype.selectedRows = function() {
   return [];
@@ -860,4 +871,5 @@ BaseView.prototype.isRecordCardDisabled = function() {
   return this.viewSection.isTableRecordCardDisabled();
 }
 
+
 module.exports = BaseView;

app/client/components/Cursor.ts

@@ -112,7 +112,7 @@ export class Cursor extends Disposable {
 
     this.fieldIndex = baseView.viewSection.viewFields().makeLiveIndex(optCursorPos.fieldIndex || 0);
 
-    this.autoDispose(commands.createGroup(Cursor.editorCommands, this, baseView.viewSection.enableCommands));
+    this.autoDispose(commands.createGroup(Cursor.editorCommands, this, baseView.viewSection.hasRegionFocus));
 
     // RowId might diverge from the one stored in _rowId when the data changes (it is filtered out). So here
     // we will calculate rowId based on rowIndex (so in reverse order), to have a proper value.

app/client/components/CustomView.ts

@@ -129,7 +129,7 @@ export class CustomView extends Disposable {
 
     this.autoDispose(this.customDef.pluginId.subscribe(this._updatePluginInstance, this));
     this.autoDispose(this.customDef.sectionId.subscribe(this._updateCustomSection, this));
-    this.autoDispose(commands.createGroup(CustomView._commands, this, this.viewSection.enableCommands));
+    this.autoDispose(commands.createGroup(CustomView._commands, this, this.viewSection.hasRegionFocus));
 
     this._unmappedColumns = this.autoDispose(ko.pureComputed(() => {
       const columns = this.viewSection.columnsToMap();

app/client/components/DetailView.js

@@ -18,6 +18,7 @@ const selector      = require('./CellSelector');
 const {CopySelection} = require('./CopySelection');
 const RecordLayout  = require('./RecordLayout');
 const commands      = require('./commands');
+const {viewCommands} = require('./RegionFocusSwitcher');
 const tableUtil     = require('../lib/tableUtil');
 const {CardContextMenu} = require('../ui/CardContextMenu');
 const {FieldContextMenu} = require('../ui/FieldContextMenu');
@@ -127,8 +128,8 @@ function DetailView(gristDoc, viewSectionModel) {
 
   //--------------------------------------------------
   // Instantiate CommandGroups for the different modes.
-  this.autoDispose(commands.createGroup(DetailView.generalCommands, this, this.viewSection.enableCommands));
-  this.autoDispose(commands.createGroup(DetailView.fieldCommands, this, this.viewSection.enableCommands));
+  this.autoDispose(commands.createGroup(viewCommands(DetailView.detailCommands, this), this, this.viewSection.hasFocus));
+  this.autoDispose(commands.createGroup(DetailView.detailFocusedCommands, this, this.viewSection.hasRegionFocus));
   const hasSelection = this.autoDispose(ko.pureComputed(() =>
     !this.cellSelector.isCurrentSelectType('') || this.copySelection()));
   this.autoDispose(commands.createGroup(DetailView.selectionCommands, this, hasSelection));
@@ -154,31 +155,33 @@ DetailView.prototype._updateFloatingRow = function() {
 };
 
 /**
- * DetailView commands.
+ * DetailView commands, enabled when the view is the active one.
  */
-DetailView.generalCommands = {
-  cursorUp: function() { this.cursor.fieldIndex(this.cursor.fieldIndex() - 1); },
-  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); },
-  copy: function() { return this.copy(this.getSelection()); },
-  cut: function() { return this.cut(this.getSelection()); },
-  paste: function(pasteObj, cutCallback) {
-    return this.gristDoc.docData.bundleActions(null, () => this.paste(pasteObj, cutCallback));
-  },
-
+DetailView.detailCommands = {
   editLayout: function() {
     if (this.scrolly()) {
       this.scrolly().scrollRowIntoView(this.cursor.rowIndex());
     }
     this.recordLayout.editLayout(this.cursor.rowIndex());
   },
+  hideCardFields: function() { this._hideCardFields(); },
+  copy: function() { return this.copy(this.getSelection()); },
+  cut: function() { return this.cut(this.getSelection()); },
+  paste: function(pasteObj, cutCallback) {
+    return this.gristDoc.docData.bundleActions(null, () => this.paste(pasteObj, cutCallback));
+  },
 };
 
-DetailView.fieldCommands = {
+/**
+ * DetailView commands, enabled when the view is user-focused.
+ */
+DetailView.detailFocusedCommands = {
+  cursorUp: function() { this.cursor.fieldIndex(this.cursor.fieldIndex() - 1); },
+  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); },
   clearValues: function() { this._clearCardFields(); },
-  hideCardFields: function() { this._hideCardFields(); },
-};
+}
 
 DetailView.selectionCommands = {
   clearCopySelection: function() { this._clearCopySelection(); },

app/client/components/Forms/FormView.ts

@@ -348,7 +348,7 @@ export class FormView extends Disposable {
       editField: keyboardActions.edit,
       deleteFields: keyboardActions.clearValues,
       hideFields: keyboardActions.hideFields,
-    }, this, this.viewSection.enableCommands));
+    }, this, this.viewSection.hasRegionFocus));
 
     this._previewUrl = Computed.create(this, use => {
       const doc = use(this.gristDoc.docPageModel.currentDoc);

app/client/components/GridView.js

@@ -16,6 +16,7 @@ const tableUtil     = require('../lib/tableUtil');
 const {addToSort, sortBy}   = require('../lib/sortUtil');
 
 const commands      = require('./commands');
+const {viewCommands} = require('./RegionFocusSwitcher');
 const viewCommon    = require('./viewCommon');
 const Base          = require('./Base');
 const BaseView      = require('./BaseView');
@@ -266,8 +267,10 @@ function GridView(gristDoc, viewSectionModel, isPreview = false) {
   this.onEvent(this.scrollPane, 'scroll', this.onScroll);
 
   //--------------------------------------------------
-  // Command group implementing all grid level commands (except cancel)
-  this.autoDispose(commands.createGroup(GridView.gridCommands, this, this.viewSection.enableCommands));
+  // Command groups implementing all grid level commands (except cancel)
+  this.autoDispose(commands.createGroup(viewCommands(GridView.gridCommands, this), this, this.viewSection.hasFocus));
+  this.autoDispose(commands.createGroup(GridView.gridFocusedCommands, this, this.viewSection.hasRegionFocus));
+
   // Cancel command is registered conditionally, only when there is an active
   // cell selection. This command is also used by Raw Data Views, to close the Grid popup.
   const hasSelection = this.autoDispose(ko.pureComputed(() =>
@@ -295,6 +298,56 @@ GridView.selectionCommands = {
 // TODO: move commands with modifications to gridEditCommands and use a single guard for
 // readonly state.
 GridView.gridCommands = {
+  fillSelectionDown: function() { tableUtil.fillSelectionDown(this.getSelection(), this.tableModel); },
+  selectAll: function() { this.selectAll(); },
+  insertFieldBefore: function(event) { this._insertField(event, this.cursor.fieldIndex()); },
+  insertFieldAfter: function(event) { this._insertField(event, this.cursor.fieldIndex() + 1); },
+  makeHeadersFromRow: function() { this.makeHeadersFromRow(this.getSelection()); },
+  renameField: function() { this.renameColumn(this.cursor.fieldIndex()); },
+  hideFields: function() { this.hideFields(this.getSelection()); },
+  deleteFields: function() { this._deleteFields(); },
+  clearColumns: function() { this._clearColumns(this.getSelection()); },
+  convertFormulasToData: function() { this._convertFormulasToData(this.getSelection()); },
+  sortAsc: function() {
+    sortBy(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.ASC);
+  },
+  sortDesc: function() {
+    sortBy(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.DESC);
+  },
+  addSortAsc: function() {
+    addToSort(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.ASC);
+  },
+  addSortDesc: function() {
+    addToSort(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.DESC);
+  },
+  toggleFreeze: function() {
+    // get column selection
+    const selection = this.getSelection();
+    // convert it menu option
+    const options = this._getColumnMenuOptions(selection);
+    // generate action that is available for freeze toggle
+    const action = freezeAction(options);
+    // if no action, do nothing
+    if (!action) { return; }
+    // if grist document is in readonly - simply change the value
+    // without saving
+    if (this.isReadonly) {
+      this.viewSection.rawNumFrozen(action.numFrozen);
+      return;
+    }
+    this.viewSection.rawNumFrozen.setAndSave(action.numFrozen);
+  },
+  copy: function() { console.log(this.getSelection()); return this.copy(this.getSelection()); },
+  cut: function() { return this.cut(this.getSelection()); },
+  paste: async function(pasteObj, cutCallback) {
+    if (this.gristDoc.isReadonly.get()) { return; }
+    await this.gristDoc.docData.bundleActions(null, () => this.paste(pasteObj, cutCallback));
+    await this.scrollToCursor(false);
+  },
+};
+
+// These commands are enabled only when the grid is the user-focused region.
+GridView.gridFocusedCommands = {
   cursorDown: function() {
     if (this.cursor.rowIndex() === this.viewData.peekLength - 1) {
       // When the cursor is in the bottom row, the view may not be scrolled all the way to
@@ -335,58 +388,10 @@ GridView.gridCommands = {
   ctrlShiftUp: function () { this._shiftSelectUntilFirstOrLastNonEmptyCell({direction: 'up'}); },
   ctrlShiftRight: function () { this._shiftSelectUntilFirstOrLastNonEmptyCell({direction: 'right'}); },
   ctrlShiftLeft: function () { this._shiftSelectUntilFirstOrLastNonEmptyCell({direction: 'left'}); },
-  fillSelectionDown: function() { tableUtil.fillSelectionDown(this.getSelection(), this.tableModel); },
-
-  selectAll: function() { this.selectAll(); },
-
   fieldEditSave: function() { this.cursor.rowIndex(this.cursor.rowIndex() + 1); },
   // Re-define editField after fieldEditSave to make it take precedence for the Enter key.
   editField: function(event) { closeRegisteredMenu(); this.scrollToCursor(true); this.activateEditorAtCursor({event}); },
-  insertFieldBefore: function(event) { this._insertField(event, this.cursor.fieldIndex()); },
-  insertFieldAfter: function(event) { this._insertField(event, this.cursor.fieldIndex() + 1); },
-  makeHeadersFromRow: function() { this.makeHeadersFromRow(this.getSelection()); },
-  renameField: function() { this.renameColumn(this.cursor.fieldIndex()); },
-  hideFields: function() { this.hideFields(this.getSelection()); },
-  deleteFields: function() { this._deleteFields(); },
   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: async function(pasteObj, cutCallback) {
-    if (this.gristDoc.isReadonly.get()) { return; }
-    await this.gristDoc.docData.bundleActions(null, () => this.paste(pasteObj, cutCallback));
-    await this.scrollToCursor(false);
-  },
-  sortAsc: function() {
-    sortBy(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.ASC);
-  },
-  sortDesc: function() {
-    sortBy(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.DESC);
-  },
-  addSortAsc: function() {
-    addToSort(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.ASC);
-  },
-  addSortDesc: function() {
-    addToSort(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.DESC);
-  },
-  toggleFreeze: function() {
-    // get column selection
-    const selection = this.getSelection();
-    // convert it menu option
-    const options = this._getColumnMenuOptions(selection);
-    // generate action that is available for freeze toggle
-    const action = freezeAction(options);
-    // if no action, do nothing
-    if (!action) { return; }
-    // if grist document is in readonly - simply change the value
-    // without saving
-    if (this.isReadonly) {
-      this.viewSection.rawNumFrozen(action.numFrozen);
-      return;
-    }
-    this.viewSection.rawNumFrozen.setAndSave(action.numFrozen);
-  },
   viewAsCard() {
     const selectedRows = this.selectedRows();
     if (selectedRows.length !== 1) { return; }

app/client/components/GristDoc.ts

@@ -21,6 +21,7 @@ import {RawDataPage, RawDataPopup} from 'app/client/components/RawDataPage';
 import {RecordCardPopup} from 'app/client/components/RecordCardPopup';
 import {ActionGroupWithCursorPos, UndoStack} from 'app/client/components/UndoStack';
 import {ViewLayout} from 'app/client/components/ViewLayout';
+import {RegionFocusSwitcher} from 'app/client/components/RegionFocusSwitcher';
 import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
 import {DocPluginManager} from 'app/client/lib/DocPluginManager';
 import {ImportSourceElement} from 'app/client/lib/ImportSourceElement';
@@ -177,6 +178,7 @@ export interface GristDoc extends DisposableWithEvents {
   isTimingOn: Observable<boolean>;
   attachmentTransfer: Observable<AttachmentTransferStatus | null>;
   canShowRawData: Observable<boolean>;
+  regionFocusSwitcher?: RegionFocusSwitcher;
 
   docId(): string;
   openDocPage(viewId: IDocPage): Promise<void>;
@@ -220,6 +222,7 @@ export class GristDocImpl extends DisposableWithEvents implements GristDoc {
   public isReadonly = this.docPageModel.isReadonly;
   public isReadonlyKo = toKo(ko, this.isReadonly);
   public comparison: DocStateComparison | null;
+  public regionFocusSwitcher?: RegionFocusSwitcher;
   // component for keeping track of latest cursor position
   public cursorMonitor: CursorMonitor;
   // component for keeping track of a cell that is being edited

app/client/components/RegionFocusSwitcher.ts

@@ -7,6 +7,7 @@ import * as commands from 'app/client/components/commands';
 import {triggerFocusGrab} from 'app/client/components/Clipboard';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {theme} from 'app/client/ui2018/cssVars';
+import BaseView from 'app/client/components/BaseView';
 
 const t = makeT('RegionFocusSwitcher');
 
@@ -53,6 +54,7 @@ export class RegionFocusSwitcher extends Disposable {
     if (this._gristDocObs && this._gristDocObs.get() === null) {
       this._gristDocObs.addListener((doc, prevDoc) => {
         if (doc && prevDoc === null) {
+          doc.regionFocusSwitcher = this;
           this.init(pageContainer);
         }
       });
@@ -95,6 +97,13 @@ export class RegionFocusSwitcher extends Disposable {
     this.current.set(region);
   }
 
+  public focusActiveSection() {
+    const gristDoc = this._getGristDoc();
+    if (gristDoc) {
+      this.focusRegion({type: 'section', id: gristDoc.viewModel.activeSectionId()});
+    }
+  }
+
   public reset(initiator?: UpdateInitiator) {
     this.focusRegion(undefined, {initiator});
   }
@@ -339,6 +348,21 @@ export class RegionFocusSwitcher extends Disposable {
   }
 }
 
+/**
+ * Helper to declare view commands that should also focus current view.
+ *
+ * Used by a view when registering command groups.
+ */
+export const viewCommands = (commandsObject: Record<string, Function>, context: BaseView) => {
+  return Object.keys(commandsObject).reduce<Record<string, Function>>((acc, key) => {
+    const originalCommand = commandsObject[key];
+    acc[key] = function(...args: any[]) {
+      context.gristDoc.regionFocusSwitcher?.focusActiveSection();
+      return originalCommand.apply(context, args);
+    };
+    return acc;
+  }, {});
+};
 
 const ATTRS = {
   regionId: 'data-grist-region-id',