From 1120b28c521860f92999be705a3c298a61e64959 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Mon, 17 Aug 2026 15:13:01 +0100 Subject: [PATCH 1/3] fix: focus the tree, not its panel, for the Object Explorer shortcut The Object Explorer shortcut, Shift+Alt+B by default, is meant to put the keyboard into the tree so that the arrow keys move between nodes. It called focus() on the rc-dock tab pane wrapping the tree, and that pane is a plain div with no tabindex, so it cannot take focus at all: the shortcut selected a node and left focus wherever it already was, which for anybody navigating by keyboard means it appeared to do nothing. Nothing needs a tabindex adding. The tree that react-aspen renders inside the panel already carries tabindex="-1", so it is focusable programmatically; the fix is to aim at it, falling back to the panel if the tree is not there so the behaviour cannot get worse than it was. Found whilst reviewing #10254, which made this visible: once the Object Explorer can be collapsed, a shortcut that silently fails to focus it is much easier to notice. --- web/pgadmin/browser/static/js/keyboard.js | 7 +- .../browser/keyboard_left_tree_spec.js | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 web/regression/javascript/browser/keyboard_left_tree_spec.js diff --git a/web/pgadmin/browser/static/js/keyboard.js b/web/pgadmin/browser/static/js/keyboard.js index e9779329903..912a1addc1d 100644 --- a/web/pgadmin/browser/static/js/keyboard.js +++ b/web/pgadmin/browser/static/js/keyboard.js @@ -212,7 +212,12 @@ _.extend(pgBrowser.keyboardNavigation, { // to give React a chance to paint the panel before we move into it. pgAdmin.Browser.Events.trigger(SHOW_OBJECT_EXPLORER_EVENT); setTimeout(()=>{ - document.querySelector('[id="id-object-explorer"]')?.focus(); + const panel = document.querySelector('[id="id-object-explorer"]'); + // Focus the tree rather than the panel around it. The panel is a plain + // div with no tabindex, so focusing it has never done anything; the + // tree carries tabindex="-1" and can actually take focus, which is + // what makes the arrow keys work once the shortcut has been pressed. + (panel?.querySelector('.file-tree') ?? panel)?.focus(); tree.t.select(tree.i); }, 0); }, diff --git a/web/regression/javascript/browser/keyboard_left_tree_spec.js b/web/regression/javascript/browser/keyboard_left_tree_spec.js new file mode 100644 index 00000000000..31c68ca4854 --- /dev/null +++ b/web/regression/javascript/browser/keyboard_left_tree_spec.js @@ -0,0 +1,84 @@ +///////////////////////////////////////////////////////////// +// +// 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'; + +/* The Object Explorer shortcut is meant to put the keyboard into the tree, so + * that the arrow keys move between nodes. It focused the rc-dock tab pane + * around the tree, which is a plain div with no tabindex and therefore cannot + * take focus at all, so the shortcut only ever selected a node and left focus + * wherever it was. */ +describe('keyboardNavigation.bindLeftTree', () => { + let select; + + const buildObjectExplorer = ({withTree = true} = {}) => { + const pane = document.createElement('div'); + pane.id = 'id-object-explorer'; + pane.className = 'dock-tabpane dock-tabpane-active'; + + let tree = null; + if (withTree) { + tree = document.createElement('div'); + tree.className = 'file-tree'; + // As react-aspen renders it: programmatically focusable, not tabbable. + tree.setAttribute('tabindex', '-1'); + pane.appendChild(tree); + } + + document.body.appendChild(pane); + return {pane, tree}; + }; + + beforeEach(() => { + jest.useFakeTimers(); + document.body.innerHTML = ''; + select = jest.fn(); + pgAdmin.Browser.keyboardNavigation.getTreeDetails = () => ({ + t: {select}, i: 'some-tree-item', + }); + }); + + afterEach(() => { + jest.useRealTimers(); + document.body.innerHTML = ''; + }); + + it('moves focus into the tree', () => { + const {tree} = buildObjectExplorer(); + + pgAdmin.Browser.keyboardNavigation.bindLeftTree(); + jest.runAllTimers(); + + expect(document.activeElement).toBe(tree); + expect(select).toHaveBeenCalledWith('some-tree-item'); + }); + + it('falls back to the panel when there is no tree to focus', () => { + buildObjectExplorer({withTree: false}); + + expect(() => { + pgAdmin.Browser.keyboardNavigation.bindLeftTree(); + jest.runAllTimers(); + }).not.toThrow(); + expect(select).toHaveBeenCalled(); + }); + + it('does not throw when the Object Explorer is not in the DOM', () => { + expect(() => { + pgAdmin.Browser.keyboardNavigation.bindLeftTree(); + jest.runAllTimers(); + }).not.toThrow(); + }); +}); From 1faa4616aed78d9190bf5d11f1bfd0e95d8dbc07 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Mon, 17 Aug 2026 15:45:06 +0100 Subject: [PATCH 2/3] Draw the Object Explorer tree's focus indicator ourselves Now that the shortcut actually lands keyboard focus on the tree, the focus indicator becomes something users see routinely, and left to the browser it is an outline-style: auto ring drawn in the host's accent colour. In practice that means it appears orange in one browser and blue in another, and Safari may not draw it at all, so a keyboard user there gets no indication of where focus has gone. Style it with theme.otherVars.activeBorder instead, matching how the dock tabs already indicate focus, so it is consistent across browsers and themes: #326690 on light, #d4d4d4 on dark, #fff on high contrast. The -1px outline offset keeps it inside the scrolling container rather than being clipped at the edges. Verified in the browser: with focus on the tree, the computed outline is "rgb(50, 102, 144) solid 1px" with a -1px offset, in place of the previous "auto" ring. --- .../static/js/Theme/overrides/reactaspen.override.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/web/pgadmin/static/js/Theme/overrides/reactaspen.override.js b/web/pgadmin/static/js/Theme/overrides/reactaspen.override.js index 2be6e2d188e..a8acfae3d60 100644 --- a/web/pgadmin/static/js/Theme/overrides/reactaspen.override.js +++ b/web/pgadmin/static/js/Theme/overrides/reactaspen.override.js @@ -32,6 +32,17 @@ export default function reactAspenOverride(theme) { display: 'inline-block', position: 'relative', width: '100%', + // The tree carries tabindex="-1" and the Object Explorer shortcut + // focuses it deliberately, so draw the focus indicator ourselves. + // Left to the browser this is an outline-style: auto ring, which takes + // the host's accent colour - orange in one browser, blue in another - + // and Safari may not draw it at all, leaving keyboard users with no + // indication of where focus has landed. The negative offset keeps the + // outline inside the scrolling container so it is not clipped. + '&:focus-visible': { + outline: '1px solid ' + theme.otherVars.activeBorder, + outlineOffset: '-1px', + }, '&, & *': { boxSizing: 'border-box', }, From 12f62a8a2197d624559e20aecf400fd319fac1b3 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 1 Sep 2026 12:04:39 +0100 Subject: [PATCH 3/3] Drop the Object Explorer focus fallback that could never work bindLeftTree() fell back to focusing the Object Explorer panel when the tree was not there to focus, which the comment alongside it already admitted has never done anything: the panel is a plain div with no tabindex. The fallback was therefore dead code that read as though it did something. Rather than give the panel a tabindex so that the fallback starts working, the fallback has gone. Landing the keyboard on a container that handles no keys is worse for anyone navigating by keyboard than leaving focus where they left it, and if the tree is missing there is nothing useful for the arrow keys to do anyway. The regression test now asserts that, rather than only asserting that nothing throws. --- web/pgadmin/browser/static/js/keyboard.js | 6 +++++- .../javascript/browser/keyboard_left_tree_spec.js | 12 ++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/web/pgadmin/browser/static/js/keyboard.js b/web/pgadmin/browser/static/js/keyboard.js index 912a1addc1d..9f00c5644f3 100644 --- a/web/pgadmin/browser/static/js/keyboard.js +++ b/web/pgadmin/browser/static/js/keyboard.js @@ -217,7 +217,11 @@ _.extend(pgBrowser.keyboardNavigation, { // div with no tabindex, so focusing it has never done anything; the // tree carries tabindex="-1" and can actually take focus, which is // what makes the arrow keys work once the shortcut has been pressed. - (panel?.querySelector('.file-tree') ?? panel)?.focus(); + // There is deliberately no fallback to the panel: it cannot take focus, + // and giving it a tabindex purely to catch this case would land the + // keyboard on a container that handles no keys, which is worse for + // anyone navigating by keyboard than leaving focus where it was. + panel?.querySelector('.file-tree')?.focus(); tree.t.select(tree.i); }, 0); }, diff --git a/web/regression/javascript/browser/keyboard_left_tree_spec.js b/web/regression/javascript/browser/keyboard_left_tree_spec.js index 31c68ca4854..d7b16510ecb 100644 --- a/web/regression/javascript/browser/keyboard_left_tree_spec.js +++ b/web/regression/javascript/browser/keyboard_left_tree_spec.js @@ -65,13 +65,21 @@ describe('keyboardNavigation.bindLeftTree', () => { expect(select).toHaveBeenCalledWith('some-tree-item'); }); - it('falls back to the panel when there is no tree to focus', () => { - buildObjectExplorer({withTree: false}); + it('leaves focus alone when there is no tree to focus', () => { + const {pane} = buildObjectExplorer({withTree: false}); + const elsewhere = document.createElement('button'); + document.body.appendChild(elsewhere); + elsewhere.focus(); expect(() => { pgAdmin.Browser.keyboardNavigation.bindLeftTree(); jest.runAllTimers(); }).not.toThrow(); + // The panel cannot take focus and is not given a tabindex to make it + // able to, so focus stays where the user left it rather than landing on + // a container that handles no keys. + expect(document.activeElement).toBe(elsewhere); + expect(document.activeElement).not.toBe(pane); expect(select).toHaveBeenCalled(); });