Shortcut refactor and fix UX issues with keyboard shortcuts

docs/generatedApiDocs/utils/FeatureGate-API.md

@@ -89,6 +89,15 @@ if(FeatureGate.isFeatureEnabled(FEATURE_NEW_COLORS)){
 
 Returns **[boolean][3]** 
 
+## setFeatureEnabled
+
+Sets the enabled state of a specific feature in the application.
+
+### Parameters
+
+*   `featureName` **[string][2]** The name of the feature to be modified.
+*   `isEnabled` **[boolean][3]** A boolean flag indicating whether the feature should be enabled (true) or disabled (false).
+
 [1]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Statements/function
 
 [2]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String

src/base-config/keyboard.json

@@ -1,21 +1,12 @@
 {
-    "file.newFile":  [
-        "Alt-N"
+    "file.newFile": [
+        "Ctrl-1"
     ],
     "file.newFolder":  [
-        "Ctrl-Alt-N"
-    ],
-    "file.new_window":  [
-        "Alt-Shift-N"
-    ],
-    "file.open":  [
-        "Ctrl-O"
+        "Ctrl-3"
     ],
     "file.close":  [
-        "Alt-W"
-    ],
-    "file.openFolder": [
-        "Ctrl-Alt-O"
+        "Ctrl-`"
     ],
     "file.close_all":  [
         "Alt-Shift-W"
@@ -30,7 +21,7 @@
         "Ctrl-Shift-S"
     ],
     "file.liveFilePreview":  [
-        "Ctrl-Alt-P"
+        "Ctrl-Alt-L"
     ],
     "file.reloadLivePreview": [
         {
@@ -44,7 +35,7 @@
         "Ctrl-Q"
     ],
     "file.duplicateFile": [
-        "Alt-Shift-D"
+        "Ctrl-2"
     ],
     "edit.undo": [
         "Ctrl-Z"
@@ -286,6 +277,9 @@
     "navigate.quickOpen":  [
         "Ctrl-Shift-O"
     ],
+    "navigate.showInFileTree":  [
+        "Ctrl-Shift-0"
+    ],
     "navigate.gotoLine":  [
         {
             "key": "Ctrl-G"

src/command/DefaultMenus.js

@@ -63,21 +63,60 @@ define(function (require, exports, module) {
         }
     }
 
+    const isBrowser = !Phoenix.browser.isTauri;
+    const isDesktop = Phoenix.browser.isTauri;
+    const fileNewShortcut = isDesktop ? "Ctrl-N" : ""; // Ctrl-1 universal shortcut is set in keyboard.json
+    //`Ctrl-Shift-N` - desktop only. In browser, use can do `ctrl-T` and type phcode.dev using browser shortcuts itself. So we dont make a browser shortcut for this.
+    const fileNewWindowShortcut = isDesktop ? "Ctrl-Shift-N" : "";
+    const fileCloseShortcut = isDesktop ? "Ctrl-W" : ""; // Ctrl-` universal shortcut is set in keyboard.json
+    // In browser open file option is not available, so we assign ctrl-o to open folder in browser
+    // open folder has no shortcut in desktop, as this is not a frequently used workflow
+    // and only ever used when the user opens a project once in a while.
+    const openFileShortcut = isDesktop ? "Ctrl-O" : "";
+    const openFolderShortcut = isBrowser ? "Ctrl-O" : "";
+
     AppInit.htmlReady(function () {
-        /*
-         * File menu
-         */
+        /**
+         * Rules to follow when figuring out default shortcuts:
+         * 1. Default shortcuts should only be applied for frequently used workflows. If it's rarely used like
+         *    opening a project, etc., we should not allocate a shortcut. Ideally, assign a shortcut if the user is likely
+         *    to use it several times in a five-minute window.
+         * 2. In Windows, shortcuts cannot start with a `Ctrl-Alt-<something>` as it's reserved for special OS functionalities. See wiki.
+         * 3. In macOS, shortcuts can't start with a single Alt-key combo as it's used for unicode typing in Eastern European languages.
+         * 3. Maintain Consistency Across Browser and Desktop Applications: Shortcuts should offer a uniform experience
+         *    in both browser-based and desktop environments, even when accounting for platform-specific restrictions.
+         *    For instance, 'Ctrl-N' is used for creating a new file in the desktop, but this shortcut
+         *    is reserved by the browser, an alternative like 'Ctrl-1' can was used in the browser. To ensure
+         *    consistency, the same 'Ctrl-1' shortcut was also be enabled for creating a new file in the desktop
+         *    app along with `Ctrl-N`. This approach helps users experience predictable across different platforms.
+         *
+         * Additional considerations:
+         * 4. Avoid conflicts with standard OS shortcuts: Ensure that the chosen shortcuts do not conflict with common
+         *    operating system shortcuts. For instance, shortcuts like Alt-F4 in Windows or Cmd-Q in macOS are universally
+         *    used for closing applications and should not be overridden.
+         * 6. Consistency with similar applications: Where possible, align shortcuts with those used in similar applications
+         *    to reduce the learning curve for new users. For example, using Ctrl/Cmd + S for 'save' is a widely recognized standard.
+         * 7. Avoid overloading single keys with multiple modifiers: Combining too many modifier keys (like Ctrl-Shift-Alt-Cmd-K)
+         *    can make shortcuts hard to remember and physically challenging to perform.
+         * 8. Localization and Internationalization: Be aware of how shortcuts might interact with different keyboard layouts
+         *    and languages. For example, what is convenient on a QWERTY keyboard might not be on AZERTY or QWERTZ.
+         * 9. User Customization: Allow users to customize or reassign shortcuts, as personal preferences and workflows vary.
+         *    This also helps users resolve any unforeseen conflicts with other software they use.
+         * 10. Document Shortcuts: See keyboard.json for most shortcuts. Other shortcuts may be set programmatically
+         *    by the phoenix extensions.
+         **/
+
         var menu;
         menu = Menus.addMenu(Strings.FILE_MENU, Menus.AppMenuBar.FILE_MENU);
-        menu.addMenuItem(Commands.FILE_NEW);
+        menu.addMenuItem(Commands.FILE_NEW, fileNewShortcut);
         menu.addMenuItem(Commands.FILE_NEW_FOLDER);
-        menu.addMenuItem(Commands.FILE_NEW_WINDOW);
+        menu.addMenuItem(Commands.FILE_NEW_WINDOW, fileNewWindowShortcut);
         menu.addMenuDivider();
         if(Phoenix.browser.isTauri){
-            menu.addMenuItem(Commands.FILE_OPEN);
+            menu.addMenuItem(Commands.FILE_OPEN, openFileShortcut);
         }
-        menu.addMenuItem(Commands.FILE_OPEN_FOLDER);
-        menu.addMenuItem(Commands.FILE_CLOSE);
+        menu.addMenuItem(Commands.FILE_OPEN_FOLDER, openFolderShortcut);
+        menu.addMenuItem(Commands.FILE_CLOSE, fileCloseShortcut);
         menu.addMenuItem(Commands.FILE_CLOSE_ALL);
         menu.addMenuDivider();
         menu.addMenuItem(Commands.FILE_SAVE);
@@ -90,6 +129,10 @@ define(function (require, exports, module) {
         // menu.addMenuItem(Commands.FILE_PROJECT_SETTINGS); not yet available in phoenix
         menu.addMenuDivider();
         menu.addMenuItem(Commands.FILE_EXTENSION_MANAGER);
+        if(Phoenix.browser.isTauri){
+            menu.addMenuDivider();
+            menu.addMenuItem(Commands.FILE_QUIT);
+        }
 
         /*
          * Edit  menu

src/command/KeyBindingManager.js

@@ -51,6 +51,9 @@ define(function (require, exports, module) {
     let KEYMAP_FILENAME     = "keymap.json",
         _userKeyMapFilePath = path.normalize(brackets.app.getApplicationSupportDirectory() + "/" + KEYMAP_FILENAME);
 
+    const EVENT_KEY_BINDING_ADDED = "keyBindingAdded",
+        EVENT_KEY_BINDING_REMOVED = "keyBindingRemoved";
+
     /**
      * @private
      * Maps normalized shortcut descriptor to key binding info.
@@ -569,7 +572,7 @@ define(function (require, exports, module) {
                 });
 
                 if (command) {
-                    command.trigger("keyBindingRemoved", {key: normalizedKey, displayKey: binding.displayKey});
+                    command.trigger(EVENT_KEY_BINDING_REMOVED, {key: normalizedKey, displayKey: binding.displayKey});
                 }
             }
         }
@@ -649,9 +652,17 @@ define(function (require, exports, module) {
 
         // skip if the key binding is invalid
         if (!normalized) {
-            console.error("Unable to parse key binding " + key + ". Permitted modifiers: Ctrl, Cmd, Alt, Opt, Shift; separated by '-' (not '+').");
+            console.error(`Unable to parse key binding '${key}' for command '${commandID}'. Permitted modifiers: Ctrl, Cmd, Alt, Opt, Shift; separated by '-' (not '+').`);
             return null;
         }
+        function isSingleCharAZ(str) {
+            return /^[A-Z]$/i.test(str);
+        }
+        const keySplit = normalized.split("-");
+        if((keySplit.length ===2 && keySplit[0] === 'Alt' && isSingleCharAZ(keySplit[1])) ||
+            (keySplit.length ===3 && keySplit[0] === 'Alt' && keySplit[1] === 'Shift' && isSingleCharAZ(keySplit[2]))){
+            console.error(`Key binding '${normalized}' for command '${commandID}' may cause issues. The key combinations starting with 'Alt-<letter>' and 'Alt-Shift-<letter>' are reserved. On macOS, they are used for AltGr internationalization, and on Windows/Linux, they are used for menu navigation shortcuts.`);
+        }
 
         // check for duplicate key bindings
         existing = _keyMap[normalized];
@@ -763,7 +774,7 @@ define(function (require, exports, module) {
         command = CommandManager.get(commandID);
 
         if (command) {
-            command.trigger("keyBindingAdded", result);
+            command.trigger(EVENT_KEY_BINDING_ADDED, result, commandID);
         }
 
         return result;
@@ -937,6 +948,23 @@ define(function (require, exports, module) {
         return bindings || [];
     }
 
+    /**
+     * Retrieves the platform-specific string representation of the key bindings for a specified command.
+     * This function is useful for displaying the keyboard shortcut associated with a given command ID to the user.
+     * If a key binding is found for the command, it returns the formatted key descriptor. Otherwise, it returns null.
+     *
+     * @param {string} commandID - The unique identifier of the command for which the key binding is to be retrieved.
+     * @returns {string|null} The formatted key binding as a string if available; otherwise, null.
+     */
+    function getKeyBindingsDisplay(commandID) {
+        let shortCut = getKeyBindings(commandID);
+        if (shortCut && shortCut[0] && shortCut[0].displayKey) {
+            return formatKeyDescriptor(shortCut[0].displayKey);
+        }
+        return null;
+    }
+
+
     /**
      * Adds default key bindings when commands are registered to CommandManager
      * @param {$.Event} event jQuery event
@@ -1479,9 +1507,14 @@ define(function (require, exports, module) {
     exports.removeBinding = removeBinding;
     exports.formatKeyDescriptor = formatKeyDescriptor;
     exports.getKeyBindings = getKeyBindings;
+    exports.getKeyBindingsDisplay = getKeyBindingsDisplay;
     exports.addGlobalKeydownHook = addGlobalKeydownHook;
     exports.removeGlobalKeydownHook = removeGlobalKeydownHook;
 
+    // public events
+    exports.EVENT_KEY_BINDING_ADDED = EVENT_KEY_BINDING_ADDED;
+    exports.EVENT_KEY_BINDING_REMOVED = EVENT_KEY_BINDING_REMOVED;
+
     /**
      * Use windows-specific bindings if no other are found (e.g. Linux).
      * Core Brackets modules that use key bindings should always define at

src/document/DocumentCommandHandlers.js

@@ -1596,12 +1596,18 @@ define(function (require, exports, module) {
     function handleFileCloseWindow(commandData) {
         return _handleWindowGoingAway(
             commandData,
-            function () {
-                window.close();
+            function (closeSuccess) {
+                console.log('close success: ', closeSuccess);
+                raceAgainstTime(window.PhStore.flushDB(), 8000)
+                    .finally(()=>{
+                        raceAgainstTime(NodeConnector.terminateNode())
+                            .finally(()=>{
+                                Phoenix.app.closeWindow();
+                            });
+                    });
             },
-            function () {
-                // if fail, tell the app to abort any pending quit operation.
-                brackets.app.abortQuit();
+            function (err) {
+                console.error("Quit failed! ", err);
             }
         );
     }
@@ -1633,12 +1639,18 @@ define(function (require, exports, module) {
     function handleFileQuit(commandData) {
         return _handleWindowGoingAway(
             commandData,
-            function () {
-                brackets.app.quit();
+            function (closeSuccess) {
+                console.log('close success: ', closeSuccess);
+                raceAgainstTime(window.PhStore.flushDB(), 8000)
+                    .finally(()=>{
+                        raceAgainstTime(NodeConnector.terminateNode())
+                            .finally(()=>{
+                                Phoenix.app.closeWindow();
+                            });
+                    });
             },
-            function () {
-                // if fail, don't exit: user canceled (or asked us to save changes first, but we failed to do so)
-                brackets.app.abortQuit();
+            function (err) {
+                console.error("Quit failed! ", err);
             }
         );
     }

src/editor/CodeHintManager.js

@@ -705,6 +705,12 @@ define(function (require, exports, module) {
 
     function activeEditorChangeHandler(event, current, previous) {
         if (current) {
+            current.off("editorChange", _handleChange);
+            current.off("keydown",  _handleKeydownEvent);
+            current.off("keypress", _handleKeypressEvent);
+            current.off("keyup",    _handleKeyupEvent);
+            current.off("cursorActivity", _handleCursorActivity);
+
             current.on("editorChange", _handleChange);
             current.on("keydown",  _handleKeydownEvent);
             current.on("keypress", _handleKeypressEvent);

src/extensions/default/CodeFolding/main.js

@@ -49,12 +49,12 @@ define(function (require, exports, module) {
         GUTTER_NAME             = "CodeMirror-foldgutter",
         CODE_FOLDING_GUTTER_PRIORITY   = Editor.CODE_FOLDING_GUTTER_PRIORITY,
         codeFoldingMenuDivider  = "codefolding.divider",
-        collapseKey             = "Ctrl-Alt-[",
-        expandKey               = "Ctrl-Alt-]",
-        collapseAllKey          = "Alt-1",
-        expandAllKey            = "Shift-Alt-1",
-        collapseAllKeyMac       = "Cmd-1",
-        expandAllKeyMac         = "Cmd-Shift-1";
+        collapseKey             = "Alt-Shift-Left",
+        collapseKeyDisplay      = "Alt-Shift-←",
+        expandKey               = "Alt-Shift-Right",
+        expandKeyDisplay               = "Alt-Shift-→",
+        collapseAllKey          = "Ctrl-Alt-[",
+        expandAllKey            = "Ctrl-Alt-]";
 
     ExtensionUtils.loadStyleSheet(module, "main.less");
 
@@ -362,8 +362,6 @@ define(function (require, exports, module) {
         KeyBindingManager.removeBinding(expandKey);
         KeyBindingManager.removeBinding(collapseAllKey);
         KeyBindingManager.removeBinding(expandAllKey);
-        KeyBindingManager.removeBinding(collapseAllKeyMac);
-        KeyBindingManager.removeBinding(expandAllKeyMac);
 
         //remove menus
         Menus.getMenu(Menus.AppMenuBar.VIEW_MENU).removeMenuDivider(codeFoldingMenuDivider.id);
@@ -421,10 +419,10 @@ define(function (require, exports, module) {
         Menus.getMenu(Menus.AppMenuBar.VIEW_MENU).addMenuItem(EXPAND);
 
         //register keybindings
-        KeyBindingManager.addBinding(COLLAPSE_ALL, [ {key: collapseAllKey}, {key: collapseAllKeyMac, platform: "mac"} ]);
-        KeyBindingManager.addBinding(EXPAND_ALL, [ {key: expandAllKey}, {key: expandAllKeyMac, platform: "mac"} ]);
-        KeyBindingManager.addBinding(COLLAPSE, collapseKey);
-        KeyBindingManager.addBinding(EXPAND, expandKey);
+        KeyBindingManager.addBinding(COLLAPSE_ALL, [ {key: collapseAllKey}]);
+        KeyBindingManager.addBinding(EXPAND_ALL, [ {key: expandAllKey}]);
+        KeyBindingManager.addBinding(COLLAPSE, [{key: collapseKey, displayKey: collapseKeyDisplay}]);
+        KeyBindingManager.addBinding(EXPAND, [{key:expandKey, displayKey: expandKeyDisplay}]);
 
 
         // Add gutters & restore saved expand/collapse state in all currently open editors

src/extensions/default/DebugCommands/main.js

@@ -789,8 +789,8 @@ define(function (require, exports, module) {
 
     CommandManager.register(Strings.CMD_OPEN_PREFERENCES, DEBUG_OPEN_PREFERENCES_IN_SPLIT_VIEW, handleOpenPrefsInSplitView);
     const debugMenu = Menus.getMenu(Menus.AppMenuBar.DEBUG_MENU);
-    debugMenu.addMenuItem(DEBUG_REFRESH_WINDOW, KeyboardPrefs.refreshWindow);
-    debugMenu.addMenuItem(DEBUG_RELOAD_WITHOUT_USER_EXTS, KeyboardPrefs.reloadWithoutUserExts);
+    debugMenu.addMenuItem(DEBUG_REFRESH_WINDOW, window.debugMode ? KeyboardPrefs.refreshWindow : undefined);
+    debugMenu.addMenuItem(DEBUG_RELOAD_WITHOUT_USER_EXTS, window.debugMode ? KeyboardPrefs.reloadWithoutUserExts : undefined);
     debugMenu.addMenuItem(DEBUG_LOAD_CURRENT_EXTENSION);
     debugMenu.addMenuItem(DEBUG_UNLOAD_CURRENT_EXTENSION, undefined, undefined, undefined, {
         hideWhenCommandDisabled: true
@@ -840,9 +840,9 @@ define(function (require, exports, module) {
 
     const fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU);
     // this command will enable defaultPreferences and brackets preferences to be open side by side in split view.
-    fileMenu.addMenuItem(DEBUG_OPEN_PREFERENCES_IN_SPLIT_VIEW, null, Menus.AFTER, Menus.MenuSection.FILE_SETTINGS.sectionMarker);
+    fileMenu.addMenuItem(DEBUG_OPEN_PREFERENCES_IN_SPLIT_VIEW, null, Menus.BEFORE, Menus.MenuSection.FILE_SETTINGS.sectionMarker);
     // this command is defined in core, but exposed only in Debug menu for now
-    fileMenu.addMenuItem(Commands.FILE_OPEN_KEYMAP, null, Menus.AFTER, Menus.MenuSection.FILE_SETTINGS.sectionMarker);
+    fileMenu.addMenuItem(Commands.FILE_OPEN_KEYMAP, null, Menus.BEFORE, Menus.MenuSection.FILE_SETTINGS.sectionMarker);
 
     // exposed for convenience, but not official API
     exports._runUnitTests = _runUnitTests;

src/extensions/default/NavigationAndHistory/FileRecovery.js

@@ -125,7 +125,7 @@ define(function (require, exports, module) {
 
     function getRestoreFilePath(projectFilePath, projectRootPath) {
         if(!projectFilePath.startsWith(projectRootPath) || !trackedProjects[projectRootPath]){
-            console.error(`[recovery] cannot backed up as ${projectRootPath} is not in project ${projectRootPath}`);
+            console.error(`[recovery] cannot backed up as ${projectFilePath} is not in project ${projectRootPath}`);
             return null;
         }
         let pathWithinProject = projectFilePath.replace(projectRootPath, "");
@@ -268,8 +268,10 @@ define(function (require, exports, module) {
         let trackedFilePaths =  Object.keys(project.trackedFileContents);
         for(let trackedFilePath of trackedFilePaths){
             const restorePath = getRestoreFilePath(trackedFilePath, projectRoot.fullPath);
-            const content = project.trackedFileContents[trackedFilePath];
-            await writeFileIgnoreFailure(restorePath, content);
+            if(restorePath) {
+                const content = project.trackedFileContents[trackedFilePath];
+                await writeFileIgnoreFailure(restorePath, content);
+            }
             delete project.trackedFileContents[trackedFilePath];
         }
     }
@@ -280,7 +282,9 @@ define(function (require, exports, module) {
         for(let trackedPath of allTrackingPaths){
             if(!docPathsToTrack[trackedPath]){
                 const restoreFile = getRestoreFilePath(trackedPath, projectRoot.fullPath);
-                await silentlyRemoveFile(restoreFile);
+                if(restoreFile) {
+                    await silentlyRemoveFile(restoreFile);
+                }
                 delete project.trackedFileUpdateTimestamps[trackedPath];
             }
         }