diff --git a/docs/en_US/keyboard_shortcuts.rst b/docs/en_US/keyboard_shortcuts.rst index 5f766637865..32091d8e44f 100644 --- a/docs/en_US/keyboard_shortcuts.rst +++ b/docs/en_US/keyboard_shortcuts.rst @@ -44,9 +44,9 @@ When using main browser window, the following keyboard shortcuts are available: +----------------------------+--------------------+------------------------------------+ | Shift + Alt + s | Shift + Option + s | Search objects | +----------------------------+--------------------+------------------------------------+ - | Shift + Alt + [ | Shift + Option + [ | Tabbed panel backward | + | Ctrl + Alt + [ | Ctrl + Option + [ | Tabbed panel backward | +----------------------------+--------------------+------------------------------------+ - | Shift + Alt + ] | Shift + Option + ] | Tabbed panel forward | + | Ctrl + Alt + ] | Ctrl + Option + ] | Tabbed panel forward | +----------------------------+--------------------+------------------------------------+ | Shift + Alt + w | Shift + Ctrl + w | Close tab panel | +----------------------------+--------------------+------------------------------------+ @@ -98,6 +98,8 @@ When using the syntax-highlighting SQL editors, the following shortcuts are avai +--------------------------+----------------------+-------------------------------------+ | Ctrl + / | Cmd + / | Comment/Uncomment code (Block) | +--------------------------+----------------------+-------------------------------------+ + | Ctrl + Shift + d | Cmd + Shift + d | Duplicate current line/selection | + +--------------------------+----------------------+-------------------------------------+ | Ctrl + a | Cmd + a | Select all | +--------------------------+----------------------+-------------------------------------+ | Ctrl + c | Cmd + c | Copy selected text to the clipboard | diff --git a/web/pgadmin/browser/register_browser_preferences.py b/web/pgadmin/browser/register_browser_preferences.py index d2fe51dc5fa..064f23d2a2c 100644 --- a/web/pgadmin/browser/register_browser_preferences.py +++ b/web/pgadmin/browser/register_browser_preferences.py @@ -170,9 +170,9 @@ def register_browser_preferences(self): 'keyboardshortcut', { 'alt': True, - 'shift': True, - 'control': False, - 'key': {'key_code': 91, 'char': '['} + 'shift': False, + 'control': True, + 'key': {'key_code': 219, 'char': '['} }, category_label=PREF_LABEL_KEYBOARD_SHORTCUTS, fields=fields @@ -200,9 +200,9 @@ def register_browser_preferences(self): 'keyboardshortcut', { 'alt': True, - 'shift': True, - 'control': False, - 'key': {'key_code': 93, 'char': ']'} + 'shift': False, + 'control': True, + 'key': {'key_code': 221, 'char': ']'} }, category_label=PREF_LABEL_KEYBOARD_SHORTCUTS, fields=fields diff --git a/web/pgadmin/browser/static/js/keyboard.js b/web/pgadmin/browser/static/js/keyboard.js index 662b8dd0239..cf9c7bbbbbd 100644 --- a/web/pgadmin/browser/static/js/keyboard.js +++ b/web/pgadmin/browser/static/js/keyboard.js @@ -150,42 +150,49 @@ _.extend(pgBrowser.keyboardNavigation, { bindRightPanel: function(event, combo) { const self = this; const shortcutObj = this.keyboardShortcut; - const activeElement = document.activeElement; + const rootDock = document.getElementById('root'); + if (!rootDock) return; - if (activeElement.closest('.dock-tab-btn')) { - const currDockTab = activeElement.closest('.dock-tab-btn'); - const dockLayout = currDockTab.closest('.dock-layout'); - const dockLayoutTabs = dockLayout ? Array.from(dockLayout.querySelectorAll('.dock-tab-btn')) : null; + // Find the active workspace tab independently of where the keyboard focus + // currently sits. Previously this relied on document.activeElement, which + // breaks when focus is inside a tool's own (nested) dock layout - the SQL + // editor, an ERD/Schema Diff canvas - or inside the PSQL iframe, because + // the resolved tab id then belonged to the tool's inner tab rather than a + // main workspace tab (issue #7232). + // + // rc-dock renders the object explorer and the workspace as separate + // tab-sets, and each marks its own active tab with `dock-tab-active`, so + // prefer the active tab that is not the object explorer. + // + // Only the outermost dock layout is considered. The SQL editor, ERD and + // Schema Diff each render a DockLayout of their own inside a workspace + // tab, and their active tabs (Data Output, Messages, Notifications and + // so on) carry the same `dock-tab-active` class, so a search across the + // whole tree can land on one of those and cycle a tool's inner tabs + // instead of the workspace tabs it was asked to move between. + const topDockLayout = rootDock.querySelector('.dock-layout'); + if (!topDockLayout) return; - if (dockLayoutTabs && dockLayoutTabs.length > 1) { - const activeTabIndex = dockLayoutTabs.indexOf(currDockTab); - self._focusTab(dockLayoutTabs, activeTabIndex, shortcutObj, combo); - } - } - else if (activeElement.nodeName === 'IFRAME' || activeElement.closest('.dock-tabpane.dock-tabpane-active')) { - let activeTabId = ''; - activeTabId = (activeElement.nodeName === 'IFRAME') ? activeElement.id : activeElement.closest('.dock-tabpane.dock-tabpane-active').id; - const dockLayout = document.getElementById('root'); - const dockLayoutTabs = dockLayout ? Array.from(dockLayout.querySelectorAll('.dock-tab-btn')) : null; + const activeTabBtns = Array.from( + topDockLayout.querySelectorAll('.dock-tab.dock-tab-active .dock-tab-btn') + ).filter(tab => tab.closest('.dock-layout') === topDockLayout); + const activeTabBtn = + activeTabBtns.find(tab => !tab.id.includes('id-object-explorer')) || + activeTabBtns[0]; + if (!activeTabBtn) return; - if (dockLayoutTabs && dockLayoutTabs.length > 1 && activeTabId) { - const activeTabIndex = dockLayoutTabs.findIndex(tab => tab.id.slice(14) === activeTabId); - self._focusTab(dockLayoutTabs, activeTabIndex, shortcutObj, combo); - } - } - else if (activeElement === document.body || document.querySelector('div[data-test="app-menu-bar"]')) { - const activeTabs = document.getElementsByClassName('dock-tabpane dock-tabpane-active'); + // Restrict navigation to the tabs of the same tab-set (dock panel) as the + // active tab, so cycling stays within the workspace tabs and does not + // include the object explorer or a tool's nested tabs. + const panel = activeTabBtn.closest('.dock-panel'); + const dockLayoutTabs = panel ? Array.from( + panel.querySelectorAll('.dock-tab-btn')) + .filter(tab => tab.closest('.dock-panel') === panel && + tab.closest('.dock-layout') === topDockLayout) : []; - if (activeTabs.length > 1) { - const activeTabId = activeTabs[1].id; - const dockLayout = document.getElementById('root'); - const dockLayoutTabs = dockLayout ? Array.from(dockLayout.querySelectorAll('.dock-tab-btn')) : null; - - if (dockLayoutTabs && dockLayoutTabs.length > 1 && activeTabId) { - const activeTabIndex = dockLayoutTabs.findIndex(tab => tab.id.slice(14) === activeTabId); - self._focusTab(dockLayoutTabs, activeTabIndex, shortcutObj, combo); - } - } + if (dockLayoutTabs.length > 1) { + const activeTabIndex = dockLayoutTabs.indexOf(activeTabBtn); + self._focusTab(dockLayoutTabs, activeTabIndex, shortcutObj, combo); } }, _focusTab: function(dockLayoutTabs, activeTabIdx, shortcut_obj, combo){ diff --git a/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx b/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx index 3c83f8e7ab3..bf3067ccb3f 100644 --- a/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx +++ b/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx @@ -97,7 +97,7 @@ export default function SchemaDialogView({ ); }; - const save = (changeData) => { + const save = (changeData, onSaved) => { props.onSave(schemaState.isNew, changeData) .then(()=>{ if(schema.informText) { @@ -106,6 +106,7 @@ export default function SchemaDialogView({ schema.informText, ); } + onSaved?.(); }).catch((err)=>{ schemaState.setError({ name: 'apierror', @@ -119,7 +120,11 @@ export default function SchemaDialogView({ }); }; - const onSaveClick = () => { + // closeOnSave is only ever passed explicitly as true, by the Ctrl/Cmd+Enter + // handler below. The Save button's onClick passes its click event instead, + // which is never === true, so a plain Save click keeps its existing + // behaviour (some dialogs, e.g. object properties, stay open after Save). + const onSaveClick = (closeOnSave) => { // Do nothing when there is no change or there is an error if ( !schemaState._changes || Object.keys(schemaState._changes).length === 0 || @@ -129,15 +134,17 @@ export default function SchemaDialogView({ setSaving(true); setLoaderText(schemaState.customLoadingText || gettext('Saving...')); + const onSaved = closeOnSave === true ? () => props.onClose?.() : undefined; + if (!schema.warningText) { - save(schemaState.changes(true)); + save(schemaState.changes(true), onSaved); return; } Notifier.confirm( gettext('Warning'), schema.warningText, - () => { save(schemaState.changes(true)); }, + () => { save(schemaState.changes(true), onSaved); }, () => { setSaving(false); setLoaderText(''); @@ -177,9 +184,35 @@ export default function SchemaDialogView({ return ; }; + const onKeyDown = (e) => { + // Ctrl/Cmd+Enter saves and closes the dialog from anywhere within it + // (issue #7167). onSaveClick is a no-op when there is nothing to save or + // there is a validation error, so this is safe to call unconditionally. + if ((e.ctrlKey || e.metaKey) && !e.altKey && e.key === 'Enter') { + e.preventDefault(); + onSaveClick(true); + return; + } + + // Escape closes the dialog, mirroring the Close button (issue #5691). + // This is needed for dialogs rendered as dockable panels (Properties, + // Backup, and other utility dialogs); dialogs rendered inside a MUI modal + // already close on Escape, so skip those to avoid a double close. The + // !e.defaultPrevented guard lets an inner control that handles Escape + // (e.g. an open dropdown) consume it first. + if (e.key === 'Escape' && !e.defaultPrevented && props.onClose && + !e.currentTarget.closest('.MuiDialog-root')) { + e.preventDefault(); + props.onClose(); + } + }; + /* I am Groot */ - return useMemo(() => - + // Only the children are memoized: the wrapper carries onKeyDown, which + // closes over props.onClose and onSaveClick, and memoizing it would pin + // whichever versions of those existed when the deps last changed. + const dialogContent = useMemo(() => + <> @@ -234,8 +267,10 @@ export default function SchemaDialogView({ } - , [schema._id, viewHelperProps.mode, resetKey] + , [schema._id, viewHelperProps.mode, resetKey] ); + + return {dialogContent}; } SchemaDialogView.propTypes = { diff --git a/web/pgadmin/static/js/components/ReactCodeMirror/components/Editor.jsx b/web/pgadmin/static/js/components/ReactCodeMirror/components/Editor.jsx index 013aa3a03cf..cc9b54448f6 100644 --- a/web/pgadmin/static/js/components/ReactCodeMirror/components/Editor.jsx +++ b/web/pgadmin/static/js/components/ReactCodeMirror/components/Editor.jsx @@ -30,7 +30,7 @@ import { keymap, } from '@codemirror/view'; import { EditorState, Compartment } from '@codemirror/state'; -import { history, defaultKeymap, historyKeymap, indentLess, indentMore, deleteCharBackwardStrict } from '@codemirror/commands'; +import { history, defaultKeymap, historyKeymap, indentLess, indentMore, deleteCharBackwardStrict, copyLineDown } from '@codemirror/commands'; import { closeBrackets, autocompletion, closeBracketsKeymap, completionKeymap, acceptCompletion } from '@codemirror/autocomplete'; import { foldGutter, @@ -140,6 +140,12 @@ const defaultExtensions = [ key: 'Backspace', preventDefault: true, run: deleteCharBackwardStrict, + },{ + // Duplicate the current line, or the selected lines if there is a + // selection (issue #3834). + key: 'Mod-Shift-d', + preventDefault: true, + run: copyLineDown, }]), PgSQL.language.data.of({ autocomplete: false, diff --git a/web/regression/javascript/SchemaView/SchemaDialogViewKeyboard.spec.js b/web/regression/javascript/SchemaView/SchemaDialogViewKeyboard.spec.js new file mode 100644 index 00000000000..fc022a8809d --- /dev/null +++ b/web/regression/javascript/SchemaView/SchemaDialogViewKeyboard.spec.js @@ -0,0 +1,150 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { act, fireEvent, render, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import BaseUISchema from 'sources/SchemaView/base_schema.ui'; +import SchemaView from '../../../pgadmin/static/js/SchemaView'; +import { TestSchema } from './TestSchema.ui'; +import { withBrowser } from '../genericFunctions'; + +// A single required field is all the Ctrl/Cmd+Enter save-and-close tests +// need; TestSchema's nested tab and row collection require a lot more +// simulated input just to get to a savable state. +class MinimalSchema extends BaseUISchema { + constructor() { + super({field1: null}); + } + + get baseFields() { + return [ + { + id: 'field1', label: 'Field1', type: 'text', group: null, + mode: ['properties', 'edit', 'create'], disabled: false, visible: true, + }, + ]; + } +} + +// Escape closes a dialog rendered as a dockable panel (issue #5691). The +// handler sits on the dialog wrapper, which must not be memoized along with +// the dialog body: the body only changes with the schema, the mode or the +// reset key, whereas onClose is a fresh function on most parent renders, and +// a memoized wrapper would keep calling whichever one it captured first. +describe('SchemaDialogView keyboard handling', () => { + const SchemaViewWithBrowser = withBrowser(SchemaView); + const user = userEvent.setup(); + + const dialog = (schema, onClose) => ( + Promise.resolve())} + onClose={onClose} + onHelp={jest.fn()} + onEdit={jest.fn()} + onDataChange={jest.fn()} + hasSQL={false} + disableSqlHelp={true} + disableDialogHelp={true} + /> + ); + + const renderDialog = async (onClose, onSave = jest.fn(() => Promise.resolve()), schema = new TestSchema()) => { + let ctrl; + await act(async () => { + ctrl = render( + + ); + }); + return ctrl; + }; + + const pressEscape = async (ctrl) => { + await act(async () => { + fireEvent.keyDown(ctrl.container.firstChild, {key: 'Escape'}); + }); + }; + + const pressCtrlEnter = async (ctrl) => { + await act(async () => { + fireEvent.keyDown(ctrl.container.firstChild, {key: 'Enter', ctrlKey: true}); + }); + }; + + it('closes the dialog on Escape', async () => { + const onClose = jest.fn(); + const ctrl = await renderDialog(onClose); + + await pressEscape(ctrl); + + expect(onClose).toHaveBeenCalled(); + }); + + it('calls the current onClose, not the one from the first render', + async () => { + const firstOnClose = jest.fn(); + const secondOnClose = jest.fn(); + // The same schema throughout: the memo deps are the schema id, the mode + // and the reset key, so this is the case where nothing invalidates the + // memo and only the callback has changed. + const schema = new TestSchema(); + + let ctrl; + await act(async () => { + ctrl = render(dialog(schema, firstOnClose)); + }); + + // The parent re-renders with a new callback, as it does whenever it + // defines onClose inline. + await act(async () => { + ctrl.rerender(dialog(schema, secondOnClose)); + }); + + await pressEscape(ctrl); + + expect(secondOnClose).toHaveBeenCalled(); + expect(firstOnClose).not.toHaveBeenCalled(); + }); + + // Ctrl/Cmd+Enter is meant to save and close in one step (issue #7167). + // onSaveClick alone is not enough: it only calls onSave, so this covers the + // dialog actually closing once that save resolves. + it('saves and closes the dialog on Ctrl/Cmd+Enter', async () => { + const onClose = jest.fn(); + const onSave = jest.fn(() => Promise.resolve()); + const ctrl = await renderDialog(onClose, onSave, new MinimalSchema()); + + // Wait for the dialog's auto-focus to settle before typing, as the other + // SchemaDialogView specs do. + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 500)); + }); + await user.type(ctrl.container.querySelector('[name="field1"]'), 'val1'); + + await pressCtrlEnter(ctrl); + + expect(onSave).toHaveBeenCalled(); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + }); +}); diff --git a/web/regression/javascript/browser/keyboard_navigation_spec.js b/web/regression/javascript/browser/keyboard_navigation_spec.js new file mode 100644 index 00000000000..357c4c1d08d --- /dev/null +++ b/web/regression/javascript/browser/keyboard_navigation_spec.js @@ -0,0 +1,176 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +// keyboard.js reaches pgadmin.js by relative path, which skips the +// sources/pgadmin alias that maps to the fake, so point it there explicitly. +jest.mock('../../../pgadmin/static/js/pgadmin', () => + jest.requireActual('../fake_pgadmin')); + +import pgAdmin from 'sources/pgadmin'; +import '../../../pgadmin/browser/static/js/keyboard'; + +// Cycling between workspace tabs must stay in the workspace: the object +// explorer is a tab-set of its own, and the SQL editor, ERD and Schema Diff +// each render a nested DockLayout whose own tabs carry the same +// dock-tab-active class (issue #7232). +describe('keyboardNavigation.bindRightPanel', () => { + const shortcutObj = { + tabbed_panel_forward: 'ctrl+alt+]', + tabbed_panel_backward: 'ctrl+alt+[', + close_tab_panel: 'shift+alt+w', + }; + + const tabBtn = (id, isActive) => { + const tab = document.createElement('div'); + tab.className = isActive ? 'dock-tab dock-tab-active' : 'dock-tab'; + const btn = document.createElement('div'); + btn.className = 'dock-tab-btn'; + btn.id = `rc-dock-tab-btn-${id}`; + tab.appendChild(btn); + return tab; + }; + + const panel = (tabs) => { + const el = document.createElement('div'); + el.className = 'dock-panel'; + tabs.forEach((tab) => el.appendChild(tab)); + return el; + }; + + /* A workspace holding the object explorer, three workspace tabs and, inside + * the active workspace tab, a tool with its own dock layout. */ + const buildLayout = ({activeWorkspaceTab = 'id-dashboard'} = {}) => { + const root = document.createElement('div'); + root.id = 'root'; + + const topLayout = document.createElement('div'); + topLayout.className = 'dock-layout'; + root.appendChild(topLayout); + + topLayout.appendChild(panel([tabBtn('id-object-explorer', true)])); + + const workspaceTabs = ['id-dashboard', 'id-properties', 'id-sql'].map( + (id) => tabBtn(id, id === activeWorkspaceTab)); + const workspacePanel = panel(workspaceTabs); + topLayout.appendChild(workspacePanel); + + // The tool's own dock layout, nested inside the workspace panel exactly + // as the SQL editor's is. + const innerLayout = document.createElement('div'); + innerLayout.className = 'dock-layout'; + innerLayout.appendChild(panel([ + tabBtn('id-dataoutput', true), + tabBtn('id-messages', false), + ])); + workspacePanel.appendChild(innerLayout); + + document.body.appendChild(root); + return {root, workspacePanel}; + }; + + let focusTabSpy; + + beforeEach(() => { + document.body.innerHTML = ''; + pgAdmin.Browser.keyboardNavigation.keyboardShortcut = shortcutObj; + focusTabSpy = jest.spyOn( + pgAdmin.Browser.keyboardNavigation, '_focusTab' + ).mockImplementation(() => {}); + }); + + afterEach(() => { + focusTabSpy.mockRestore(); + document.body.innerHTML = ''; + }); + + it('cycles the workspace tabs, not a tool\'s nested tabs', () => { + buildLayout(); + + pgAdmin.Browser.keyboardNavigation.bindRightPanel( + new Event('keydown'), {key: shortcutObj.tabbed_panel_forward}); + + expect(focusTabSpy).toHaveBeenCalled(); + const [tabs, activeIdx] = focusTabSpy.mock.calls[0]; + const ids = tabs.map((tab) => tab.id); + + expect(ids).toEqual([ + 'rc-dock-tab-btn-id-dashboard', + 'rc-dock-tab-btn-id-properties', + 'rc-dock-tab-btn-id-sql', + ]); + // Neither the nested tool tabs nor the object explorer may take part. + expect(ids).not.toContain('rc-dock-tab-btn-id-dataoutput'); + expect(ids).not.toContain('rc-dock-tab-btn-id-object-explorer'); + expect(tabs[activeIdx].id).toBe('rc-dock-tab-btn-id-dashboard'); + }); + + it('starts from whichever workspace tab is active', () => { + buildLayout({activeWorkspaceTab: 'id-sql'}); + + pgAdmin.Browser.keyboardNavigation.bindRightPanel( + new Event('keydown'), {key: shortcutObj.tabbed_panel_backward}); + + const [tabs, activeIdx] = focusTabSpy.mock.calls[0]; + expect(tabs[activeIdx].id).toBe('rc-dock-tab-btn-id-sql'); + }); + + /* The selection must not depend on where the nested layout happens to sit + * in the DOM: rc-dock is free to order panels as it likes, and an inner + * layout appearing first would otherwise win the search for the active + * tab and cycle a tool's own tabs. */ + it('ignores nested tool tabs even when they come first in the DOM', () => { + const root = document.createElement('div'); + root.id = 'root'; + const topLayout = document.createElement('div'); + topLayout.className = 'dock-layout'; + root.appendChild(topLayout); + + const innerLayout = document.createElement('div'); + innerLayout.className = 'dock-layout'; + innerLayout.appendChild(panel([ + tabBtn('id-dataoutput', true), + tabBtn('id-messages', false), + ])); + topLayout.appendChild(innerLayout); + + topLayout.appendChild(panel([tabBtn('id-object-explorer', true)])); + topLayout.appendChild(panel([ + tabBtn('id-dashboard', false), + tabBtn('id-sql', true), + ])); + document.body.appendChild(root); + + pgAdmin.Browser.keyboardNavigation.bindRightPanel( + new Event('keydown'), {key: shortcutObj.tabbed_panel_forward}); + + expect(focusTabSpy).toHaveBeenCalled(); + const [tabs, activeIdx] = focusTabSpy.mock.calls[0]; + const ids = tabs.map((tab) => tab.id); + expect(ids).toEqual([ + 'rc-dock-tab-btn-id-dashboard', + 'rc-dock-tab-btn-id-sql', + ]); + expect(tabs[activeIdx].id).toBe('rc-dock-tab-btn-id-sql'); + }); + + it('does nothing when only the object explorer is present', () => { + const root = document.createElement('div'); + root.id = 'root'; + const topLayout = document.createElement('div'); + topLayout.className = 'dock-layout'; + topLayout.appendChild(panel([tabBtn('id-object-explorer', true)])); + root.appendChild(topLayout); + document.body.appendChild(root); + + pgAdmin.Browser.keyboardNavigation.bindRightPanel( + new Event('keydown'), {key: shortcutObj.tabbed_panel_forward}); + + expect(focusTabSpy).not.toHaveBeenCalled(); + }); +});