From dfb240518c8bda58c5e4c1940a8e23b60e36bbe8 Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 17:37:48 +0530 Subject: [PATCH 01/12] feat: nav forward and back shortcut change and hover display --- src/base-config/keyboard.json | 3 +++ src/command/KeyBindingManager.js | 18 ++++++++++++++++++ src/editor/CodeHintManager.js | 6 ++++++ .../NavigationAndHistory/NavigationProvider.js | 18 ++++++++++++++---- .../default/NavigationAndHistory/keyboard.json | 12 ++---------- 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index 2c3040dfb..828ce856c 100644 --- a/src/base-config/keyboard.json +++ b/src/base-config/keyboard.json @@ -286,6 +286,9 @@ "navigate.quickOpen": [ "Ctrl-Shift-O" ], + "navigate.showInFileTree": [ + "Ctrl-Shift-;" + ], "navigate.gotoLine": [ { "key": "Ctrl-G" diff --git a/src/command/KeyBindingManager.js b/src/command/KeyBindingManager.js index 15d668be0..b8d99fffe 100644 --- a/src/command/KeyBindingManager.js +++ b/src/command/KeyBindingManager.js @@ -937,6 +937,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,6 +1496,7 @@ define(function (require, exports, module) { exports.removeBinding = removeBinding; exports.formatKeyDescriptor = formatKeyDescriptor; exports.getKeyBindings = getKeyBindings; + exports.getKeyBindingsDisplay = getKeyBindingsDisplay; exports.addGlobalKeydownHook = addGlobalKeydownHook; exports.removeGlobalKeydownHook = removeGlobalKeydownHook; diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index ee0e455d4..ffd7a2732 100644 --- a/src/editor/CodeHintManager.js +++ b/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); diff --git a/src/extensions/default/NavigationAndHistory/NavigationProvider.js b/src/extensions/default/NavigationAndHistory/NavigationProvider.js index 65cc6a045..a99d4ceef 100644 --- a/src/extensions/default/NavigationAndHistory/NavigationProvider.js +++ b/src/extensions/default/NavigationAndHistory/NavigationProvider.js @@ -652,6 +652,14 @@ define(function (require, exports, module) { CommandManager.execute(Commands.FILE_NEW_PROJECT); } + function _getShortcutDisplay(baseStr, commandID) { + let shortCut = KeyBindingManager.getKeyBindingsDisplay(commandID); + if(shortCut) { + return `${baseStr} (${shortCut})`; + } + return baseStr; + } + function _setupNavigationButtons() { let $mainNavBarRight = $("#mainNavBarRight"); let $mainNavBarLeft = $("#mainNavBarLeft"); @@ -662,10 +670,12 @@ define(function (require, exports, module) { $newProject = $mainNavBarLeft.find("#newProject"); - $navback.attr("title", Strings.CMD_NAVIGATE_BACKWARD); - $navForward.attr("title", Strings.CMD_NAVIGATE_FORWARD); - $showInTree.attr("title", Strings.CMD_SHOW_IN_TREE); - $searchNav.attr("title", Strings.CMD_FIND_IN_FILES); + + $navback.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_BACKWARD, NAVIGATION_JUMP_BACK)); + $navForward.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_FORWARD, NAVIGATION_JUMP_FWD)); + $showInTree.attr("title", _getShortcutDisplay(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE)); + $searchNav.attr("title", _getShortcutDisplay(Strings.CMD_FIND_IN_FILES, Commands.CMD_FIND_IN_FILES)); + // new project extension is not yet loaded, so we cant show keyboard shortcut here. $newProject.attr("title", Strings.CMD_PROJECT_NEW); $navback.on("click", _navigateBackClicked); diff --git a/src/extensions/default/NavigationAndHistory/keyboard.json b/src/extensions/default/NavigationAndHistory/keyboard.json index 8b327da70..4b0dfc81e 100644 --- a/src/extensions/default/NavigationAndHistory/keyboard.json +++ b/src/extensions/default/NavigationAndHistory/keyboard.json @@ -30,20 +30,12 @@ ], "navigation.jump.back": [ { - "key": "Alt-I" - }, - { - "key": "Cmd-Alt-Left", - "platform": "mac" + "key": "Ctrl-Shift-," } ], "navigation.jump.fwd": [ { - "key": "Alt-Shift-I" - }, - { - "key": "Cmd-Alt-Right", - "platform": "mac" + "key": "Ctrl-Shift-." } ] } From 106d430aa468ec1d1c95a2529dc35429e1f03619 Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 17:48:20 +0530 Subject: [PATCH 02/12] refactor: use getKeyBindingsDisplay api --- src/extensions/default/Phoenix/guided-tour.js | 4 ++-- src/search/FindBar.js | 4 ++-- src/view/ViewCommandHandlers.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/extensions/default/Phoenix/guided-tour.js b/src/extensions/default/Phoenix/guided-tour.js index a80928a8b..5bde253d3 100644 --- a/src/extensions/default/Phoenix/guided-tour.js +++ b/src/extensions/default/Phoenix/guided-tour.js @@ -82,8 +82,8 @@ define(function (require, exports, module) { return; } setTimeout(()=>{ - let keyboardShortcut = KeyBindingManager.getKeyBindings(Commands.EDIT_BEAUTIFY_CODE); - keyboardShortcut = (keyboardShortcut && keyboardShortcut[0]) ? keyboardShortcut[0].displayKey : "-"; + let keyboardShortcut = KeyBindingManager.getKeyBindingsDisplay(Commands.EDIT_BEAUTIFY_CODE); + keyboardShortcut = keyboardShortcut || ""; userAlreadyDidAction.beautifyCodeShown = true; PhStore.setItem(GUIDED_TOUR_LOCAL_STORAGE_KEY, JSON.stringify(userAlreadyDidAction)); Metrics.countEvent(Metrics.EVENT_TYPE.UI, "guide", "beautify"); diff --git a/src/search/FindBar.js b/src/search/FindBar.js index 588bc5c00..168f5039e 100644 --- a/src/search/FindBar.js +++ b/src/search/FindBar.js @@ -218,11 +218,11 @@ define(function (require, exports, module) { * @param {string} commandId The ID for the command whose keyboard shortcut to show. */ FindBar.prototype._addShortcutToTooltip = function ($elem, commandId) { - var replaceShortcut = KeyBindingManager.getKeyBindings(commandId)[0]; + const replaceShortcut = KeyBindingManager.getKeyBindingsDisplay(commandId); if (replaceShortcut) { var oldTitle = $elem.attr("title"); oldTitle = (oldTitle ? oldTitle + " " : ""); - $elem.attr("title", oldTitle + "(" + KeyBindingManager.formatKeyDescriptor(replaceShortcut.displayKey) + ")"); + $elem.attr("title", oldTitle + "(" + replaceShortcut + ")"); } }; diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index e45be7641..ad353637a 100644 --- a/src/view/ViewCommandHandlers.js +++ b/src/view/ViewCommandHandlers.js @@ -355,8 +355,8 @@ define(function (require, exports, module) { // most reliable zoom for now. return new $.Deferred().reject("use browser zoom"); } else { - const zoomInKey = KeyBindingManager.getKeyBindings(Commands.VIEW_ZOOM_IN)[0].displayKey, - zoomOutKey = KeyBindingManager.getKeyBindings(Commands.VIEW_ZOOM_OUT)[0].displayKey; + const zoomInKey = KeyBindingManager.getKeyBindingsDisplay(Commands.VIEW_ZOOM_IN) || '', + zoomOutKey = KeyBindingManager.getKeyBindingsDisplay(Commands.VIEW_ZOOM_OUT) || ''; let message = StringUtils.format(Strings.ZOOM_WITH_SHORTCUTS_DETAILS, zoomInKey, zoomOutKey); Dialogs.showModalDialog(DefaultDialogs.DIALOG_ID_INFO, Strings.ZOOM_WITH_SHORTCUTS, message); return new $.Deferred().resolve(); From 42c9af11499bfe6e68f56fb60017aa19b000ceaf Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 21:44:54 +0530 Subject: [PATCH 03/12] fix: key shortcut display issues and file recovery errors --- src/base-config/keyboard.json | 4 +-- src/command/KeyBindingManager.js | 21 ++++++++++++--- .../NavigationAndHistory/FileRecovery.js | 12 ++++++--- .../NavigationProvider.js | 27 ++++++++++++------- .../NavigationAndHistory/keyboard.json | 6 +++-- src/extensions/default/Phoenix/new-project.js | 2 +- 6 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index 828ce856c..3eceec1a0 100644 --- a/src/base-config/keyboard.json +++ b/src/base-config/keyboard.json @@ -44,7 +44,7 @@ "Ctrl-Q" ], "file.duplicateFile": [ - "Alt-Shift-D" + "Ctrl-2" ], "edit.undo": [ "Ctrl-Z" @@ -287,7 +287,7 @@ "Ctrl-Shift-O" ], "navigate.showInFileTree": [ - "Ctrl-Shift-;" + "Ctrl-Shift-0" ], "navigate.gotoLine": [ { diff --git a/src/command/KeyBindingManager.js b/src/command/KeyBindingManager.js index b8d99fffe..32abe62d0 100644 --- a/src/command/KeyBindingManager.js +++ b/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-' and 'Alt-Shift-' 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; @@ -1500,6 +1511,10 @@ define(function (require, exports, module) { 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 diff --git a/src/extensions/default/NavigationAndHistory/FileRecovery.js b/src/extensions/default/NavigationAndHistory/FileRecovery.js index f34e38c4a..c6999c567 100644 --- a/src/extensions/default/NavigationAndHistory/FileRecovery.js +++ b/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]; } } diff --git a/src/extensions/default/NavigationAndHistory/NavigationProvider.js b/src/extensions/default/NavigationAndHistory/NavigationProvider.js index a99d4ceef..7e51ef0ca 100644 --- a/src/extensions/default/NavigationAndHistory/NavigationProvider.js +++ b/src/extensions/default/NavigationAndHistory/NavigationProvider.js @@ -51,7 +51,8 @@ define(function (require, exports, module) { let $navback = null, $navForward = null, $searchNav = null, - $newProject = null; + $newProject = null, + $showInTree = null; /** * Contains list of most recently known cursor positions. @@ -660,23 +661,29 @@ define(function (require, exports, module) { return baseStr; } + function updateTooltips() { + $navback.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_BACKWARD, NAVIGATION_JUMP_BACK)); + $navForward.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_FORWARD, NAVIGATION_JUMP_FWD)); + $showInTree.attr("title", _getShortcutDisplay(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE)); + $searchNav.attr("title", _getShortcutDisplay(Strings.CMD_FIND_IN_FILES, Commands.CMD_FIND_IN_FILES)); + // new project extension is not yet loaded, so we cant show keyboard shortcut here. + $newProject.attr("title", Strings.CMD_PROJECT_NEW); + } + function _setupNavigationButtons() { let $mainNavBarRight = $("#mainNavBarRight"); let $mainNavBarLeft = $("#mainNavBarLeft"); - let $showInTree = $mainNavBarRight.find("#showInfileTree"); + $showInTree = $mainNavBarRight.find("#showInfileTree"); $navback = $mainNavBarRight.find("#navBackButton"); $navForward = $mainNavBarRight.find("#navForwardButton"); $searchNav = $mainNavBarRight.find("#searchNav"); $newProject = $mainNavBarLeft.find("#newProject"); - - - $navback.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_BACKWARD, NAVIGATION_JUMP_BACK)); - $navForward.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_FORWARD, NAVIGATION_JUMP_FWD)); - $showInTree.attr("title", _getShortcutDisplay(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE)); - $searchNav.attr("title", _getShortcutDisplay(Strings.CMD_FIND_IN_FILES, Commands.CMD_FIND_IN_FILES)); - // new project extension is not yet loaded, so we cant show keyboard shortcut here. - $newProject.attr("title", Strings.CMD_PROJECT_NEW); + updateTooltips(); + CommandManager.get(NAVIGATION_JUMP_BACK).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips); + CommandManager.get(NAVIGATION_JUMP_FWD).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips); + CommandManager.get(Commands.NAVIGATE_SHOW_IN_FILE_TREE).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips); + CommandManager.get(Commands.CMD_FIND_IN_FILES).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips); $navback.on("click", _navigateBackClicked); $navForward.on("click", _navigateForwardClicked); diff --git a/src/extensions/default/NavigationAndHistory/keyboard.json b/src/extensions/default/NavigationAndHistory/keyboard.json index 4b0dfc81e..8a72e0042 100644 --- a/src/extensions/default/NavigationAndHistory/keyboard.json +++ b/src/extensions/default/NavigationAndHistory/keyboard.json @@ -30,12 +30,14 @@ ], "navigation.jump.back": [ { - "key": "Ctrl-Shift-," + "key": "Alt-Left", + "displayKey": "Alt-←" } ], "navigation.jump.fwd": [ { - "key": "Ctrl-Shift-." + "key": "Alt-Right", + "displayKey": "Alt-→" } ] } diff --git a/src/extensions/default/Phoenix/new-project.js b/src/extensions/default/Phoenix/new-project.js index 187d18091..13204fea2 100644 --- a/src/extensions/default/Phoenix/new-project.js +++ b/src/extensions/default/Phoenix/new-project.js @@ -72,7 +72,7 @@ define(function (require, exports, module) { function _addMenuEntries() { CommandManager.register(Strings.CMD_PROJECT_NEW, Commands.FILE_NEW_PROJECT, _showNewProjectDialogue); const fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU); - fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Alt-Shift-P", Menus.AFTER, Commands.FILE_NEW_FOLDER); + fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Ctrl-Shift-2", Menus.AFTER, Commands.FILE_NEW_FOLDER); } function closeDialogue() { From 39f954b194bbb4833e8b82d3d2069e29373bfc6f Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 21:51:02 +0530 Subject: [PATCH 04/12] chore: f5 and shift-f5 key to restart phcode only if enable detailed logs(debug mode) is enabled --- src/extensions/default/DebugCommands/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extensions/default/DebugCommands/main.js b/src/extensions/default/DebugCommands/main.js index e57350d7c..a1619bbf9 100644 --- a/src/extensions/default/DebugCommands/main.js +++ b/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 From c017dc6b3d714d8574954eff5e5c7836980c2cf5 Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 23:39:16 +0530 Subject: [PATCH 05/12] chore: new project, window, file shortcut change --- src/base-config/keyboard.json | 10 ++-------- src/command/DefaultMenus.js | 7 +++++-- src/extensions/default/Phoenix/new-project.js | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index 3eceec1a0..fa99740fc 100644 --- a/src/base-config/keyboard.json +++ b/src/base-config/keyboard.json @@ -1,12 +1,6 @@ { - "file.newFile": [ - "Alt-N" - ], "file.newFolder": [ - "Ctrl-Alt-N" - ], - "file.new_window": [ - "Alt-Shift-N" + "Ctrl-M" ], "file.open": [ "Ctrl-O" @@ -30,7 +24,7 @@ "Ctrl-Shift-S" ], "file.liveFilePreview": [ - "Ctrl-Alt-P" + "Ctrl-Alt-L" ], "file.reloadLivePreview": [ { diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js index f7fd4831e..7d3e94427 100644 --- a/src/command/DefaultMenus.js +++ b/src/command/DefaultMenus.js @@ -63,15 +63,18 @@ define(function (require, exports, module) { } } + const fileNewShortcut = Phoenix.browser.isTauri ? "Ctrl-N" : "Ctrl-1"; + const fileNewWindowShortcut = Phoenix.browser.isTauri ? "Ctrl-Shift-N" : "Ctrl-3"; + AppInit.htmlReady(function () { /* * File menu */ 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); diff --git a/src/extensions/default/Phoenix/new-project.js b/src/extensions/default/Phoenix/new-project.js index 13204fea2..39091586a 100644 --- a/src/extensions/default/Phoenix/new-project.js +++ b/src/extensions/default/Phoenix/new-project.js @@ -72,7 +72,7 @@ define(function (require, exports, module) { function _addMenuEntries() { CommandManager.register(Strings.CMD_PROJECT_NEW, Commands.FILE_NEW_PROJECT, _showNewProjectDialogue); const fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU); - fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Ctrl-Shift-2", Menus.AFTER, Commands.FILE_NEW_FOLDER); + fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Ctrl-Alt-P", Menus.AFTER, Commands.FILE_NEW_FOLDER); } function closeDialogue() { From 311f85143a7b08c04279c112f2d67d3e36593eae Mon Sep 17 00:00:00 2001 From: abose Date: Fri, 29 Dec 2023 00:11:48 +0530 Subject: [PATCH 06/12] feat: file menu>quit support in desktop builds --- src/command/DefaultMenus.js | 4 +++ src/document/DocumentCommandHandlers.js | 32 +++++++++++++------ src/extensions/default/DebugCommands/main.js | 4 +-- .../default/Phoenix-live-preview/main.js | 4 +-- src/nls/root/strings.js | 2 +- 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js index 7d3e94427..2293e0a65 100644 --- a/src/command/DefaultMenus.js +++ b/src/command/DefaultMenus.js @@ -93,6 +93,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 diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 14f07616f..9b218cf9d 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/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); } ); } diff --git a/src/extensions/default/DebugCommands/main.js b/src/extensions/default/DebugCommands/main.js index a1619bbf9..c735d0199 100644 --- a/src/extensions/default/DebugCommands/main.js +++ b/src/extensions/default/DebugCommands/main.js @@ -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; diff --git a/src/extensions/default/Phoenix-live-preview/main.js b/src/extensions/default/Phoenix-live-preview/main.js index 1b9732069..2fcbed7e5 100644 --- a/src/extensions/default/Phoenix-live-preview/main.js +++ b/src/extensions/default/Phoenix-live-preview/main.js @@ -369,8 +369,8 @@ define(function (require, exports, module) { _toggleVisibilityOnClick(); }); let fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU); - fileMenu.addMenuDivider(); - fileMenu.addMenuItem(Commands.FILE_LIVE_FILE_PREVIEW, ""); + fileMenu.addMenuItem(Commands.FILE_LIVE_FILE_PREVIEW, "", Menus.AFTER, Commands.FILE_EXTENSION_MANAGER); + fileMenu.addMenuDivider(Menus.BEFORE, Commands.FILE_LIVE_FILE_PREVIEW); // We always show the live preview panel on startup if there is a preview file setTimeout(async ()=>{ LiveDevelopment.openLivePreview(); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 32a2149fc..512d2b086 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -362,7 +362,7 @@ define({ "CMD_FILE_PASTE": "Paste", "CMD_PROJECT_NEW": "New Project\u2026", "CMD_FILE_NEW_FOLDER": "New Folder", - "CMD_FILE_OPEN": "Open\u2026", + "CMD_FILE_OPEN": "Open Files\u2026", "CMD_RECENT_FILES_OPEN": "Recent Files\u2026", "CMD_ADD_TO_WORKING_SET": "Open To Working Set", "CMD_OPEN_DROPPED_FILES": "Open Dropped Files", From e72e581ca32eb7dc65306ca465f370a7db7f2baa Mon Sep 17 00:00:00 2001 From: abose Date: Sat, 30 Dec 2023 21:30:21 +0530 Subject: [PATCH 07/12] feat: refactor shortcuts to be consistant across desktop and browser --- src/base-config/keyboard.json | 13 ++--- src/command/DefaultMenus.js | 52 ++++++++++++++++--- src/extensions/default/Phoenix/new-project.js | 2 +- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index fa99740fc..70532f4ad 100644 --- a/src/base-config/keyboard.json +++ b/src/base-config/keyboard.json @@ -1,15 +1,12 @@ { - "file.newFolder": [ - "Ctrl-M" + "file.newFile": [ + "Ctrl-1" ], - "file.open": [ - "Ctrl-O" + "file.newFolder": [ + "Ctrl-3" ], "file.close": [ - "Alt-W" - ], - "file.openFolder": [ - "Ctrl-Alt-O" + "Ctrl-`" ], "file.close_all": [ "Alt-Shift-W" diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js index 2293e0a65..2a0c6422c 100644 --- a/src/command/DefaultMenus.js +++ b/src/command/DefaultMenus.js @@ -63,13 +63,49 @@ define(function (require, exports, module) { } } - const fileNewShortcut = Phoenix.browser.isTauri ? "Ctrl-N" : "Ctrl-1"; - const fileNewWindowShortcut = Phoenix.browser.isTauri ? "Ctrl-Shift-N" : "Ctrl-3"; + const isBrowser = !Phoenix.browser.isTauri; + const isDesktop = Phoenix.browser.isTauri; + const fileNewShortcut = isDesktop ? "Ctrl-N" : ""; // Ctrl-1 universal shortcut is set in keyboard.json + // user can use ctrl-t browser shortcut to create new window in browser, so no shortcut in browser. + 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 every 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-` 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, fileNewShortcut); @@ -77,10 +113,10 @@ define(function (require, exports, module) { 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); diff --git a/src/extensions/default/Phoenix/new-project.js b/src/extensions/default/Phoenix/new-project.js index 39091586a..0e1e9fb07 100644 --- a/src/extensions/default/Phoenix/new-project.js +++ b/src/extensions/default/Phoenix/new-project.js @@ -72,7 +72,7 @@ define(function (require, exports, module) { function _addMenuEntries() { CommandManager.register(Strings.CMD_PROJECT_NEW, Commands.FILE_NEW_PROJECT, _showNewProjectDialogue); const fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU); - fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Ctrl-Alt-P", Menus.AFTER, Commands.FILE_NEW_FOLDER); + fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "", Menus.AFTER, Commands.FILE_NEW_FOLDER); } function closeDialogue() { From 167163fd7746c4c3821a25962da925673076fc45 Mon Sep 17 00:00:00 2001 From: abose Date: Sat, 30 Dec 2023 21:48:14 +0530 Subject: [PATCH 08/12] docs: update code docs --- src/command/DefaultMenus.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/command/DefaultMenus.js b/src/command/DefaultMenus.js index 2a0c6422c..17d86acd9 100644 --- a/src/command/DefaultMenus.js +++ b/src/command/DefaultMenus.js @@ -66,12 +66,12 @@ 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 - // user can use ctrl-t browser shortcut to create new window in browser, so no shortcut in browser. + //`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 every used when the user opens a project once in a while. + // and only ever used when the user opens a project once in a while. const openFileShortcut = isDesktop ? "Ctrl-O" : ""; const openFolderShortcut = isBrowser ? "Ctrl-O" : ""; From 20c2a177826d6a5061a9936390c905ce98c7c85c Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 1 Jan 2024 15:07:40 +0530 Subject: [PATCH 09/12] fix: keybindings unit test fialures --- docs/generatedApiDocs/utils/FeatureGate-API.md | 9 +++++++++ .../duplicateShortcuts.json | 4 ++-- test/spec/KeyBindingManager-test-files/keymap.json | 2 +- .../spec/KeyBindingManager-test-files/macKeymap.json | 2 +- test/spec/KeyBindingManager-test.js | 12 ++++++------ 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/docs/generatedApiDocs/utils/FeatureGate-API.md b/docs/generatedApiDocs/utils/FeatureGate-API.md index afa704711..d83aebf59 100644 --- a/docs/generatedApiDocs/utils/FeatureGate-API.md +++ b/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 diff --git a/test/spec/KeyBindingManager-test-files/duplicateShortcuts.json b/test/spec/KeyBindingManager-test-files/duplicateShortcuts.json index 999b5991a..3e09da7b3 100644 --- a/test/spec/KeyBindingManager-test-files/duplicateShortcuts.json +++ b/test/spec/KeyBindingManager-test-files/duplicateShortcuts.json @@ -1,8 +1,8 @@ { "documentation": "https://github.com/adobe/brackets/wiki/Key-Bindings", "overrides": { - "ctrl-2": "file.openFolder", - "Ctrl-2": "file.openFolder", + "ctrl-2": "file.newFile", + "Ctrl-2": "file.newFile", "Ctrl-Alt-4": "view.toggleSidebar", "Alt-Ctrl-4": "view.toggleSidebar" } diff --git a/test/spec/KeyBindingManager-test-files/keymap.json b/test/spec/KeyBindingManager-test-files/keymap.json index 8444272dd..6044f7056 100644 --- a/test/spec/KeyBindingManager-test-files/keymap.json +++ b/test/spec/KeyBindingManager-test-files/keymap.json @@ -1,7 +1,7 @@ { "documentation": "https://github.com/adobe/brackets/wiki/Key-Bindings", "overrides": { - "ctrl-2": "file.openFolder", + "ctrl-2": "file.newFile", "Alt-Ctrl-Backspace": "view.toggleSidebar", "ctrl-l": "navigate.gotoDefinition", "Alt-Ctrl-L": null diff --git a/test/spec/KeyBindingManager-test-files/macKeymap.json b/test/spec/KeyBindingManager-test-files/macKeymap.json index e0e47a777..f76c28f60 100644 --- a/test/spec/KeyBindingManager-test-files/macKeymap.json +++ b/test/spec/KeyBindingManager-test-files/macKeymap.json @@ -1,7 +1,7 @@ { "documentation": "https://github.com/adobe/brackets/wiki/Key-Bindings", "overrides": { - "ctrl-2": "file.openFolder", + "ctrl-2": "file.newFile", "Alt-Cmd-Backspace": "view.toggleSidebar", "ctrl-l": "navigate.gotoDefinition", "Alt-Cmd-L": null diff --git a/test/spec/KeyBindingManager-test.js b/test/spec/KeyBindingManager-test.js index 5107d9760..1e42c3cd1 100644 --- a/test/spec/KeyBindingManager-test.js +++ b/test/spec/KeyBindingManager-test.js @@ -49,7 +49,7 @@ define(function (require, exports, module) { "Alt-Shift-Down": "edit.addCursorToNextLine", "Alt-Shift-Up": "edit.addCursorToPrevLine", "F8": "navigate.gotoFirstProblem", - "Ctrl-Alt-O": "file.openFolder", + "Ctrl-1": "file.newFile", "Ctrl-Alt-H": "view.toggleSidebar", "Ctrl-Shift-O": "navigate.quickOpen", "Ctrl-T": "navigate.gotoDefinition" @@ -60,7 +60,7 @@ define(function (require, exports, module) { "Alt-Shift-Down": "edit.addCursorToNextLine", "Alt-Shift-Up": "edit.addCursorToPrevLine", "Cmd-'": "navigate.gotoFirstProblem", - "Alt-Cmd-O": "file.openFolder", + "Cmd-1": "file.newFile", "Shift-Cmd-H": "view.toggleSidebar", "Shift-Cmd-O": "navigate.quickOpen", "Cmd-T": "navigate.gotoDefinition" @@ -618,7 +618,7 @@ define(function (require, exports, module) { called = true; var msgPrefix = Strings.ERROR_NONEXISTENT_COMMANDS.replace("{0}", ""); expect(message).toMatch(msgPrefix); - expect(message).toMatch("file.openFolder"); + expect(message).toMatch("file.newFile"); expect(message).toMatch("view.toggleSidebar"); return {done: function (callback) { callback(Dialogs.DIALOG_BTN_OK); } }; }; @@ -652,7 +652,7 @@ define(function (require, exports, module) { let reassignedKey1 = (platform === "mac") ? "Alt-Cmd-Backspace" : "Ctrl-Alt-Backspace", reassignedKey2 = (platform === "mac") ? "Cmd-T" : "Ctrl-T"; expect(called).toBeFalse(); - expect(keymap["Ctrl-2"].commandID).toEqual("file.openFolder"); + expect(keymap["Ctrl-2"].commandID).toEqual("file.newFile"); expect(keymap["Alt-Cmd-O"]).toBeFalsy(); expect(keymap["Alt-Ctrl-O"]).toBeFalsy(); @@ -692,7 +692,7 @@ define(function (require, exports, module) { var keymap = KeyBindingManager.getKeymap(), reassignedKey1 = (platform === "mac") ? "Alt-Cmd-Backspace" : "Ctrl-Alt-Backspace", - reassignedKey2 = (platform === "mac") ? "Alt-Cmd-O" : "Ctrl-Alt-O", + reassignedKey2 = "Ctrl-1", reassignedKey3 = (platform === "mac") ? "Alt-Cmd-L" : "Ctrl-Alt-L"; expect(called).toBeFalse(); @@ -702,7 +702,7 @@ define(function (require, exports, module) { expect(keymap[reassignedKey1]).toBeFalsy(); // Default key binding for "file.openFolder" is restored. - expect(keymap[reassignedKey2].commandID).toEqual("file.openFolder"); + expect(keymap[reassignedKey2].commandID).toEqual("file.newFile"); expect(keymap["Ctrl-L"].commandID).toEqual("navigate.gotoDefinition"); expect(keymap[reassignedKey3]).toBeFalsy(); From 1f3fd2275e8a72a92b175235fa5f5c2f6a3fbae9 Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 1 Jan 2024 15:55:07 +0530 Subject: [PATCH 10/12] chore: change expand and collapse folding shortcuts to Alt-Shift-Left/Right, fix mac test fails --- src/extensions/default/CodeFolding/main.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/extensions/default/CodeFolding/main.js b/src/extensions/default/CodeFolding/main.js index c5ce56b1d..c791352b1 100644 --- a/src/extensions/default/CodeFolding/main.js +++ b/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 From b6a9f596eb0c2ba2c2c0a6d1a76746d2e3f4f608 Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 1 Jan 2024 17:37:03 +0530 Subject: [PATCH 11/12] feat: add a simulate platform option for unit test suite in browsers --- src/index.html | 15 ++++++++- test/BootstrapReporterView.js | 12 ++++--- test/SpecRunner.html | 62 ++++++++++++++++++++++++++++++++++- test/spec/SpecRunnerUtils.js | 3 ++ 4 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/index.html b/src/index.html index 5e7db22e2..1eedd6335 100644 --- a/src/index.html +++ b/src/index.html @@ -250,8 +250,21 @@ } // Determine OS/platform + window._getPlatformOverride = function (){ + const currentUrl = window.location.href; + const searchParams = new URLSearchParams(new URL(currentUrl).search); + const platform = searchParams.get('platform'); + const allowedOverRides = ['win', 'mac', 'linux']; + if(allowedOverRides.includes(platform)){ + return platform; + } + return null; + } let platform = "win"; - if (navigator.platform && navigator.platform.match("Mac")) { + if(_getPlatformOverride() && _isTestWindow()){ + platform = _getPlatformOverride(); + console.warn("using platform override: ", platform, "This is only expected to run in tests."); + } else if (navigator.platform && navigator.platform.match("Mac")) { platform = "mac"; } else if (navigator.platform && navigator.platform.indexOf("Linux") >= 0) { platform = "linux"; diff --git a/test/BootstrapReporterView.js b/test/BootstrapReporterView.js index 3becc92e7..dfcef27bb 100644 --- a/test/BootstrapReporterView.js +++ b/test/BootstrapReporterView.js @@ -133,7 +133,8 @@ define(function (require, exports, module) { displayName = suiteName.replace(`${category[0]}:`, ''); } } - let hyperlink = `?spec=${encodeURIComponent(suiteName)}`; + const platformQueryParam = window._getPlatformOverride() ? `&platform=${window._getPlatformOverride()}` : ''; + let hyperlink = `?spec=${encodeURIComponent(suiteName)}${platformQueryParam}`; if(reporter.selectedCategories.length){ hyperlink = `${hyperlink}&category=${reporter.selectedCategories.join(',')}`; } @@ -162,7 +163,8 @@ define(function (require, exports, module) { displayName = suiteName.replace(`${category[0]}:`, ''); } } - let hyperlink = `?spec=${encodeURIComponent(suiteName)}`; + const platformQueryParam = window._getPlatformOverride() ? `&platform=${window._getPlatformOverride()}` : ''; + let hyperlink = `?spec=${encodeURIComponent(suiteName)}${platformQueryParam}`; if(reporter.selectedCategories.length){ hyperlink = `${hyperlink}&category=${reporter.selectedCategories.join(',')}`; } @@ -347,7 +349,8 @@ define(function (require, exports, module) { } // print spec name - let hyperlink = `?spec=${encodeURIComponent(specData.name)}`; + const platformQueryParam = window._getPlatformOverride() ? `&platform=${window._getPlatformOverride()}` : ''; + let hyperlink = `?spec=${encodeURIComponent(specData.name)}${platformQueryParam}`; if(reporter.selectedCategories.length){ hyperlink = `${hyperlink}&category=${reporter.selectedCategories.join(',')}`; } @@ -386,7 +389,8 @@ define(function (require, exports, module) { if (specData.passed && specData.perf) { // add spec name - let hyperlink = `?spec=${encodeURIComponent(specData.name)}`; + const platformQueryParam = window._getPlatformOverride() ? `&platform=${window._getPlatformOverride()}` : ''; + let hyperlink = `?spec=${encodeURIComponent(specData.name)}${platformQueryParam}`; if(reporter.selectedCategories.length){ hyperlink = `${hyperlink}&category=${reporter.selectedCategories.join(',')}`; } diff --git a/test/SpecRunner.html b/test/SpecRunner.html index 0ba248516..116646b6e 100644 --- a/test/SpecRunner.html +++ b/test/SpecRunner.html @@ -170,6 +170,17 @@ return 'Other'; } + + window._getPlatformOverride = function (){ + const currentUrl = window.location.href; + const searchParams = new URLSearchParams(new URL(currentUrl).search); + const platform = searchParams.get('platform'); + const allowedOverRides = ['win', 'mac', 'linux']; + if(allowedOverRides.includes(platform)){ + return platform; + } + return null; + } function getBrowserDetails() { let isChrome = navigator.userAgent.indexOf("Chrome") !== -1; let isEdgeBrowser = navigator.userAgent.indexOf("Edg") !== -1; @@ -231,7 +242,10 @@ } // Determine OS/platform let platform = "win"; - if (navigator.platform && navigator.platform.match("Mac")) { + if(window._getPlatformOverride() && _isTestWindow()){ + console.warn("using platform override: ", platform); + platform = window._getPlatformOverride(); + } else if (navigator.platform && navigator.platform.match("Mac")) { platform = "mac"; } else if (navigator.platform && navigator.platform.indexOf("Linux") >= 0) { platform = "linux"; @@ -419,6 +433,45 @@ controlBar.addEventListener('mouseout', function() { iframeContainer.classList.remove('hovered'); }); + + const simulateOS = document.getElementById("os-select"); + if(Phoenix.browser.isTauri) { + // only available in browsers + simulateOS.classList.add("forced-hidden") + } else { + const platform = window._getPlatformOverride(); + if(platform){ + simulateOS.value = platform; + document.getElementById("all").setAttribute('href', `?category=all&platform=${platform}`); + document.getElementById("unit").setAttribute('href', `?category=unit&platform=${platform}`); + document.getElementById("integration").setAttribute('href', `?category=integration&platform=${platform}`); + document.getElementById("LegacyInteg").setAttribute('href', `?category=LegacyInteg&platform=${platform}`); + document.getElementById("livepreview").setAttribute('href', `?category=livepreview&platform=${platform}`); + document.getElementById("mainview").setAttribute('href', `?category=mainview&platform=${platform}`); + document.getElementById("performance").setAttribute('href', `?category=performance&platform=${platform}`); + document.getElementById("extension").setAttribute('href', `?category=extension&platform=${platform}`); + document.getElementById("individualrun").setAttribute('href', `?category=individualrun&platform=${platform}`); + } + simulateOS.addEventListener('change', function() { + const selectedPlatform = this.value; + const currentUrl = new URL(window.location.href); + // Manually construct the query string. we cant use `currentUrl.searchParams.set` due to + encoding to %20 + let queryString = ''; + currentUrl.search.slice(1).split('&').forEach(paramEqValStr => { + const [key] = paramEqValStr.split('='); + if (key !== 'platform') { + queryString += `${paramEqValStr}&`; + } + }); + + const allowedOverRides = ['win', 'mac', 'linux']; + if(allowedOverRides.includes(selectedPlatform)){ + queryString += `platform=${selectedPlatform}`; + } + + window.location.href = `${currentUrl.origin}${currentUrl.pathname}?${queryString}`; + }); + } } @@ -449,6 +502,13 @@
  • Reset and Reload Tests
  • Toggle Printable Report
    • + diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index 2b02f718a..ffce4af14 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -594,6 +594,9 @@ define(function (require, exports, module) { // disable loading of sample project params.put("skipSampleProjectLoad", true); + if(window._getPlatformOverride()){ + params.put("platform", window._getPlatformOverride()); + } // disable initial dialog for live development params.put("skipLiveDevelopmentInfo", true); From 90a48929df33599179edda200ec9d7bab4879c1c Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 1 Jan 2024 17:40:08 +0530 Subject: [PATCH 12/12] fix: key bindings unit test fail in mac --- test/spec/KeyBindingManager-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/KeyBindingManager-test.js b/test/spec/KeyBindingManager-test.js index 1e42c3cd1..07bbf017b 100644 --- a/test/spec/KeyBindingManager-test.js +++ b/test/spec/KeyBindingManager-test.js @@ -692,7 +692,7 @@ define(function (require, exports, module) { var keymap = KeyBindingManager.getKeymap(), reassignedKey1 = (platform === "mac") ? "Alt-Cmd-Backspace" : "Ctrl-Alt-Backspace", - reassignedKey2 = "Ctrl-1", + reassignedKey2 = (platform === "mac") ? "Cmd-1" : "Ctrl-1", reassignedKey3 = (platform === "mac") ? "Alt-Cmd-L" : "Ctrl-Alt-L"; expect(called).toBeFalse();