(core) Removing warnings about deprecated shortcuts.

Summary:
Warnings about deprecated shortcuts are no longer needed.
As a side effect it fixes a bug that caused those warnings to not persist its
state on pages with charts.

Test Plan: Removed

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3820

app/client/components/DeprecatedCommands.ts

@@ -1,136 +0,0 @@
-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<string, Command>);
-    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) {
-    if (!this._hasSeenWarning(c.name)) {
-      this._showWarning(c);
-      return false; // Stop processing.
-    } else {
-      return true; // Continue processing.
-    }
-  }
-
-  private _showWarning(c: Command) {
-    // 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");
-    const seenWarnings = this._gristDoc.docPageModel.appModel.deprecatedWarnings;
-    function onClose(checked: boolean) {
-      if (checked) {
-        // For deprecated zoom commands we have the same messages, so mark them as seen.
-        const zoomCommands: DeprecationWarning[] = [
-          'deprecatedDeleteRecords',
-          'deprecatedInsertRecordAfter',
-          'deprecatedInsertRowBefore',
-        ];
-        if (zoomCommands.includes(c.name as any)) {
-          zoomCommands.forEach((name) => markAsSeen(seenWarnings, name));
-        } else {
-          markAsSeen(seenWarnings, c.name);
-        }
-      }
-    }
-    if (!selectedCell) {
-      reportMessage(() => dom('div', this._createMessage(c.desc)), {
-        level: 'info',
-        key: 'deprecated-command',
-      });
-    } else {
-      showDeprecatedWarning(
-        selectedCell,
-        this._createMessage(c.desc),
-        onClose
-      );
-    }
-  }
-
-  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(() => dom('span', 'or')));
-        }
-      } else {
-        elements.push(cssTallerText(part));
-      }
-    }
-    return elements;
-  }
-}
-
-const cssTallerText = styled('span', `
-  line-height: 24px;
-`);

app/client/components/GristDoc.ts

@@ -11,7 +11,6 @@ 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";
@@ -440,7 +439,6 @@ export class GristDoc extends DisposableWithEvents {
     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 : []);

app/client/components/commandList.js

@@ -1,6 +1,3 @@
-const { DeprecationWarning } = require('app/common/Prefs');
-
-
 // The top-level groups, and the ordering within them are for user-facing documentation.
 exports.groups = [{
   group: 'General',
@@ -408,21 +405,6 @@ 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,
     },
   ],
 }, {

app/client/models/AppModel.ts

@@ -13,7 +13,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 {DeprecationWarning, DismissedPopup, DismissedReminder, UserPrefs} from 'app/common/Prefs';
+import {DismissedPopup, DismissedReminder, UserPrefs} from 'app/common/Prefs';
 import {isOwner, isOwnerOrEditor} from 'app/common/roles';
 import {getTagManagerScript} from 'app/common/tagManager';
 import {getDefaultThemePrefs, Theme, ThemeAppearance, ThemeColors, ThemePrefs,
@@ -94,10 +94,6 @@ export interface AppModel {
    * Popups that user has seen.
    */
   dismissedPopups: Observable<DismissedPopup[]>;
-  /**
-   * Deprecation messages that user has seen.
-   */
-  deprecatedWarnings: Observable<DeprecationWarning[]>;
   dismissedWelcomePopups: Observable<DismissedReminder[]>;
 
   pageType: Observable<PageType>;
@@ -245,8 +241,6 @@ export class AppModelImpl extends Disposable implements AppModel {
 
   public readonly dismissedPopups = getUserPrefObs(this.userPrefsObs, 'dismissedPopups',
     { defaultValue: [] }) as Observable<DismissedPopup[]>;
-  public readonly deprecatedWarnings = getUserPrefObs(this.userPrefsObs, 'seenDeprecatedWarnings',
-    { defaultValue: [] }) as Observable<DeprecationWarning[]>;
   public readonly dismissedWelcomePopups = getUserPrefObs(this.userPrefsObs, 'dismissedWelcomePopups',
     { defaultValue: [] }) as Observable<DismissedReminder[]>;
 

app/common/Prefs.ts

@@ -23,7 +23,7 @@ export interface UserPrefs extends Prefs {
   recordSignUpEvent?: boolean;
   // Theme-related preferences.
   theme?: ThemePrefs;
-  // List of deprecated warnings user have seen.
+  // List of deprecated warnings user have seen. Kept for historical reasons as some users have them in their prefs.
   seenDeprecatedWarnings?: DeprecationWarning[];
   // List of dismissedPopups user have seen.
   dismissedPopups?: DismissedPopup[];
@@ -61,11 +61,16 @@ export type OrgPrefs = Prefs;
  * 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.
+ *
+ * Source code for this feature was deprecated itself :). Here is a link to the latest revision:
+ * https://github.com/gristlabs/grist-core/blob/ec20e7fb68786e10979f238c16c432c50a9a7464/app/client/components/DeprecatedCommands.ts
  */
 export const DeprecationWarning = StringUnion(
-  'deprecatedInsertRowBefore',
+  // Those are not checked anymore. They are kept here for historical reasons (as some users have them marked as seen
-  'deprecatedInsertRecordAfter',
+  // so they should not be reused).
-  'deprecatedDeleteRecords',
+  // 'deprecatedInsertRowBefore',
+  // 'deprecatedInsertRecordAfter',
+  // 'deprecatedDeleteRecords',
 );
 export type DeprecationWarning = typeof DeprecationWarning.type;
 

app/server/lib/FlexServer.ts

@@ -5,7 +5,6 @@ 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';
@@ -826,9 +825,7 @@ export class FlexServer implements GristServer {
           // Note that the updateOrg() method handles all levels of prefs (for user, user+org, or org).
           await this._dbManager.updateOrg(getScope(req), 0, {userPrefs: {
             showNewUserQuestions: true,
-            recordSignUpEvent: true,
+            recordSignUpEvent: true
-            // Mark all deprecated warnings as seen.
-            seenDeprecatedWarnings: DeprecationWarning.values
           }});
 
           const domain = mreq.org ?? null;