From 2e8b20ccac59ac0b0e1a7580d6a295e9c42c7677 Mon Sep 17 00:00:00 2001 From: hehoon <100522372+hehoon@users.noreply.github.com> Date: Sat, 13 Dec 2025 16:40:58 +0100 Subject: [PATCH 01/20] Tree stores expanded state of nodes in local storage --- .../public/common/enums/storageKeys.enum.js | 1 + .../public/object/ObjectTree.class.js | 128 ++++++++++++++++-- 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/QualityControl/public/common/enums/storageKeys.enum.js b/QualityControl/public/common/enums/storageKeys.enum.js index 54953dd0f..2c6f0c10f 100644 --- a/QualityControl/public/common/enums/storageKeys.enum.js +++ b/QualityControl/public/common/enums/storageKeys.enum.js @@ -20,4 +20,5 @@ export const StorageKeysEnum = Object.freeze({ OBJECT_VIEW_LEFT_PANEL_WIDTH: 'object-view-left-panel-width', OBJECT_VIEW_INFO_VISIBILITY_SETTING: 'object-view-info-visibility-setting', + OBJECT_TREE_OPEN_NODES: 'object-tree-open-nodes', }); diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 4135b5e9b..0cbee3998 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -12,7 +12,8 @@ * or submit itself to any jurisdiction. */ -import { Observable } from '/js/src/index.js'; +import { BrowserStorage, Observable, sessionService } from '/js/src/index.js'; +import { StorageKeysEnum } from '../common/enums/storageKeys.enum.js'; /** * This class allows to transforms objects names (A/B/C) into a tree that can have @@ -27,6 +28,7 @@ export default class ObjectTree extends Observable { */ constructor(name, parent) { super(); + this.storage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_OPEN_NODES); this.initTree(name, parent); } @@ -46,12 +48,110 @@ export default class ObjectTree extends Observable { this.pathString = ''; // 'A/B' } + /** + * Load the expanded/collapsed state for this node and its children from localStorage. + * Updates the `open` property for the current node and recursively for all children. + */ + loadExpandedNodes() { + if (!this.parent) { + // The main node may not be collapsable or expandable. + // Because of this we also have to load the expanded state of their direct children. + this.children.forEach((child) => child.loadExpandedNodes()); + } + + const session = sessionService.get(); + const key = session.personid.toString(); + + // We traverse the path to reach the parent object of this node + let parentNode = this.storage.getLocalItem(key) ?? {}; + for (let i = 0; i < this.path.length - 1; i++) { + parentNode = parentNode[this.path[i]]; + if (!parentNode) { + // Cannot expand marked node because parent path does not exist + return; + } + } + + this._applyExpandedNodesRecursive(parentNode, this); + } + + /** + * Recursively traverse the stored data and update the tree nodes + * @param {object} data - The current level of the hierarchical expanded nodes object + * @param {ObjectTree} treeNode - The tree node to update + */ + _applyExpandedNodesRecursive(data, treeNode) { + if (data[treeNode.name]) { + treeNode.open = true; + Object.keys(data[treeNode.name]).forEach((childName) => { + const child = treeNode.children.find((child) => child.name === childName); + if (child) { + this._applyExpandedNodesRecursive(data[treeNode.name], child); + } + }); + } + }; + + /** + * Persist the current node's expanded/collapsed state in localStorage. + */ + storeExpandedNodes() { + const session = sessionService.get(); + const key = session.personid.toString(); + const data = this.storage.getLocalItem(key) ?? {}; + + // We traverse the path to reach the parent object of this node + let parentNode = data; + for (let i = 0; i < this.path.length - 1; i++) { + const pathKey = this.path[i]; + if (!parentNode[pathKey]) { + if (!this.open) { + // Cannot remove marked node because parent path does not exist + // Due to this the marked node also does not exist (so there is nothing to remove) + return; + } + + // Parent path does not exist, we create it here so we can mark a deeper node + parentNode[pathKey] = {}; + } + + parentNode = parentNode[pathKey]; + } + + if (this.open) { + this._markExpandedNodesRecursive(parentNode, this); + this.storage.setLocalItem(key, data); + } else if (parentNode[this.name]) { + // Deleting from `parentNode` directly updates the `data` object + delete parentNode[this.name]; + this.storage.setLocalItem(key, data); + } + } + + /** + * Recursively mark a node and all open children in the hierarchical "expanded nodes" object. + * This method updates `data` to reflect the current node's expanded state: + * - If the node has any open children, it creates an object branch and recursively marks those children. + * - If the node has no open children (or is a leaf), it stores a marker value `{}`. + * @param {object} data - The current level in the hierarchical data object where nodes are stored. + * @param {ObjectTree} treeNode - The tree node whose expanded state should be stored. + */ + _markExpandedNodesRecursive(data, treeNode) { + if (!data[treeNode.name]) { + data[treeNode.name] = {}; + } + treeNode.children + .filter((child) => child.open) + .forEach((child) => this._markExpandedNodesRecursive(data[treeNode.name], child)); + }; + /** * Toggle this node (open/close) * @returns {undefined} */ toggle() { this.open = !this.open; + this.storeExpandedNodes(); this.notify(); } @@ -70,6 +170,7 @@ export default class ObjectTree extends Observable { openAll() { this.open = true; this.children.forEach((child) => child.openAll()); + this.storeExpandedNodes(); this.notify(); } @@ -80,6 +181,7 @@ export default class ObjectTree extends Observable { closeAll() { this.open = false; this.children.forEach((child) => child.closeAll()); + this.storeExpandedNodes(); this.notify(); } @@ -97,15 +199,14 @@ export default class ObjectTree extends Observable { * addChild(o, [], ['A', 'B']) // end inserting, affecting B * @returns {undefined} */ - addChild(object, path, pathParent) { + _addChild(object, path = undefined, pathParent = []) { // Fill the path argument through recursive call if (!path) { if (!object.name) { throw new Error('Object name must exist'); } path = object.name.split('/'); - this.addChild(object, path, []); - this.notify(); + this._addChild(object, path); return; } @@ -134,15 +235,26 @@ export default class ObjectTree extends Observable { } // Pass to child - subtree.addChild(object, path, fullPath); + subtree._addChild(object, path, fullPath); } /** - * Add a list of objects by calling `addChild` + * Add a single object as a child node + * @param {object} object - child to be added + */ + addOneChild(object) { + this._addChild(object); + this.loadExpandedNodes(); + this.notify(); + } + + /** + * Add a list of objects as child nodes * @param {Array} objects - children to be added - * @returns {undefined} */ addChildren(objects) { - objects.forEach((object) => this.addChild(object)); + objects.forEach((object) => this._addChild(object)); + this.loadExpandedNodes(); + this.notify(); } } From f19578d0d3763d1bcc00f22c559b0dfbc510221f Mon Sep 17 00:00:00 2001 From: hehoon <100522372+hehoon@users.noreply.github.com> Date: Sat, 13 Dec 2025 18:22:10 +0100 Subject: [PATCH 02/20] Add a quick (and hacky) workaround for local storage reset due to non-isolated tests. --- .../test/public/features/filterTest.test.js | 8 +++++++ .../test/public/pages/object-tree.test.js | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/QualityControl/test/public/features/filterTest.test.js b/QualityControl/test/public/features/filterTest.test.js index bb4130d09..382641cb7 100644 --- a/QualityControl/test/public/features/filterTest.test.js +++ b/QualityControl/test/public/features/filterTest.test.js @@ -13,6 +13,8 @@ import { strictEqual } from 'node:assert'; import { delay } from '../../testUtils/delay.js'; +import { removeLocalStorage } from '../../testUtils/localStorage.js'; +import { StorageKeysEnum } from '../../../public/common/enums/storageKeys.enum.js'; export const filterTests = async (url, page, timeout = 5000, testParent) => { await testParent.test('filter should persist between pages', { timeout }, async () => { @@ -84,6 +86,12 @@ export const filterTests = async (url, page, timeout = 5000, testParent) => { }); await testParent.test('ObjectTreePage should apply filters for the objects', { timeout }, async () => { + // Ideally, tests should be isolated and not depend on each other. + // Currently, some tests rely on shared localStorage or page state changes from previous tests. + // As a workaround, we do targeted cleanup here to prevent issues in later tests. + const personid = await page.evaluate(() => window.model.session.personid); + await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); + await page.goto( `${url}?page=objectTree`, { waitUntil: 'networkidle0' }, diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 45788051d..573af3500 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -11,9 +11,9 @@ * or submit itself to any jurisdiction. */ -import { strictEqual, ok, deepStrictEqual } from 'node:assert'; +import { strictEqual, ok, deepStrictEqual, notDeepStrictEqual } from 'node:assert'; import { delay } from '../../testUtils/delay.js'; -import { getLocalStorage } from '../../testUtils/localStorage.js'; +import { getLocalStorage, getLocalStorageAsJson, removeLocalStorage } from '../../testUtils/localStorage.js'; import { StorageKeysEnum } from '../../../public/common/enums/storageKeys.enum.js'; const OBJECT_TREE_PAGE_PARAM = '?page=objectTree'; @@ -43,7 +43,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ok(rowsCount > 1); // more than 1 object in the tree }); - await testParent.test('should not preserve state if refreshed not in run mode', { timeout }, async () => { + await testParent.test('should preserve state if refreshed', { timeout }, async () => { const tbodyPath = 'section > div > div > div > table > tbody'; await page.locator(`${tbodyPath} > tr:nth-child(2)`).click(); await page.reload({ waitUntil: 'networkidle0' }); @@ -51,7 +51,14 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) const rowCount = await page.evaluate(() => document.querySelectorAll('section > div > div > div > table > tbody > tr').length); - strictEqual(rowCount, 2); + // Ideally, tests should be isolated and not depend on each other. + // Currently, some tests rely on shared localStorage or page state changes from previous tests. + // As a workaround, we do targeted cleanup here to prevent issues in later tests. + const personid = await page.evaluate(() => window.model.session.personid); + await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); + await page.reload({ waitUntil: 'networkidle0' }); + + strictEqual(rowCount, 3); }); await testParent.test('should have a button to sort by (default "Name" ASC)', async () => { @@ -134,6 +141,12 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) 'should maintain panel width from localStorage on page reload', { timeout }, async () => { + // Ideally, tests should be isolated and not depend on each other. + // Currently, some tests rely on shared localStorage or page state changes from previous tests. + // As a workaround, we do targeted cleanup here to prevent issues in later tests. + const personid = await page.evaluate(() => window.model.session.personid); + await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); + const dragAmount = 35; await page.reload({ waitUntil: 'networkidle0' }); await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(2)').click()); From 3f38aaa03353ab7c0c9dcdcc011865f3bbd43370 Mon Sep 17 00:00:00 2001 From: hehoon <100522372+hehoon@users.noreply.github.com> Date: Sat, 13 Dec 2025 18:22:39 +0100 Subject: [PATCH 03/20] Add a test to make sure the local storage is updated upon clicking a tree node element --- .../test/public/pages/object-tree.test.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 573af3500..3bd40b50f 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -43,6 +43,30 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ok(rowsCount > 1); // more than 1 object in the tree }); + await testParent.test('should update local storage when tree node is clicked', { timeout }, async () => { + const selector = 'section > div > div > div > table > tbody > tr:nth-child(2)'; + const personid = await page.evaluate(() => window.model.session.personid); + const storageKey = `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`; + + await page.locator(selector).click(); + const localStorageBefore = await getLocalStorageAsJson(page, storageKey); + + await page.locator(selector).click(); + const localStorageAfter = await getLocalStorageAsJson(page, storageKey); + + // Ideally, tests should be isolated and not depend on each other. + // Currently, some tests rely on shared localStorage or page state changes from previous tests. + // As a workaround, we do targeted cleanup here to prevent issues in later tests. + await removeLocalStorage(page, storageKey); + await page.reload({ waitUntil: 'networkidle0' }); + + notDeepStrictEqual( + localStorageBefore, + localStorageAfter, + 'local storage should have changed after clicking a tree node', + ); + }); + await testParent.test('should preserve state if refreshed', { timeout }, async () => { const tbodyPath = 'section > div > div > div > table > tbody'; await page.locator(`${tbodyPath} > tr:nth-child(2)`).click(); From 8a0a5a8bd36764d83c268402ce92778d3d3298ed Mon Sep 17 00:00:00 2001 From: hehoon <100522372+hehoon@users.noreply.github.com> Date: Mon, 15 Dec 2025 14:40:19 +0100 Subject: [PATCH 04/20] Refactor tests --- .../test/public/features/filterTest.test.js | 7 -- .../test/public/pages/object-tree.test.js | 68 +++++++------------ 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/QualityControl/test/public/features/filterTest.test.js b/QualityControl/test/public/features/filterTest.test.js index 382641cb7..c157fc4b7 100644 --- a/QualityControl/test/public/features/filterTest.test.js +++ b/QualityControl/test/public/features/filterTest.test.js @@ -86,18 +86,11 @@ export const filterTests = async (url, page, timeout = 5000, testParent) => { }); await testParent.test('ObjectTreePage should apply filters for the objects', { timeout }, async () => { - // Ideally, tests should be isolated and not depend on each other. - // Currently, some tests rely on shared localStorage or page state changes from previous tests. - // As a workaround, we do targeted cleanup here to prevent issues in later tests. - const personid = await page.evaluate(() => window.model.session.personid); - await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); - await page.goto( `${url}?page=objectTree`, { waitUntil: 'networkidle0' }, ); - await extendTree(3, 5); let rowCount = await page.evaluate(() => document.querySelectorAll('tr').length); strictEqual(rowCount, 7); diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 3bd40b50f..a4cfadff3 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -13,7 +13,7 @@ import { strictEqual, ok, deepStrictEqual, notDeepStrictEqual } from 'node:assert'; import { delay } from '../../testUtils/delay.js'; -import { getLocalStorage, getLocalStorageAsJson, removeLocalStorage } from '../../testUtils/localStorage.js'; +import { getLocalStorage, getLocalStorageAsJson } from '../../testUtils/localStorage.js'; import { StorageKeysEnum } from '../../../public/common/enums/storageKeys.enum.js'; const OBJECT_TREE_PAGE_PARAM = '?page=objectTree'; @@ -43,46 +43,22 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ok(rowsCount > 1); // more than 1 object in the tree }); - await testParent.test('should update local storage when tree node is clicked', { timeout }, async () => { + await testParent.test('should preserve state if refreshed', { timeout }, async () => { const selector = 'section > div > div > div > table > tbody > tr:nth-child(2)'; - const personid = await page.evaluate(() => window.model.session.personid); - const storageKey = `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`; - - await page.locator(selector).click(); - const localStorageBefore = await getLocalStorageAsJson(page, storageKey); - await page.locator(selector).click(); - const localStorageAfter = await getLocalStorageAsJson(page, storageKey); - - // Ideally, tests should be isolated and not depend on each other. - // Currently, some tests rely on shared localStorage or page state changes from previous tests. - // As a workaround, we do targeted cleanup here to prevent issues in later tests. - await removeLocalStorage(page, storageKey); await page.reload({ waitUntil: 'networkidle0' }); - notDeepStrictEqual( - localStorageBefore, - localStorageAfter, - 'local storage should have changed after clicking a tree node', - ); - }); + const rowCountExpanded = await page.evaluate(() => + document.querySelectorAll('section > div > div > div > table > tbody > tr').length); - await testParent.test('should preserve state if refreshed', { timeout }, async () => { - const tbodyPath = 'section > div > div > div > table > tbody'; - await page.locator(`${tbodyPath} > tr:nth-child(2)`).click(); + await page.locator(selector).click(); await page.reload({ waitUntil: 'networkidle0' }); - const rowCount = await page.evaluate(() => + const rowCountCollapsed = await page.evaluate(() => document.querySelectorAll('section > div > div > div > table > tbody > tr').length); - // Ideally, tests should be isolated and not depend on each other. - // Currently, some tests rely on shared localStorage or page state changes from previous tests. - // As a workaround, we do targeted cleanup here to prevent issues in later tests. - const personid = await page.evaluate(() => window.model.session.personid); - await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); - await page.reload({ waitUntil: 'networkidle0' }); - - strictEqual(rowCount, 3); + strictEqual(rowCountExpanded, 3); + strictEqual(rowCountCollapsed, 2); }); await testParent.test('should have a button to sort by (default "Name" ASC)', async () => { @@ -165,18 +141,8 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) 'should maintain panel width from localStorage on page reload', { timeout }, async () => { - // Ideally, tests should be isolated and not depend on each other. - // Currently, some tests rely on shared localStorage or page state changes from previous tests. - // As a workaround, we do targeted cleanup here to prevent issues in later tests. - const personid = await page.evaluate(() => window.model.session.personid); - await removeLocalStorage(page, `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`); - const dragAmount = 35; await page.reload({ waitUntil: 'networkidle0' }); - await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(2)').click()); - await delay(500); - await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(3)').click()); - await delay(500); await page.evaluate(() => document.querySelector('tr.object-selectable:nth-child(4)').click()); await delay(1000); const panelWidth = await page.evaluate(() => @@ -265,6 +231,24 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) } ); + await testParent.test('should update local storage when tree node is clicked', { timeout }, async () => { + const selector = 'section > div > div > div > table > tbody > tr:nth-child(2)'; + const personid = await page.evaluate(() => window.model.session.personid); + const storageKey = `${StorageKeysEnum.OBJECT_TREE_OPEN_NODES}-${personid}`; + + await page.locator(selector).click(); + const localStorageBefore = await getLocalStorageAsJson(page, storageKey); + + await page.locator(selector).click(); + const localStorageAfter = await getLocalStorageAsJson(page, storageKey); + + notDeepStrictEqual( + localStorageBefore, + localStorageAfter, + 'local storage should have changed after clicking a tree node', + ); + }); + await testParent.test('should sort list of histograms by name in descending order', async () => { await page.locator('#sortTreeButton').click(); const sortingByNameOptionPath = '#sortTreeButton > div > a:nth-child(2)'; From c10f0ed14e334515bcb8a59fe12dd5eb2cf22f93 Mon Sep 17 00:00:00 2001 From: hehoon <100522372+hehoon@users.noreply.github.com> Date: Mon, 15 Dec 2025 14:43:33 +0100 Subject: [PATCH 05/20] Remove unused imports --- QualityControl/test/public/features/filterTest.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/QualityControl/test/public/features/filterTest.test.js b/QualityControl/test/public/features/filterTest.test.js index c157fc4b7..142fc33b9 100644 --- a/QualityControl/test/public/features/filterTest.test.js +++ b/QualityControl/test/public/features/filterTest.test.js @@ -13,8 +13,6 @@ import { strictEqual } from 'node:assert'; import { delay } from '../../testUtils/delay.js'; -import { removeLocalStorage } from '../../testUtils/localStorage.js'; -import { StorageKeysEnum } from '../../../public/common/enums/storageKeys.enum.js'; export const filterTests = async (url, page, timeout = 5000, testParent) => { await testParent.test('filter should persist between pages', { timeout }, async () => { From d370eab3b83d270c14b4b61c395ac662dce781d7 Mon Sep 17 00:00:00 2001 From: crashdance Date: Mon, 5 Jan 2026 08:16:59 +0100 Subject: [PATCH 06/20] Add keyboard arrow navigation for object tree --- QualityControl/public/Model.js | 12 ++ QualityControl/public/app.css | 2 + .../public/common/enums/storageKeys.enum.js | 1 + .../public/object/ObjectTree.class.js | 176 ++++++++++++++++++ QualityControl/public/object/QCObject.js | 1 + .../public/object/objectTreePage.js | 119 ++++++------ 6 files changed, 256 insertions(+), 55 deletions(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index 4c8a238f8..8e24daae8 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -139,6 +139,18 @@ export default class Model extends Observable { } else if (code === 27 && this.isImportVisible) { this.layout.resetImport(); } + + if (this.router.params.page === 'objectTree') { + if (code === 37) { // Left arrow + this.object.tree.collapseFocusedNode(); + } else if (code === 39) { // Right arrow + this.object.tree.expandFocusedNode(); + } else if (code === 38) { // Up arrow + this.object.tree.selectPreviousNode(); + } else if (code === 40) { // Down arrow + this.object.tree.selectNextNode(); + } + } } /** diff --git a/QualityControl/public/app.css b/QualityControl/public/app.css index 4b89e34b3..98eaf347a 100644 --- a/QualityControl/public/app.css +++ b/QualityControl/public/app.css @@ -26,6 +26,8 @@ .object-selectable { cursor: pointer; text-decoration: none; } .object-selectable:hover { cursor: pointer; background-color: var(--color-gray-dark) !important; color: var(--color-gray-lighter); } +.focused-node>th, .focused-node>td { background-color: var(--color-gray-dark); color: var(--color-white); } + .layout-selectable { border: 0.0em solid var(--color-primary); transition: border 0.1s; } .layout-selected { border: 0.3em solid var(--color-primary); } .layout-edit-layer { cursor: move; opacity: 0; } diff --git a/QualityControl/public/common/enums/storageKeys.enum.js b/QualityControl/public/common/enums/storageKeys.enum.js index 2c6f0c10f..f283733d7 100644 --- a/QualityControl/public/common/enums/storageKeys.enum.js +++ b/QualityControl/public/common/enums/storageKeys.enum.js @@ -21,4 +21,5 @@ export const StorageKeysEnum = Object.freeze({ OBJECT_VIEW_LEFT_PANEL_WIDTH: 'object-view-left-panel-width', OBJECT_VIEW_INFO_VISIBILITY_SETTING: 'object-view-info-visibility-setting', OBJECT_TREE_OPEN_NODES: 'object-tree-open-nodes', + OBJECT_TREE_FOCUSED_NODE: 'object-tree-focused-node', }); diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 0cbee3998..c80b6905a 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -29,6 +29,9 @@ export default class ObjectTree extends Observable { constructor(name, parent) { super(); this.storage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_OPEN_NODES); + this.focusedNodePathStorage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_FOCUSED_NODE); + this.selector = null; // Callback for object selection + this.focusedNode = null; // Currently focused node for navigation this.initTree(name, parent); } @@ -48,6 +51,165 @@ export default class ObjectTree extends Observable { this.pathString = ''; // 'A/B' } + /** + * Collapse the currently focused node or move focus to its parent. + * - If the focused node is an object, it collapses its parent (if any) and moves focus to the parent. + * - If the focused node is a branch (a node with children): + * - If the branch is open, it collapses the branch and keeps the focus on it. + * - If the branch is already collapsed, it moves focus to its parent. + * @returns {undefined} + */ + collapseFocusedNode() { + if (!this.focusedNode) { + return; + } + + const node = this.focusedNode; + + // If focus is on a object, collapse its parent (if any) and focus the parent. + if (node.object) { + const { parent } = node; + if (!parent) { + return; + } + parent.open = false; + this.setFocusedNode(parent); + return; + } + + // If focus is on a branch: collapse it if open, otherwise move focus to parent. + if (node.open && node.children.length > 0) { + node.toggle(); // collapse current branch, keep focus here + return; + } + + if (node.parent) { + this.setFocusedNode(node.parent); + } + } + + /** + * Expand the currently focused node or move focus to its first child. + * - If the focused node is a branch (a node with children): + * - If the branch is collapsed, it expands the branch and keeps the focus on it. + * - If the branch is already expanded, it moves focus to its first child. + * @returns {undefined} + */ + expandFocusedNode() { + if (!this.focusedNode) { + return; + } + if (!this.focusedNode.open && this.focusedNode.children.length > 0) { + this.focusedNode.toggle(); + } else if (this.focusedNode.open && this.focusedNode.children.length > 0) { + this.setFocusedNode(this.focusedNode.children[0]); + } + } + + + /** + * Set the currently focused node + * @param {ObjectTree} node - node to be focused + * @returns {undefined} + */ + setFocusedNode(node) { + this.focusedNode = node; + try { + const session = sessionService.get(); + const key = session.personid.toString(); + if (node?.pathString) { + this.focusedNodePathStorage.setLocalItem(key, node.pathString); + } else { + // clear stored value + this.focusedNodePathStorage.removeLocalItem(key, null); + } + } catch { + // ignore sessionStorage errors + } + this.notify(); + } + + // Register a callback invoked when navigation selects a node with an object + setSelector(selector) { + this.selector = selector; + } + + /** + * Get all visible nodes in the tree (for navigation) + * @returns {Array.} - list of visible nodes + */ + _getVisibleNodes() { + const nodes = []; + const traverse = (n) => { + nodes.push(n); + if (n.open) { + n.children.forEach(traverse); + } + }; + this.children.forEach(traverse); + return nodes; + } + + /** + * Select the next visible node in the tree + */ + selectNextNode() { + const visible = this._getVisibleNodes(); + if (!visible.length) { + return; // no visible nodes + } + const idx = visible.indexOf(this.focusedNode); + // nothing focused yet -> focus first visible node + if (!this.focusedNode || idx === -1) { + const [first] = visible; + this.setFocusedNode(first); + if (first.object && this.selector) { + this.selector(first.object); + } + return; + } + // if already at the last visible node, do nothing + if (idx >= visible.length - 1) { + return; + } + // select next node + const next = visible[idx + 1] ?? visible[idx]; + this.setFocusedNode(next); + if (next.object && this.selector) { + this.selector(next.object); + } + } + + /** + * Select the previous visible node in the tree. + */ + selectPreviousNode() { + const visible = this._getVisibleNodes(); + if (!visible.length) { + return; // no visible nodes + } + const idx = visible.indexOf(this.focusedNode); + // if already at the first visible node, do nothing + if (idx === 0) { + return; + } + // nothing focused yet -> focus first visible node + if (!this.focusedNode || idx === -1) { + const [first] = visible; + this.setFocusedNode(first); + if (first.object && this.selector) { + this.selector(first.object); + } + return; + } + // select previous node + const prev = idx > 0 ? visible[idx - 1] : visible[0]; + this.setFocusedNode(prev); + if (prev.object && this.selector) { + this.selector(prev.object); + } + } + /** * Load the expanded/collapsed state for this node and its children from localStorage. * Updates the `open` property for the current node and recursively for all children. @@ -73,6 +235,20 @@ export default class ObjectTree extends Observable { } this._applyExpandedNodesRecursive(parentNode, this); + + // Restore previously focused node + try { + const stored = this.focusedNodePathStorage.getLocalItem(key); + if (stored) { + const visible = this._getVisibleNodes(); + const found = visible.find((n) => n.pathString === stored); + if (found) { + this.setFocusedNode(found); + } + } + } catch { + // ignore sessionStorage errors + } } /** diff --git a/QualityControl/public/object/QCObject.js b/QualityControl/public/object/QCObject.js index 840215994..d2be51f1f 100644 --- a/QualityControl/public/object/QCObject.js +++ b/QualityControl/public/object/QCObject.js @@ -52,6 +52,7 @@ export default class QCObject extends BaseViewModel { this.tree = new ObjectTree('database'); this.tree.bubbleTo(this); + this.tree.setSelector((nodeObject) => this.select(nodeObject)); this.queryingObjects = false; this.scrollTop = 0; diff --git a/QualityControl/public/object/objectTreePage.js b/QualityControl/public/object/objectTreePage.js index f53b09d38..488e16596 100644 --- a/QualityControl/public/object/objectTreePage.js +++ b/QualityControl/public/object/objectTreePage.js @@ -180,7 +180,7 @@ const treeRows = (model) => !model.object.tree ? model.object.tree.children.length === 0 ? h('.w-100.text-center', 'No objects found') - : model.object.tree.children.map((children) => treeRow(model, children, 0)); + : model.object.tree.children.map((children) => treeRow(model, children)); /** * Shows a line of object represented by parent node `tree`, also shows @@ -190,68 +190,77 @@ const treeRows = (model) => !model.object.tree ? * @param {Model} model - root model of the application * @param {ObjectTree} tree - data-structure containing an object per node * @param {number} level - used for indentation within recursive call of treeRow - * @returns {vnode} - virtual node element + * @returns {vnode[]} - virtual node element */ -function treeRow(model, tree, level) { - const padding = `${level}em`; - const levelDeeper = level + 1; - const children = tree.open ? tree.children.map((children) => treeRow(model, children, levelDeeper)) : []; - const path = tree.name; - const className = tree.object && tree.object === model.object.selected ? 'table-primary' : ''; +function treeRow(model, tree, level = 0) { + const { pathString, open, children, object, name } = tree; - if (model.object.searchInput) { - return []; - } else { - if (tree.object && tree.children.length === 0) { - return [leafRow(path, () => model.object.select(tree.object), className, padding, tree.name)]; - } else if (tree.object && tree.children.length > 0) { - return [ - leafRow(path, () => model.object.select(tree.object), className, padding, tree.name), - branchRow(path, tree, padding), - children, - ]; - } - return [ - branchRow(path, tree, padding), - children, - ]; + const childRow = open + ? children.flatMap((children) => treeRow(model, children, level + 1)) + : []; + + const rows = []; + + // Determine the class name for the row + const className = model.object.selected && object === model.object.selected + ? 'table-primary' // Selected object + : model.object.tree.focusedNode === tree + ? 'focused-node' // Focused node + : ''; + + if (object) { + // Add a leaf row (final element; cannot be expanded further) + const leaf = treeRowElement( + pathString, + name, + () => model.object.select(object), + iconBarChart, + className, + { + paddingLeft: `${level + 0.3}em`, + }, + ); + rows.push(leaf); + } + if (children.length > 0) { + // Add a branch row (expandable / collapsible element) + const branch = treeRowElement( + pathString, + name, + () => tree.toggle(), + open ? iconCaretBottom : iconCaretRight, + className, + { + paddingLeft: `${level + 0.3}em`, + }, + ); + rows.push(branch); } + + return [...rows, ...childRow]; } /** - * Creates a row containing specific visuals for leaf object and on selection - * it will plot the object with JSRoot - * @param {string} path - full name of the object - * @param {Action} selectItem - action for plotting the object - * @param {string} className - name of the row class - * @param {number} padding - space needed to be displayed so that leaf is within its parent - * @param {string} leafName - name of the object + * Creates a row containing specific visuals for either a branch or a leaf object + * and on click it will expand/collapse the branch or plot the leaf object with JSRoot + * @param {string} key - An unique identifier for this branch row element (table row) + * @param {string} title - The name of this tree object element + * @param {() => void} onclick - The action (callback) to perform upon clicking this branch row element (table row) + * @param {() => vnode} icon - Icon renderer for the row + * @param {string} className - Optional CSS class name(s) for the outer branch row element (table row) + * @param {object} style - Optional CSS styling for the inner branch row element (table data) * @returns {vnode} - virtual node element */ -const leafRow = (path, selectItem, className, padding, leafName) => +const treeRowElement = (key, title, onclick, icon, className = '', style = {}) => h('tr.object-selectable', { - key: path, title: path, onclick: selectItem, class: className, id: path, + key, + id: key, + title, + onclick, + class: className, }, [ - h('td.highlight', [ - h('span', { style: { paddingLeft: padding } }, iconBarChart()), - ' ', - leafName, + h('td.highlight.flex-row.items-center.g1', { style }, [ + icon(), + title, ]), - ]); - -/** - * Creates a row containing specific visuals for branch object and on selection - * it will open its children - * @param {string} path - full name of the object - * @param {ObjectTree} tree - current selected tree - * @param {number} padding - space needed to be displayed so that branch is within its parent - * @returns {vnode} - virtual node element - */ -const branchRow = (path, tree, padding) => - h('tr.object-selectable', { key: path, title: path, onclick: () => tree.toggle() }, [ - h('td.highlight', [ - h('span', { style: { paddingLeft: padding } }, tree.open ? iconCaretBottom() : iconCaretRight()), - ' ', - tree.name, - ]), - ]); + ]); \ No newline at end of file From 21e496428dc9d67a7738feca9af48bdc5a47e6fa Mon Sep 17 00:00:00 2001 From: crashdance Date: Mon, 5 Jan 2026 12:37:38 +0100 Subject: [PATCH 07/20] fix: keyboard navigation in object tree --- QualityControl/public/Model.js | 1 + .../public/object/ObjectTree.class.js | 57 ++----------------- QualityControl/public/object/QCObject.js | 1 - .../public/object/objectTreePage.js | 33 ++++++----- 4 files changed, 25 insertions(+), 67 deletions(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index 8e24daae8..937d92f42 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -145,6 +145,7 @@ export default class Model extends Observable { this.object.tree.collapseFocusedNode(); } else if (code === 39) { // Right arrow this.object.tree.expandFocusedNode(); + this.object.select(this.object.tree.focusedNode?.object); } else if (code === 38) { // Up arrow this.object.tree.selectPreviousNode(); } else if (code === 40) { // Down arrow diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index c80b6905a..1e3c0745f 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -29,8 +29,6 @@ export default class ObjectTree extends Observable { constructor(name, parent) { super(); this.storage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_OPEN_NODES); - this.focusedNodePathStorage = new BrowserStorage(StorageKeysEnum.OBJECT_TREE_FOCUSED_NODE); - this.selector = null; // Callback for object selection this.focusedNode = null; // Currently focused node for navigation this.initTree(name, parent); } @@ -63,12 +61,9 @@ export default class ObjectTree extends Observable { if (!this.focusedNode) { return; } - - const node = this.focusedNode; - // If focus is on a object, collapse its parent (if any) and focus the parent. - if (node.object) { - const { parent } = node; + if (this.focusedNode.object) { + const { parent } = this.focusedNode; if (!parent) { return; } @@ -76,15 +71,13 @@ export default class ObjectTree extends Observable { this.setFocusedNode(parent); return; } - // If focus is on a branch: collapse it if open, otherwise move focus to parent. - if (node.open && node.children.length > 0) { - node.toggle(); // collapse current branch, keep focus here + if (this.focusedNode.open && this.focusedNode.children.length > 0) { + this.focusedNode.toggle(); // collapse current branch, keep focus here return; } - - if (node.parent) { - this.setFocusedNode(node.parent); + if (this.focusedNode.parent) { + this.setFocusedNode(this.focusedNode.parent); } } @@ -106,7 +99,6 @@ export default class ObjectTree extends Observable { } } - /** * Set the currently focused node * @param {ObjectTree} node - node to be focused @@ -114,26 +106,9 @@ export default class ObjectTree extends Observable { */ setFocusedNode(node) { this.focusedNode = node; - try { - const session = sessionService.get(); - const key = session.personid.toString(); - if (node?.pathString) { - this.focusedNodePathStorage.setLocalItem(key, node.pathString); - } else { - // clear stored value - this.focusedNodePathStorage.removeLocalItem(key, null); - } - } catch { - // ignore sessionStorage errors - } this.notify(); } - // Register a callback invoked when navigation selects a node with an object - setSelector(selector) { - this.selector = selector; - } - /** * Get all visible nodes in the tree (for navigation) * @returns {Array.} - list of visible nodes @@ -175,9 +150,6 @@ export default class ObjectTree extends Observable { // select next node const next = visible[idx + 1] ?? visible[idx]; this.setFocusedNode(next); - if (next.object && this.selector) { - this.selector(next.object); - } } /** @@ -205,9 +177,6 @@ export default class ObjectTree extends Observable { // select previous node const prev = idx > 0 ? visible[idx - 1] : visible[0]; this.setFocusedNode(prev); - if (prev.object && this.selector) { - this.selector(prev.object); - } } /** @@ -235,20 +204,6 @@ export default class ObjectTree extends Observable { } this._applyExpandedNodesRecursive(parentNode, this); - - // Restore previously focused node - try { - const stored = this.focusedNodePathStorage.getLocalItem(key); - if (stored) { - const visible = this._getVisibleNodes(); - const found = visible.find((n) => n.pathString === stored); - if (found) { - this.setFocusedNode(found); - } - } - } catch { - // ignore sessionStorage errors - } } /** diff --git a/QualityControl/public/object/QCObject.js b/QualityControl/public/object/QCObject.js index d2be51f1f..840215994 100644 --- a/QualityControl/public/object/QCObject.js +++ b/QualityControl/public/object/QCObject.js @@ -52,7 +52,6 @@ export default class QCObject extends BaseViewModel { this.tree = new ObjectTree('database'); this.tree.bubbleTo(this); - this.tree.setSelector((nodeObject) => this.select(nodeObject)); this.queryingObjects = false; this.scrollTop = 0; diff --git a/QualityControl/public/object/objectTreePage.js b/QualityControl/public/object/objectTreePage.js index 488e16596..46227a3a9 100644 --- a/QualityControl/public/object/objectTreePage.js +++ b/QualityControl/public/object/objectTreePage.js @@ -73,7 +73,7 @@ export default (model) => { */ function objectPanel(model) { const selectedObjectName = model.object.selected.name; - if (model.object.objects && model.object.objects[selectedObjectName]) { + if (model.object.objects?.[selectedObjectName]) { return model.object.objects[selectedObjectName].match({ NotAsked: () => null, Loading: () => @@ -174,13 +174,16 @@ const tableShow = (model) => * @param {Model} model - root model of the application * @returns {vnode} - virtual node element */ -const treeRows = (model) => !model.object.tree ? - null - : - - model.object.tree.children.length === 0 - ? h('.w-100.text-center', 'No objects found') - : model.object.tree.children.map((children) => treeRow(model, children)); +const treeRows = (model) => { + if (!model.object.tree) { + return null; + } + if (model.object.tree.children.length === 0) { + return h('.w-100.text-center', 'No objects found'); + } else { + return model.object.tree.children.map((children) => treeRow(model, children)); + } +}; /** * Shows a line of object represented by parent node `tree`, also shows @@ -201,12 +204,12 @@ function treeRow(model, tree, level = 0) { const rows = []; - // Determine the class name for the row - const className = model.object.selected && object === model.object.selected - ? 'table-primary' // Selected object - : model.object.tree.focusedNode === tree - ? 'focused-node' // Focused node - : ''; + let className = ''; + if (model.object.selected && object === model.object.selected) { + className = 'table-primary'; // Selected object + } else if (model.object.tree.focusedNode === tree) { + className = 'focused-node'; // Focused node + } if (object) { // Add a leaf row (final element; cannot be expanded further) @@ -263,4 +266,4 @@ const treeRowElement = (key, title, onclick, icon, className = '', style = {}) = icon(), title, ]), - ]); \ No newline at end of file + ]); From ba3675dd5da30026bc0724cb035c79ae347509e7 Mon Sep 17 00:00:00 2001 From: crashdance Date: Mon, 5 Jan 2026 12:39:42 +0100 Subject: [PATCH 08/20] chore: remove unused storage key --- QualityControl/public/common/enums/storageKeys.enum.js | 1 - 1 file changed, 1 deletion(-) diff --git a/QualityControl/public/common/enums/storageKeys.enum.js b/QualityControl/public/common/enums/storageKeys.enum.js index f283733d7..2c6f0c10f 100644 --- a/QualityControl/public/common/enums/storageKeys.enum.js +++ b/QualityControl/public/common/enums/storageKeys.enum.js @@ -21,5 +21,4 @@ export const StorageKeysEnum = Object.freeze({ OBJECT_VIEW_LEFT_PANEL_WIDTH: 'object-view-left-panel-width', OBJECT_VIEW_INFO_VISIBILITY_SETTING: 'object-view-info-visibility-setting', OBJECT_TREE_OPEN_NODES: 'object-tree-open-nodes', - OBJECT_TREE_FOCUSED_NODE: 'object-tree-focused-node', }); From 2081fe25cd98110f0cd2b50eee8be5b38eb44182 Mon Sep 17 00:00:00 2001 From: crashdance Date: Tue, 6 Jan 2026 11:44:19 +0100 Subject: [PATCH 09/20] tests: add arrow key navigation tests for object tree --- .../test/public/pages/object-tree.test.js | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index a4cfadff3..c5f9dea3b 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -308,4 +308,116 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) deepStrictEqual(options, ['', 'runType1', 'runType2']); }, ); + + await testParent.test( + 'should focus first node when ArrowDown key is pressed', + { timeout }, + async () => { + await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); + await page.keyboard.press('ArrowDown'); // Move to the first child node + await delay(500); + const isFirstNodeHighlighted = await page.evaluate(() => { + const selectedNode = document.querySelector('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isFirstNodeHighlighted, true, 'The first node is not highlighted.'); + }, + ); + + await testParent.test( + 'should collapse tree when ArrowLeft key is pressed on a focused node', + { timeout }, + async () => { + await page.keyboard.press('ArrowDown'); // Move to the second child node + await delay(500); + // Verify the second node is highlighted + const isSecondNodeHighlighted = await page.evaluate(() => { + const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted.'); + await delay(200); + await page.keyboard.press('ArrowLeft'); // Collapse the second node + await delay(500); + // Verify that the third node is gone after collapsing + const isNodeCollapsed = await page.evaluate(() => { + const nodes = document.querySelectorAll('tr.object-selectable'); + return nodes.length < 3; // Check if there are less than 3 nodes + }); + strictEqual(isNodeCollapsed, true, 'The third node is still present after collapsing the second node.'); + }, + ); + + await testParent.test( + 'should expand tree when ArrowRight key is pressed on a focused node', + { timeout }, + async () => { + await page.keyboard.press('ArrowRight'); // Expand the second node + await delay(500); + // Verify that the third node is back after expanding + const isNodeExpanded = await page.evaluate(() => { + const nodes = document.querySelectorAll('tr.object-selectable'); + return nodes.length >= 3; // Check if there are at least 3 nodes + }); + strictEqual(isNodeExpanded, true, 'The third node is not present after expanding the second node.'); + }, + ); + + await testParent.test( + 'should select object when ArrowRight key is pressed on a focused object node', + { timeout }, + async () => { + await page.keyboard.press('ArrowDown'); // Move to the third child node + await delay(500); + await page.keyboard.press('ArrowDown'); // Move to the fourth child node which is an object + await delay(200); + await page.keyboard.press('ArrowRight'); // Select object node + await delay(500); + const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); + const isObjectPlotOpened = await page.evaluate(() => { + const objectPanel = document.querySelector('#qcObjectInfoPanel'); + return objectPanel !== null && objectPanel !== undefined; + }); + strictEqual(isObjectSelected, true, 'The focused object node was not selected on pressing ArrowRight key.'); + strictEqual(isObjectPlotOpened, true, 'The object plot panel is not opened after selecting the object node.'); + }, + ); + + await testParent.test( + 'should collapse parent node when ArrowLeft key is pressed on a selected object node', + { timeout }, + async () => { + const nodeCountBefore = await page.evaluate(() => { + const nodes = document.querySelectorAll('tr.object-selectable'); + return nodes.length; + }); + ok(nodeCountBefore > 3, `Expected node count to be greater than 3, but got ${nodeCountBefore}`); + await page.keyboard.press('ArrowLeft'); // Collapse the parent node of the selected object + await delay(500); + const nodeCountAfter = await page.evaluate(() => { + const nodes = document.querySelectorAll('tr.object-selectable'); + return nodes.length; + }); + strictEqual( + nodeCountAfter, + 3, + 'The object tree navigation does not have exactly 3 nodes after collapsing the parent.', + ); + }, + ); + + await testParent.test( + 'should focus previous node when ArrowUp key is pressed', + { timeout }, + async () => { + await page.keyboard.press('ArrowUp'); // Move back to the second child node + await delay(500); + // Verify the second node is highlighted + const isSecondNodeHighlighted = await page.evaluate(() => { + const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted after pressing ArrowUp key.'); + }, + ); }; From d087737b2a85c5d8d6e30a2a61ec58d64ed59f7c Mon Sep 17 00:00:00 2001 From: crashdance Date: Wed, 7 Jan 2026 15:10:40 +0100 Subject: [PATCH 10/20] fix: merge conflicts objects tree class --- .../public/object/ObjectTree.class.js | 104 ------------------ 1 file changed, 104 deletions(-) diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 9cd74e8a4..004517314 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -223,110 +223,6 @@ export default class ObjectTree extends Observable { } }; - /** - * Persist the current node's expanded/collapsed state in localStorage. - */ - storeExpandedNodes() { - const session = sessionService.get(); - const key = session.personid.toString(); - const data = this.storage.getLocalItem(key) ?? {}; - - // We traverse the path to reach the parent object of this node - let parentNode = data; - for (let i = 0; i < this.path.length - 1; i++) { - const pathKey = this.path[i]; - if (!parentNode[pathKey]) { - if (!this.open) { - // Cannot remove marked node because parent path does not exist - // Due to this the marked node also does not exist (so there is nothing to remove) - return; - } - - // Parent path does not exist, we create it here so we can mark a deeper node - parentNode[pathKey] = {}; - } - - parentNode = parentNode[pathKey]; - } - - if (this.open) { - this._markExpandedNodesRecursive(parentNode, this); - this.storage.setLocalItem(key, data); - } else if (parentNode[this.name]) { - // Deleting from `parentNode` directly updates the `data` object - delete parentNode[this.name]; - this.storage.setLocalItem(key, data); - } - } - - /** - * Recursively mark a node and all open children in the hierarchical "expanded nodes" object. - * This method updates `data` to reflect the current node's expanded state: - * - If the node has any open children, it creates an object branch and recursively marks those children. - * - If the node has no open children (or is a leaf), it stores a marker value `{}`. - * @param {object} data - The current level in the hierarchical data object where nodes are stored. - * @param {ObjectTree} treeNode - The tree node whose expanded state should be stored. - */ - _markExpandedNodesRecursive(data, treeNode) { - if (!data[treeNode.name]) { - data[treeNode.name] = {}; - } - treeNode.children - .filter((child) => child.open) - .forEach((child) => this._markExpandedNodesRecursive(data[treeNode.name], child)); - }; - - /** - * Toggle this node (open/close) - * @returns {undefined} - */ - toggle() { - this.open = !this.open; - this.storeExpandedNodes(); - this.notify(); - * Load the expanded/collapsed state for this node and its children from localStorage. - * Updates the `open` property for the current node and recursively for all children. - */ - loadExpandedNodes() { - if (!this.parent) { - // The main node may not be collapsable or expandable. - // Because of this we also have to load the expanded state of their direct children. - this.children.forEach((child) => child.loadExpandedNodes()); - } - - const session = sessionService.get(); - const key = session.personid.toString(); - - // We traverse the path to reach the parent object of this node - let parentNode = this.storage.getLocalItem(key) ?? {}; - for (let i = 0; i < this.path.length - 1; i++) { - parentNode = parentNode[this.path[i]]; - if (!parentNode) { - // Cannot expand marked node because parent path does not exist - return; - } - } - - this._applyExpandedNodesRecursive(parentNode, this); - } - - /** - * Recursively traverse the stored data and update the tree nodes - * @param {object} data - The current level of the hierarchical expanded nodes object - * @param {ObjectTree} treeNode - The tree node to update - */ - _applyExpandedNodesRecursive(data, treeNode) { - if (data[treeNode.name]) { - treeNode.open = true; - Object.keys(data[treeNode.name]).forEach((childName) => { - const child = treeNode.children.find((child) => child.name === childName); - if (child) { - this._applyExpandedNodesRecursive(data[treeNode.name], child); - } - }); - } - }; - /** * Persist the current node's expanded/collapsed state in localStorage. */ From b3b8a90ba4ab7b7443526c7e621a62f3d1cf359c Mon Sep 17 00:00:00 2001 From: crashdance Date: Thu, 8 Jan 2026 12:33:04 +0100 Subject: [PATCH 11/20] refactor object tree navigation and cleanup unused code --- .../public/object/ObjectTree.class.js | 24 ++++++--------- .../public/object/objectTreePage.js | 30 ------------------- 2 files changed, 9 insertions(+), 45 deletions(-) diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 004517314..70645424e 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -138,9 +138,6 @@ export default class ObjectTree extends Observable { if (!this.focusedNode || idx === -1) { const [first] = visible; this.setFocusedNode(first); - if (first.object && this.selector) { - this.selector(first.object); - } return; } // if already at the last visible node, do nothing @@ -169,9 +166,6 @@ export default class ObjectTree extends Observable { if (!this.focusedNode || idx === -1) { const [first] = visible; this.setFocusedNode(first); - if (first.object && this.selector) { - this.selector(first.object); - } return; } // select previous node @@ -362,15 +356,15 @@ export default class ObjectTree extends Observable { subtree._addChild(object, path, fullPath); } - /** - * Add a single object as a child node - * @param {object} object - child to be added - */ - addOneChild(object) { - this._addChild(object); - this.loadExpandedNodes(); - this.notify(); - } + // /** + // * Add a single object as a child node + // * @param {object} object - child to be added + // */ + // addOneChild(object) { + // this._addChild(object); + // this.loadExpandedNodes(); + // this.notify(); + // } /** * Add a list of objects as child nodes diff --git a/QualityControl/public/object/objectTreePage.js b/QualityControl/public/object/objectTreePage.js index 00aa809bf..e36e3b18a 100644 --- a/QualityControl/public/object/objectTreePage.js +++ b/QualityControl/public/object/objectTreePage.js @@ -243,36 +243,6 @@ function treeRow(model, tree, level = 0) { rows.push(branch); } - if (object) { - // Add a leaf row (final element; cannot be expanded further) - const className = object === model.object.selected ? 'table-primary' : ''; - const leaf = treeRowElement( - pathString, - name, - () => model.object.select(object), - iconBarChart, - className, - { - paddingLeft: `${level + 0.3}em`, - }, - ); - rows.push(leaf); - } - if (children.length > 0) { - // Add a branch row (expandable / collapsible element) - const branch = treeRowElement( - pathString, - name, - () => tree.toggle(), - open ? iconCaretBottom : iconCaretRight, - '', - { - paddingLeft: `${level + 0.3}em`, - }, - ); - rows.push(branch); - } - return [...rows, ...childRow]; } From d6f788b8a50f6737b01ff785e1c9355f5951cb35 Mon Sep 17 00:00:00 2001 From: crashdance Date: Thu, 8 Jan 2026 12:33:27 +0100 Subject: [PATCH 12/20] fix: ensure object selection only occurs if focused node is present --- QualityControl/public/Model.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index b9f47b257..00ca27558 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -145,7 +145,10 @@ export default class Model extends Observable { this.object.tree.collapseFocusedNode(); } else if (code === 39) { // Right arrow this.object.tree.expandFocusedNode(); - this.object.select(this.object.tree.focusedNode?.object); + const focusedObject = this.object.tree.focusedNode?.object; + if (focusedObject) { + this.object.select(focusedObject); + } } else if (code === 38) { // Up arrow this.object.tree.selectPreviousNode(); } else if (code === 40) { // Down arrow From 1fc2232c5238e8a8619e20e23bdb046b3ba0608b Mon Sep 17 00:00:00 2001 From: crashdance Date: Thu, 8 Jan 2026 12:36:02 +0100 Subject: [PATCH 13/20] refactor: remove unused addOneChild method from ObjectTree class --- QualityControl/public/object/ObjectTree.class.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 70645424e..5e436fcc3 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -356,16 +356,6 @@ export default class ObjectTree extends Observable { subtree._addChild(object, path, fullPath); } - // /** - // * Add a single object as a child node - // * @param {object} object - child to be added - // */ - // addOneChild(object) { - // this._addChild(object); - // this.loadExpandedNodes(); - // this.notify(); - // } - /** * Add a list of objects as child nodes * @param {Array} objects - children to be added From 4cd197936f6183e801acea61e7c519ae6b1ab2a2 Mon Sep 17 00:00:00 2001 From: crashdance Date: Thu, 8 Jan 2026 14:00:17 +0100 Subject: [PATCH 14/20] tests: add arrow left key navigation test and refactor object tree tests --- .../test/public/pages/object-tree.test.js | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 9d02b61b8..3e40001c1 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -343,7 +343,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) { timeout }, async () => { await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); - await page.keyboard.press('ArrowDown'); // Move to the first child node + await page.keyboard.press('ArrowDown'); // Move to the first node await delay(500); const isFirstNodeHighlighted = await page.evaluate(() => { const selectedNode = document.querySelector('tr.object-selectable'); @@ -354,26 +354,16 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ); await testParent.test( - 'should collapse tree when ArrowLeft key is pressed on a focused node', + 'should focus next node when ArrowDown key is pressed', { timeout }, async () => { - await page.keyboard.press('ArrowDown'); // Move to the second child node + await page.keyboard.press('ArrowDown'); // Move to the second node await delay(500); - // Verify the second node is highlighted const isSecondNodeHighlighted = await page.evaluate(() => { const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); return selectedNode?.classList.contains('focused-node'); }); strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted.'); - await delay(200); - await page.keyboard.press('ArrowLeft'); // Collapse the second node - await delay(500); - // Verify that the third node is gone after collapsing - const isNodeCollapsed = await page.evaluate(() => { - const nodes = document.querySelectorAll('tr.object-selectable'); - return nodes.length < 3; // Check if there are less than 3 nodes - }); - strictEqual(isNodeCollapsed, true, 'The third node is still present after collapsing the second node.'); }, ); @@ -383,7 +373,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) async () => { await page.keyboard.press('ArrowRight'); // Expand the second node await delay(500); - // Verify that the third node is back after expanding + // Verify that the third node is visible after expanding const isNodeExpanded = await page.evaluate(() => { const nodes = document.querySelectorAll('tr.object-selectable'); return nodes.length >= 3; // Check if there are at least 3 nodes @@ -394,13 +384,15 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) await testParent.test( 'should select object when ArrowRight key is pressed on a focused object node', - { timeout }, + { timeout: timeout * 20 }, async () => { - await page.keyboard.press('ArrowDown'); // Move to the third child node - await delay(500); - await page.keyboard.press('ArrowDown'); // Move to the fourth child node which is an object + await page.keyboard.press('ArrowDown'); // Move to the third node + await delay(200); + await page.keyboard.press('ArrowRight'); // expand the third node + await delay(200); + await page.keyboard.press('ArrowDown'); // Move to the object node await delay(200); - await page.keyboard.press('ArrowRight'); // Select object node + await page.keyboard.press('ArrowRight'); // select the object node await delay(500); const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); const isObjectPlotOpened = await page.evaluate(() => { @@ -449,4 +441,19 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted after pressing ArrowUp key.'); }, ); + + await testParent.test( + 'should collapse tree when ArrowLeft key is pressed on a focused node', + { timeout }, + async () => { + await page.keyboard.press('ArrowLeft'); // Collapse the second node + await delay(500); + // Verify that the third node is gone after collapsing + const isNodeCollapsed = await page.evaluate(() => { + const nodes = document.querySelectorAll('tr.object-selectable'); + return nodes.length < 3; // Check if there are less than 3 nodes + }); + strictEqual(isNodeCollapsed, true, 'The third node is still present after collapsing the second node.'); + }, + ); }; From 6c529746c233721b63218ca68714658a070f2339 Mon Sep 17 00:00:00 2001 From: crashdance Date: Thu, 8 Jan 2026 14:16:53 +0100 Subject: [PATCH 15/20] refactor: simplify comments for object tree key navigation tests --- QualityControl/public/object/ObjectTree.class.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 5e436fcc3..477ebf7c6 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -51,10 +51,6 @@ export default class ObjectTree extends Observable { /** * Collapse the currently focused node or move focus to its parent. - * - If the focused node is an object, it collapses its parent (if any) and moves focus to the parent. - * - If the focused node is a branch (a node with children): - * - If the branch is open, it collapses the branch and keeps the focus on it. - * - If the branch is already collapsed, it moves focus to its parent. * @returns {undefined} */ collapseFocusedNode() { @@ -82,10 +78,7 @@ export default class ObjectTree extends Observable { } /** - * Expand the currently focused node or move focus to its first child. - * - If the focused node is a branch (a node with children): - * - If the branch is collapsed, it expands the branch and keeps the focus on it. - * - If the branch is already expanded, it moves focus to its first child. + * If focus is on a branch: expand it if closed, otherwise move focus to first child. * @returns {undefined} */ expandFocusedNode() { From 897c29e3ddbd9c4d565e05f86e47909089a87899 Mon Sep 17 00:00:00 2001 From: crashdance Date: Fri, 9 Jan 2026 14:39:50 +0100 Subject: [PATCH 16/20] feat: keyboard navigation for virtual table when search input is active --- QualityControl/public/Model.js | 62 +++++++++++++++---- .../public/object/ObjectTree.class.js | 28 ++++----- QualityControl/public/object/QCObject.js | 18 ++++++ QualityControl/public/object/virtualTable.js | 17 ++++- 4 files changed, 97 insertions(+), 28 deletions(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index 00ca27558..b0a5cf743 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -141,18 +141,58 @@ export default class Model extends Observable { } if (this.router.params.page === 'objectTree') { - if (code === 37) { // Left arrow - this.object.tree.collapseFocusedNode(); - } else if (code === 39) { // Right arrow - this.object.tree.expandFocusedNode(); - const focusedObject = this.object.tree.focusedNode?.object; - if (focusedObject) { - this.object.select(focusedObject); + const isUp = code === 38; + const isDown = code === 40; + const isLeft = code === 37; + const isRightOrEnter = code === 39 || code === 13; + const searchActive = Boolean(this.object.searchInput?.trim()); + // Use virtual table navigation when search is active + if (searchActive) { + const results = this.object.searchResult || []; + if (!results.length) { + return; + } + const focusedResult = this.object.focusedSearchResult; + const focusedIndex = focusedResult ? results.indexOf(focusedResult) : -1; + if (isUp) { + const nextIndex = focusedIndex > 0 ? focusedIndex - 1 : 0; + this.object.setFocusedSearchResult(results[nextIndex] ?? results[0]); + return; + } + if (isDown) { + const nextIndex = focusedIndex >= 0 ? Math.min(results.length - 1, focusedIndex + 1) : 0; + this.object.setFocusedSearchResult(results[nextIndex] ?? results[0]); + return; + } + if (isRightOrEnter) { + if (this.object.focusedSearchResult) { + this.object.select(this.object.focusedSearchResult); + } + return; + } + } else { + // No search, use tree navigation + if (isUp) { + this.object.tree.focusPreviousNode(); + return; + } + if (isDown) { + this.object.tree.focusNextNode(); + return; + } + if (isLeft) { + this.object.tree.collapseFocusedNode(); + return; + } + if (isRightOrEnter) { + const focusedObject = this.object.tree.focusedNode?.object; + if (focusedObject) { + this.object.select(focusedObject); + } else { + this.object.tree.expandFocusedNode(); + } + return; } - } else if (code === 38) { // Up arrow - this.object.tree.selectPreviousNode(); - } else if (code === 40) { // Down arrow - this.object.tree.selectNextNode(); } } } diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index 477ebf7c6..b6260e2cb 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -49,6 +49,16 @@ export default class ObjectTree extends Observable { this.pathString = ''; // 'A/B' } + /** + * Set the currently focused node + * @param {ObjectTree} node - node to be focused + * @returns {undefined} + */ + setFocusedNode(node) { + this.focusedNode = node; + this.notify(); + } + /** * Collapse the currently focused node or move focus to its parent. * @returns {undefined} @@ -92,16 +102,6 @@ export default class ObjectTree extends Observable { } } - /** - * Set the currently focused node - * @param {ObjectTree} node - node to be focused - * @returns {undefined} - */ - setFocusedNode(node) { - this.focusedNode = node; - this.notify(); - } - /** * Get all visible nodes in the tree (for navigation) * @returns {Array.} - list of visible nodes @@ -119,9 +119,9 @@ export default class ObjectTree extends Observable { } /** - * Select the next visible node in the tree + * Focus the next visible node in the tree */ - selectNextNode() { + focusNextNode() { const visible = this._getVisibleNodes(); if (!visible.length) { return; // no visible nodes @@ -143,9 +143,9 @@ export default class ObjectTree extends Observable { } /** - * Select the previous visible node in the tree. + * Focus the previous visible node in the tree. */ - selectPreviousNode() { + focusPreviousNode() { const visible = this._getVisibleNodes(); if (!visible.length) { return; // no visible nodes diff --git a/QualityControl/public/object/QCObject.js b/QualityControl/public/object/QCObject.js index 840215994..6be04594e 100644 --- a/QualityControl/public/object/QCObject.js +++ b/QualityControl/public/object/QCObject.js @@ -42,6 +42,8 @@ export default class QCObject extends BaseViewModel { this.searchInput = ''; // String - content of input search this.searchResult = []; // Array - result list of search + this.focusedSearchResult = null; // Object - focused item in search results for keyboard navigation + this.sortBy = { field: 'name', title: 'Name', @@ -82,6 +84,19 @@ export default class QCObject extends BaseViewModel { this.notify(); } + /** + * Set focused item in search results (used by keyboard navigation). + * @param {object} next - next object to be focused + * @returns {undefined} + */ + setFocusedSearchResult(next) { + if (!next || next === this.focusedSearchResult) { + return; + } + this.focusedSearchResult = next; + this.notify(); + } + /** * Set searched items table UI sizes to allow virtual scrolling * @param {number} scrollTop - position of the user's scroll cursor @@ -391,6 +406,7 @@ export default class QCObject extends BaseViewModel { } else { await this.loadObjectByName(this.selected.name); } + this.notify(); } @@ -404,6 +420,8 @@ export default class QCObject extends BaseViewModel { this._computeFilters(); this.sortListByField(this.searchResult, this.sortBy.field, this.sortBy.order); + this.focusedSearchResult = null; + this.notify(); } diff --git a/QualityControl/public/object/virtualTable.js b/QualityControl/public/object/virtualTable.js index 29ebda4d4..0f202393a 100644 --- a/QualityControl/public/object/virtualTable.js +++ b/QualityControl/public/object/virtualTable.js @@ -63,8 +63,18 @@ export default function virtualTable(model, location = 'main', objects = []) { * @param {string} location - location of the object * @returns {vnode} - virtual node element */ -const objectFullRow = (model, item, location) => - h('tr.object-selectable', { +const objectFullRow = (model, item, location) => { + const isSelected = item && item === model.object.selected; + const isFocused = item && model.object.focusedSearchResult && item.name === model.object.focusedSearchResult.name; + + let className = ''; + if (isSelected) { + className = 'table-primary'; // Selected object + } else if (isFocused) { + className = 'focused-node'; // Focused node + } + + return h('tr.object-selectable', { key: item.name, title: item.name, onclick: () => model.object.select(item), @@ -84,7 +94,7 @@ const objectFullRow = (model, item, location) => model.layout.moveTabObjectStop(); } }, - class: item && item === model.object.selected ? 'table-primary' : '', + class: className, draggable: location === 'side', }, [ h('td.highlight.text-ellipsis', [ @@ -93,6 +103,7 @@ const objectFullRow = (model, item, location) => item.name, ]), ]); +}; /** * Create a table header separately so that it does not get included From 06aeabff3a6b715d20c357a77828efcf1ceef993 Mon Sep 17 00:00:00 2001 From: crashdance Date: Fri, 9 Jan 2026 15:24:00 +0100 Subject: [PATCH 17/20] tests: add tests key navigation object tree page when search is active --- .../test/public/pages/object-tree.test.js | 92 ++++++++++++++++--- 1 file changed, 81 insertions(+), 11 deletions(-) diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 3e40001c1..22b27c3a3 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -343,7 +343,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) { timeout }, async () => { await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); - await page.keyboard.press('ArrowDown'); // Move to the first node + await page.keyboard.press('ArrowDown'); // Focus the first node await delay(500); const isFirstNodeHighlighted = await page.evaluate(() => { const selectedNode = document.querySelector('tr.object-selectable'); @@ -357,7 +357,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) 'should focus next node when ArrowDown key is pressed', { timeout }, async () => { - await page.keyboard.press('ArrowDown'); // Move to the second node + await page.keyboard.press('ArrowDown'); // Focus to the next (second) node await delay(500); const isSecondNodeHighlighted = await page.evaluate(() => { const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); @@ -371,7 +371,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) 'should expand tree when ArrowRight key is pressed on a focused node', { timeout }, async () => { - await page.keyboard.press('ArrowRight'); // Expand the second node + await page.keyboard.press('ArrowRight'); // Expand the focused node await delay(500); // Verify that the third node is visible after expanding const isNodeExpanded = await page.evaluate(() => { @@ -384,15 +384,15 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) await testParent.test( 'should select object when ArrowRight key is pressed on a focused object node', - { timeout: timeout * 20 }, + { timeout }, async () => { - await page.keyboard.press('ArrowDown'); // Move to the third node + await page.keyboard.press('ArrowDown'); // Focus to the third node await delay(200); - await page.keyboard.press('ArrowRight'); // expand the third node + await page.keyboard.press('ArrowRight'); // Expand the third node await delay(200); - await page.keyboard.press('ArrowDown'); // Move to the object node + await page.keyboard.press('ArrowDown'); // Focus next node (object node) await delay(200); - await page.keyboard.press('ArrowRight'); // select the object node + await page.keyboard.press('ArrowRight'); // Select the object node await delay(500); const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); const isObjectPlotOpened = await page.evaluate(() => { @@ -414,7 +414,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) }); ok(nodeCountBefore > 3, `Expected node count to be greater than 3, but got ${nodeCountBefore}`); await page.keyboard.press('ArrowLeft'); // Collapse the parent node of the selected object - await delay(500); + await delay(200); const nodeCountAfter = await page.evaluate(() => { const nodes = document.querySelectorAll('tr.object-selectable'); return nodes.length; @@ -432,7 +432,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) { timeout }, async () => { await page.keyboard.press('ArrowUp'); // Move back to the second child node - await delay(500); + await delay(200); // Verify the second node is highlighted const isSecondNodeHighlighted = await page.evaluate(() => { const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); @@ -447,7 +447,7 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) { timeout }, async () => { await page.keyboard.press('ArrowLeft'); // Collapse the second node - await delay(500); + await delay(200); // Verify that the third node is gone after collapsing const isNodeCollapsed = await page.evaluate(() => { const nodes = document.querySelectorAll('tr.object-selectable'); @@ -456,4 +456,74 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) strictEqual(isNodeCollapsed, true, 'The third node is still present after collapsing the second node.'); }, ); + + await testParent.test( + 'should focus first object in search results when ArrowDown key is pressed and search is active', + { timeout }, + async () => { + // reset state by navigating to the object tree page again + await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); + await page.focus('#searchObjectTree'); + await page.type('#searchObjectTree', 'qc/test/object'); + await page.keyboard.press('ArrowDown'); // Focus first object in search results + await delay(200); + const isFirstObjectHighlighted = await page.evaluate(() => { + const selectedNode = document.querySelector('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isFirstObjectHighlighted, true, 'The first object in search results is not highlighted.'); + }, + ); + + await testParent.test( + 'should focus next object in search results when ArrowDown key is pressed and search is active', + { timeout }, + async () => { + await page.keyboard.press('ArrowDown'); // Focus next object in search results + await delay(500); + const isSecondObjectHighlighted = await page.evaluate(() => { + const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isSecondObjectHighlighted, true, 'The second object in search results is not highlighted.'); + }, + ); + + await testParent.test( + 'should focus previous object in search results when ArrowUp key is pressed and search is active', + { timeout }, + async () => { + await page.keyboard.press('ArrowUp'); // Focus previous object in search results + await delay(200); + const isFirstObjectHighlighted = await page.evaluate(() => { + const selectedNode = document.querySelector('tr.object-selectable'); + return selectedNode?.classList.contains('focused-node'); + }); + strictEqual(isFirstObjectHighlighted, true, 'The first object in search results is not highlighted.'); + }, + ); + + await testParent.test( + 'should select focused object in search results when ArrowRight key is pressed and search is active', + { timeout }, + async () => { + await page.keyboard.press('ArrowRight'); // Select focused object + await delay(500); + const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); + const isObjectPlotOpened = await page.evaluate(() => { + const objectPanel = document.querySelector('#qcObjectInfoPanel'); + return objectPanel !== null && objectPanel !== undefined; + }); + strictEqual( + isObjectSelected, + true, + 'The focused object in search results was not selected on pressing ArrowRight key.', + ); + strictEqual( + isObjectPlotOpened, + true, + 'The object plot panel is not opened after selecting the focused object in search results.', + ); + }, + ); }; From 82302a9cbfe4ab3580757170b4493182ce83eed2 Mon Sep 17 00:00:00 2001 From: crashdance Date: Sat, 10 Jan 2026 16:38:14 +0100 Subject: [PATCH 18/20] feat: enhance keyboard navigation in search results and move focus to selected item on click --- QualityControl/public/Model.js | 7 +- .../public/object/ObjectTree.class.js | 65 +++++++++++++------ QualityControl/public/object/QCObject.js | 37 +++++++++-- 3 files changed, 82 insertions(+), 27 deletions(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index b0a5cf743..67a2120eb 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -152,16 +152,15 @@ export default class Model extends Observable { if (!results.length) { return; } - const focusedResult = this.object.focusedSearchResult; - const focusedIndex = focusedResult ? results.indexOf(focusedResult) : -1; + const focusedIndex = this.object.focusedSearchIndex ?? -1; if (isUp) { const nextIndex = focusedIndex > 0 ? focusedIndex - 1 : 0; - this.object.setFocusedSearchResult(results[nextIndex] ?? results[0]); + this.object.setFocusedSearchResultAt(nextIndex); return; } if (isDown) { const nextIndex = focusedIndex >= 0 ? Math.min(results.length - 1, focusedIndex + 1) : 0; - this.object.setFocusedSearchResult(results[nextIndex] ?? results[0]); + this.object.setFocusedSearchResultAt(nextIndex); return; } if (isRightOrEnter) { diff --git a/QualityControl/public/object/ObjectTree.class.js b/QualityControl/public/object/ObjectTree.class.js index b6260e2cb..d9d3b777f 100644 --- a/QualityControl/public/object/ObjectTree.class.js +++ b/QualityControl/public/object/ObjectTree.class.js @@ -49,12 +49,37 @@ export default class ObjectTree extends Observable { this.pathString = ''; // 'A/B' } + /** + * Focus the node identified by a slash-separated path (e.g. "A/B/C"). + * Does nothing if path is empty or not found. + * @param {string} pathString - A slash-separated string representing the path to the node (e.g., "A/B/C"). + * @returns {void} + */ + focusNodeByPath(pathString) { + if (!pathString) { + return; + } + const parts = pathString.split('/').filter(Boolean); + const findNode = (node, remainingParts) => { + if (remainingParts.length === 0) { + return node; + } + const [nextPart, ...rest] = remainingParts; + const nextNode = node.children.find((child) => child.name === nextPart); + return nextNode ? findNode(nextNode, rest) : null; + }; + const targetNode = findNode(this, parts); + if (targetNode) { + this._setFocusedNode(targetNode); + } + } + /** * Set the currently focused node * @param {ObjectTree} node - node to be focused * @returns {undefined} */ - setFocusedNode(node) { + _setFocusedNode(node) { this.focusedNode = node; this.notify(); } @@ -74,7 +99,7 @@ export default class ObjectTree extends Observable { return; } parent.open = false; - this.setFocusedNode(parent); + this._setFocusedNode(parent); return; } // If focus is on a branch: collapse it if open, otherwise move focus to parent. @@ -83,7 +108,7 @@ export default class ObjectTree extends Observable { return; } if (this.focusedNode.parent) { - this.setFocusedNode(this.focusedNode.parent); + this._setFocusedNode(this.focusedNode.parent); } } @@ -98,7 +123,7 @@ export default class ObjectTree extends Observable { if (!this.focusedNode.open && this.focusedNode.children.length > 0) { this.focusedNode.toggle(); } else if (this.focusedNode.open && this.focusedNode.children.length > 0) { - this.setFocusedNode(this.focusedNode.children[0]); + this._setFocusedNode(this.focusedNode.children[0]); } } @@ -106,7 +131,7 @@ export default class ObjectTree extends Observable { * Get all visible nodes in the tree (for navigation) * @returns {Array.} - list of visible nodes */ - _getVisibleNodes() { + getVisibleNodes() { const nodes = []; const traverse = (n) => { nodes.push(n); @@ -122,48 +147,50 @@ export default class ObjectTree extends Observable { * Focus the next visible node in the tree */ focusNextNode() { - const visible = this._getVisibleNodes(); + const visible = this.getVisibleNodes(); + // No visible nodes if (!visible.length) { - return; // no visible nodes + return; } const idx = visible.indexOf(this.focusedNode); - // nothing focused yet -> focus first visible node + // Nothing focused yet -> focus first visible node if (!this.focusedNode || idx === -1) { const [first] = visible; - this.setFocusedNode(first); + this._setFocusedNode(first); return; } - // if already at the last visible node, do nothing + // At the last visible node, do nothing if (idx >= visible.length - 1) { return; } - // select next node + // Select next node const next = visible[idx + 1] ?? visible[idx]; - this.setFocusedNode(next); + this._setFocusedNode(next); } /** * Focus the previous visible node in the tree. */ focusPreviousNode() { - const visible = this._getVisibleNodes(); + const visible = this.getVisibleNodes(); + // No visible nodes if (!visible.length) { - return; // no visible nodes + return; } const idx = visible.indexOf(this.focusedNode); - // if already at the first visible node, do nothing + // At the first visible node, do nothing if (idx === 0) { return; } - // nothing focused yet -> focus first visible node + // Nothing focused yet -> focus first visible node if (!this.focusedNode || idx === -1) { const [first] = visible; - this.setFocusedNode(first); + this._setFocusedNode(first); return; } - // select previous node + // Select previous node const prev = idx > 0 ? visible[idx - 1] : visible[0]; - this.setFocusedNode(prev); + this._setFocusedNode(prev); } /** diff --git a/QualityControl/public/object/QCObject.js b/QualityControl/public/object/QCObject.js index 6be04594e..60e2dd87a 100644 --- a/QualityControl/public/object/QCObject.js +++ b/QualityControl/public/object/QCObject.js @@ -43,6 +43,7 @@ export default class QCObject extends BaseViewModel { this.searchInput = ''; // String - content of input search this.searchResult = []; // Array - result list of search this.focusedSearchResult = null; // Object - focused item in search results for keyboard navigation + this.focusedSearchIndex = -1; // Number - index of focused search result this.sortBy = { field: 'name', @@ -86,17 +87,34 @@ export default class QCObject extends BaseViewModel { /** * Set focused item in search results (used by keyboard navigation). - * @param {object} next - next object to be focused + * @param {number} nextIndex - index of the next object to be focused * @returns {undefined} */ - setFocusedSearchResult(next) { - if (!next || next === this.focusedSearchResult) { + setFocusedSearchResult(nextIndex) { + if (!this.searchResult.length) { return; } - this.focusedSearchResult = next; + if (nextIndex < 0 || nextIndex >= this.searchResult.length) { + return; + } + if (nextIndex === this.focusedSearchIndex && + this.searchResult[nextIndex] === this.focusedSearchResult) { + return; + } + this.focusedSearchIndex = nextIndex; + this.focusedSearchResult = this.searchResult[nextIndex]; this.notify(); } + /** + * Focus a search result by index without a linear search. + * @param {number} index - index to focus + * @returns {undefined} + */ + setFocusedSearchResultAt(index) { + this.setFocusedSearchResult(index); + } + /** * Set searched items table UI sizes to allow virtual scrolling * @param {number} scrollTop - position of the user's scroll cursor @@ -407,6 +425,16 @@ export default class QCObject extends BaseViewModel { await this.loadObjectByName(this.selected.name); } + // Move focus to selected object + if (this.searchInput === '') { + this.tree.focusNodeByPath(this.selected.name); + } else { + const idx = this.searchResult.findIndex(({ name }) => name === this.selected.name); + if (idx >= 0) { + this.setFocusedSearchResultAt(idx); + } + } + this.notify(); } @@ -421,6 +449,7 @@ export default class QCObject extends BaseViewModel { this.sortListByField(this.searchResult, this.sortBy.field, this.sortBy.order); this.focusedSearchResult = null; + this.focusedSearchIndex = -1; this.notify(); } From 307137e5e687fe31e8807becddc1715eba2440ad Mon Sep 17 00:00:00 2001 From: crashdance Date: Sat, 10 Jan 2026 16:40:49 +0100 Subject: [PATCH 19/20] tests: refine object tree keyboard navigation tests and added tests for when search is active --- .../test/public/pages/object-tree.test.js | 151 +++++------------- 1 file changed, 36 insertions(+), 115 deletions(-) diff --git a/QualityControl/test/public/pages/object-tree.test.js b/QualityControl/test/public/pages/object-tree.test.js index 22b27c3a3..653c6689e 100644 --- a/QualityControl/test/public/pages/object-tree.test.js +++ b/QualityControl/test/public/pages/object-tree.test.js @@ -339,116 +339,69 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ); await testParent.test( - 'should focus first node when ArrowDown key is pressed', + 'should navigate object tree and search results with arrow keys and enter key', { timeout }, async () => { await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); - await page.keyboard.press('ArrowDown'); // Focus the first node - await delay(500); - const isFirstNodeHighlighted = await page.evaluate(() => { + + // Focus first node + await page.keyboard.press('ArrowDown'); + await delay(200); + const isFirstObjectFocused = await page.evaluate(() => { const selectedNode = document.querySelector('tr.object-selectable'); return selectedNode?.classList.contains('focused-node'); }); - strictEqual(isFirstNodeHighlighted, true, 'The first node is not highlighted.'); - }, - ); + strictEqual(isFirstObjectFocused, true, 'The first object is not focused.'); - await testParent.test( - 'should focus next node when ArrowDown key is pressed', - { timeout }, - async () => { - await page.keyboard.press('ArrowDown'); // Focus to the next (second) node - await delay(500); - const isSecondNodeHighlighted = await page.evaluate(() => { + // Focus second node and expand + await page.keyboard.press('ArrowDown'); // Focus second node + await page.keyboard.press('ArrowRight'); // Expand focused node + await delay(200); + const isNodeExpanded = await page.evaluate(() => { const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); return selectedNode?.classList.contains('focused-node'); }); - strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted.'); - }, - ); + strictEqual(isNodeExpanded, true, 'The focused node was not expanded on pressing ArrowRight key.'); - await testParent.test( - 'should expand tree when ArrowRight key is pressed on a focused node', - { timeout }, - async () => { - await page.keyboard.press('ArrowRight'); // Expand the focused node - await delay(500); - // Verify that the third node is visible after expanding - const isNodeExpanded = await page.evaluate(() => { - const nodes = document.querySelectorAll('tr.object-selectable'); - return nodes.length >= 3; // Check if there are at least 3 nodes - }); - strictEqual(isNodeExpanded, true, 'The third node is not present after expanding the second node.'); - }, - ); - - await testParent.test( - 'should select object when ArrowRight key is pressed on a focused object node', - { timeout }, - async () => { - await page.keyboard.press('ArrowDown'); // Focus to the third node - await delay(200); - await page.keyboard.press('ArrowRight'); // Expand the third node - await delay(200); - await page.keyboard.press('ArrowDown'); // Focus next node (object node) - await delay(200); - await page.keyboard.press('ArrowRight'); // Select the object node + // Focus third node, expand and select a leaf node + await page.keyboard.press('ArrowDown'); // Focus third node + await page.keyboard.press('ArrowRight'); // Expand focused node + await page.keyboard.press('ArrowDown'); // Focus fourth node + await page.keyboard.press('Enter'); // Select focused leaf node await delay(500); const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); const isObjectPlotOpened = await page.evaluate(() => { const objectPanel = document.querySelector('#qcObjectInfoPanel'); return objectPanel !== null && objectPanel !== undefined; }); - strictEqual(isObjectSelected, true, 'The focused object node was not selected on pressing ArrowRight key.'); - strictEqual(isObjectPlotOpened, true, 'The object plot panel is not opened after selecting the object node.'); - }, - ); + strictEqual(isObjectSelected, true, 'Focused leaf node was not selected on pressing ArrowRight key.'); + strictEqual(isObjectPlotOpened, true, 'Object plot panel is not opened after selecting the focused leaf node.'); - await testParent.test( - 'should collapse parent node when ArrowLeft key is pressed on a selected object node', - { timeout }, - async () => { - const nodeCountBefore = await page.evaluate(() => { - const nodes = document.querySelectorAll('tr.object-selectable'); - return nodes.length; - }); - ok(nodeCountBefore > 3, `Expected node count to be greater than 3, but got ${nodeCountBefore}`); - await page.keyboard.press('ArrowLeft'); // Collapse the parent node of the selected object + // Collapse parent node of the focused leaf node + await page.keyboard.press('ArrowLeft'); await delay(200); - const nodeCountAfter = await page.evaluate(() => { + const nodeCountAfterCollapse = await page.evaluate(() => { const nodes = document.querySelectorAll('tr.object-selectable'); return nodes.length; }); strictEqual( - nodeCountAfter, + nodeCountAfterCollapse, 3, 'The object tree navigation does not have exactly 3 nodes after collapsing the parent.', ); - }, - ); - await testParent.test( - 'should focus previous node when ArrowUp key is pressed', - { timeout }, - async () => { - await page.keyboard.press('ArrowUp'); // Move back to the second child node + // Focus previous node + await page.keyboard.press('ArrowUp'); await delay(200); - // Verify the second node is highlighted - const isSecondNodeHighlighted = await page.evaluate(() => { + const isSecondNodeHighlightedAgain = await page.evaluate(() => { const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); return selectedNode?.classList.contains('focused-node'); }); - strictEqual(isSecondNodeHighlighted, true, 'The second node is not highlighted after pressing ArrowUp key.'); - }, - ); + strictEqual(isSecondNodeHighlightedAgain, true, 'The second node is not highlighted after pressing ArrowUp key.'); - await testParent.test( - 'should collapse tree when ArrowLeft key is pressed on a focused node', - { timeout }, - async () => { - await page.keyboard.press('ArrowLeft'); // Collapse the second node + // Collapse tree + await page.keyboard.press('ArrowLeft'); await delay(200); - // Verify that the third node is gone after collapsing const isNodeCollapsed = await page.evaluate(() => { const nodes = document.querySelectorAll('tr.object-selectable'); return nodes.length < 3; // Check if there are less than 3 nodes @@ -458,56 +411,24 @@ export const objectTreePageTests = async (url, page, timeout = 5000, testParent) ); await testParent.test( - 'should focus first object in search results when ArrowDown key is pressed and search is active', + 'should navigate object tree and search results with arrow keys and enter key when search active', { timeout }, async () => { - // reset state by navigating to the object tree page again await page.goto(`${url}${OBJECT_TREE_PAGE_PARAM}`, { waitUntil: 'networkidle0' }); await page.focus('#searchObjectTree'); await page.type('#searchObjectTree', 'qc/test/object'); - await page.keyboard.press('ArrowDown'); // Focus first object in search results - await delay(200); - const isFirstObjectHighlighted = await page.evaluate(() => { - const selectedNode = document.querySelector('tr.object-selectable'); - return selectedNode?.classList.contains('focused-node'); - }); - strictEqual(isFirstObjectHighlighted, true, 'The first object in search results is not highlighted.'); - }, - ); - await testParent.test( - 'should focus next object in search results when ArrowDown key is pressed and search is active', - { timeout }, - async () => { - await page.keyboard.press('ArrowDown'); // Focus next object in search results - await delay(500); - const isSecondObjectHighlighted = await page.evaluate(() => { - const [, selectedNode] = document.querySelectorAll('tr.object-selectable'); - return selectedNode?.classList.contains('focused-node'); - }); - strictEqual(isSecondObjectHighlighted, true, 'The second object in search results is not highlighted.'); - }, - ); - - await testParent.test( - 'should focus previous object in search results when ArrowUp key is pressed and search is active', - { timeout }, - async () => { - await page.keyboard.press('ArrowUp'); // Focus previous object in search results + // Focus first object in search results + await page.keyboard.press('ArrowDown'); await delay(200); const isFirstObjectHighlighted = await page.evaluate(() => { const selectedNode = document.querySelector('tr.object-selectable'); return selectedNode?.classList.contains('focused-node'); }); - strictEqual(isFirstObjectHighlighted, true, 'The first object in search results is not highlighted.'); - }, - ); + strictEqual(isFirstObjectHighlighted, true, 'The first object in search results is not highlighted.'); - await testParent.test( - 'should select focused object in search results when ArrowRight key is pressed and search is active', - { timeout }, - async () => { - await page.keyboard.press('ArrowRight'); // Select focused object + // Select focused object + await page.keyboard.press('Enter'); await delay(500); const isObjectSelected = await page.evaluate(() => model.object.selected !== undefined); const isObjectPlotOpened = await page.evaluate(() => { From 0875a1b4e667cadef0064e70a646dbfd61b3df33 Mon Sep 17 00:00:00 2001 From: crashdance Date: Sun, 11 Jan 2026 11:44:47 +0100 Subject: [PATCH 20/20] refactor: simplify search result handling in keyboard navigation --- QualityControl/public/Model.js | 8 +++---- QualityControl/public/object/QCObject.js | 29 ++++++++---------------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/QualityControl/public/Model.js b/QualityControl/public/Model.js index 67a2120eb..06c8bbc7c 100644 --- a/QualityControl/public/Model.js +++ b/QualityControl/public/Model.js @@ -152,15 +152,13 @@ export default class Model extends Observable { if (!results.length) { return; } - const focusedIndex = this.object.focusedSearchIndex ?? -1; + const focusedIndex = this.object.focusedSearchIndex; if (isUp) { - const nextIndex = focusedIndex > 0 ? focusedIndex - 1 : 0; - this.object.setFocusedSearchResultAt(nextIndex); + this.object.setFocusedSearchResult(focusedIndex - 1); return; } if (isDown) { - const nextIndex = focusedIndex >= 0 ? Math.min(results.length - 1, focusedIndex + 1) : 0; - this.object.setFocusedSearchResultAt(nextIndex); + this.object.setFocusedSearchResult(focusedIndex + 1); return; } if (isRightOrEnter) { diff --git a/QualityControl/public/object/QCObject.js b/QualityControl/public/object/QCObject.js index 60e2dd87a..1e0e0be18 100644 --- a/QualityControl/public/object/QCObject.js +++ b/QualityControl/public/object/QCObject.js @@ -87,34 +87,25 @@ export default class QCObject extends BaseViewModel { /** * Set focused item in search results (used by keyboard navigation). - * @param {number} nextIndex - index of the next object to be focused + * @param {number} index - index of the next object to be focused * @returns {undefined} */ - setFocusedSearchResult(nextIndex) { + setFocusedSearchResult(index) { if (!this.searchResult.length) { - return; + return; // list empty, nothing to focus } - if (nextIndex < 0 || nextIndex >= this.searchResult.length) { - return; + if (index < 0 || index >= this.searchResult.length) { + return; // out of bounds, do nothing } - if (nextIndex === this.focusedSearchIndex && - this.searchResult[nextIndex] === this.focusedSearchResult) { - return; + if (index === this.focusedSearchIndex && + this.searchResult[index] === this.focusedSearchResult) { + return; // already focused, avoid notify } - this.focusedSearchIndex = nextIndex; - this.focusedSearchResult = this.searchResult[nextIndex]; + this.focusedSearchIndex = index; + this.focusedSearchResult = this.searchResult[index]; this.notify(); } - /** - * Focus a search result by index without a linear search. - * @param {number} index - index to focus - * @returns {undefined} - */ - setFocusedSearchResultAt(index) { - this.setFocusedSearchResult(index); - } - /** * Set searched items table UI sizes to allow virtual scrolling * @param {number} scrollTop - position of the user's scroll cursor