From a9f25733b0663440056aefd330001daf9b05b577 Mon Sep 17 00:00:00 2001 From: MartinBozhilov-coh Date: Mon, 7 Apr 2025 13:43:54 +0300 Subject: [PATCH 1/5] Fix spatial navigation not focusing correct element when scroll is present --- .../src/lib_components/spatial-navigation.js | 130 ++++++++++++++---- 1 file changed, 101 insertions(+), 29 deletions(-) diff --git a/interaction-manager/src/lib_components/spatial-navigation.js b/interaction-manager/src/lib_components/spatial-navigation.js index 9a39123e..f6855096 100644 --- a/interaction-manager/src/lib_components/spatial-navigation.js +++ b/interaction-manager/src/lib_components/spatial-navigation.js @@ -27,11 +27,12 @@ class SpatialNavigation { // eslint-disable-next-line require-jsdoc constructor() { this.enabled = false; - this.navigatableElements = { default: [] }; + this.navigatableElements = { default: { elements: [], distance: 0 } }; this.registeredKeys = new Set(); this.clearCurrentActiveKeys = false; this.overlapPercentage = 0.5; this.lastFocusedElement = null; + this.overflow = { x: 0, y: 0 }; } /** @@ -63,7 +64,7 @@ class SpatialNavigation { if (!this.enabled) return; this.enabled = false; - this.navigatableElements = { default: [] }; + this.navigatableElements = { default: { elements: [], distance: 0 } }; this.removeKeyActions(); this.overlapPercentage = 0.5; this.lastFocusedElement = null; @@ -109,7 +110,8 @@ class SpatialNavigation { domElements.forEach(this.makeFocusable); - this.navigatableElements.default.push(...domElements); + this.navigatableElements.default.elements.push(...domElements); + this.navigatableElements.default.distance = this.getElementsDistance(this.navigatableElements.default.elements); } /** @@ -130,9 +132,49 @@ class SpatialNavigation { if (domElements.length === 0) return console.error(`${navArea.elements.join(', ')} are either not a correct selectors or the elements are not present in the DOM.`); - if (!this.navigatableElements[navArea.area]) this.navigatableElements[navArea.area] = []; + if (!this.navigatableElements[navArea.area]) { + this.navigatableElements[navArea.area] = { elements: [], distance: 0 }; + } + + this.navigatableElements[navArea.area].elements.push(...domElements); + this.navigatableElements[navArea.area].distance = this.getElementsDistance(domElements); + } + + /** + * Calculates the distance between the provided elements and return the max distance + * @param {HTMLElement[]} elements + * @returns {number} - The max distance between the elements + */ + getElementsDistance(elements) { + const distances = elements.map((el) => { + const { x, y } = el.getBoundingClientRect(); + return Math.hypot(x, y); + }); + + this.setOverflowValues(elements[0].parentElement); + + return Math.max(...distances); + } + + /** + * Calculates the distance between the provided elements and return the max distance + * @param {HTMLElement[]} element + * @returns {number} - The max distance between the elements + */ + setOverflowValues(element) { + if (!element) return null; // Base case: reached the root + + const { scrollWidth, scrollHeight } = element; + const overflowX = Math.max(0, scrollWidth - window.innerWidth); + const overflowY = Math.max(0, scrollHeight - window.innerHeight); + + if (overflowX > 0 || overflowY > 0) { + this.overflow = { x: overflowX, y: overflowY }; + return; + } - this.navigatableElements[navArea.area].push(...domElements); + // Recursively check the parent element + return this.setOverflowValues(element.parentElement); } /** @@ -144,25 +186,39 @@ class SpatialNavigation { } /** - * Checks if the passed element is within a group and returns the rest of the elements in the group + * Returns the valid focusable elements in the navigatable area * @param {HTMLElement} targetElement + * @param {HTMLElement[]} elements + * @param {number} distance * @returns {NavigationObject[]} */ - getFocusableGroup(targetElement) { - return Object.values(this.navigatableElements).reduce((acc, el) => { - if (el.includes(targetElement)) { - acc = el.reduce((accumulator, element) => { - if (element !== targetElement && !element.hasAttribute('disabled')) { - const { x, y, height, width } = element.getBoundingClientRect(); - accumulator.push({ element, x, y, height, width }); - } - return accumulator; - }, []); + getFocusableGroup(targetElement, elements, distance) { + return elements.reduce((accumulator, element) => { + if (element !== targetElement && !element.hasAttribute('disabled')) { + const { x, y, height, width } = element.getBoundingClientRect(); + accumulator.push({ + element, + x: x + distance, + y: y + distance, + height, + width, + }); } - return acc; + return accumulator; }, []); } + /** + * Checks if the passed element is within a group and returns the rest of the elements in the group + * @param {HTMLElement} targetElement + * @returns {NavigationObject[]} + */ + getCurrentArea(targetElement) { + return Object.values(this.navigatableElements).find((area) => { + if (area.elements.includes(targetElement)) return true; + }); + } + /** * Gets the element closest to the opposite edge of the navigation direction * @param {string} direction @@ -170,10 +226,14 @@ class SpatialNavigation { * @param {Object} focusedElement * @param {number} focusedElement.x * @param {number} focusedElement.y + * @param {number} distance * @returns {NavigationObject} */ - getClosestToEdge(direction, elements, focusedElement) { + getClosestToEdge(direction, elements, focusedElement, distance) { let newDistance, oldDistance; + const bottomEdge = window.innerHeight + distance + this.overflow.y; + const rightEdge = window.innerWidth + distance + this.overflow.x; + return elements.reduce((acc, el) => { switch (direction) { case 'down': @@ -181,16 +241,16 @@ class SpatialNavigation { oldDistance = Math.hypot(acc.x - focusedElement.x, acc.y); break; case 'up': - newDistance = Math.hypot(el.x - focusedElement.x, window.innerHeight - el.y); - oldDistance = Math.hypot(acc.x - focusedElement.x, window.innerHeight - acc.y); + newDistance = Math.hypot(el.x - focusedElement.x, bottomEdge - el.y); + oldDistance = Math.hypot(acc.x - focusedElement.x, bottomEdge - acc.y); break; case 'right': newDistance = Math.hypot(el.x, el.y - focusedElement.y); oldDistance = Math.hypot(acc.x, acc.y - focusedElement.y); break; case 'left': - newDistance = Math.hypot(window.innerWidth - el.x - el.width, el.y - focusedElement.y); - oldDistance = Math.hypot(window.innerWidth - acc.x - acc.width, acc.y - focusedElement.y); + newDistance = Math.hypot(rightEdge - el.x, el.y - focusedElement.y); + oldDistance = Math.hypot(rightEdge - acc.x, acc.y - focusedElement.y); break; } acc = newDistance < oldDistance ? el : acc; @@ -209,20 +269,32 @@ class SpatialNavigation { const activeElement = this.checkActiveElementInGroup(); - const focusableGroup = this.getFocusableGroup(activeElement); + const currentArea = this.getCurrentArea(activeElement); + if (!currentArea) return console.error('The active element is not in a focusable area!'); + + const distance = currentArea.distance; + const focusableGroup = this.getFocusableGroup(activeElement, currentArea.elements, distance); const { x, y, width, height } = activeElement.getBoundingClientRect(); + const adjustedDimensions = { + x: x + distance, + y: y + distance, + width, + height, + }; + if (focusableGroup.length === 0) return; - const currentAxisGroup = this.filterGroupByCurrentAxis(direction, focusableGroup, { x, y, width, height }); + const currentAxisGroup = this.filterGroupByCurrentAxis(direction, focusableGroup, adjustedDimensions); if (!currentAxisGroup.length) return; - let nextFocusableElement = this.findNextElement(direction, currentAxisGroup, x, y); + let nextFocusableElement = this.findNextElement( + direction, currentAxisGroup, adjustedDimensions.x, adjustedDimensions.y); if (!nextFocusableElement) { - nextFocusableElement = this.getClosestToEdge(direction, currentAxisGroup, { x, y }); + nextFocusableElement = this.getClosestToEdge(direction, currentAxisGroup, adjustedDimensions, distance); } if (nextFocusableElement) { @@ -414,7 +486,7 @@ class SpatialNavigation { return console.error(`The area '${area}' you are trying to focus doesn't exist or the spatial navigation hasn't been initialized`); } - this.lastFocusedElement = navigatableElements[0]; + this.lastFocusedElement = navigatableElements.elements[0]; this.lastFocusedElement.focus(); } @@ -431,7 +503,7 @@ class SpatialNavigation { return console.error(`The area '${area}' you are trying to focus doesn't exist or the spatial navigation hasn't been initialized`); } - this.lastFocusedElement = navigatableElements.slice(-1)[0]; + this.lastFocusedElement = navigatableElements.elements.slice(-1)[0]; this.lastFocusedElement.focus(); } @@ -448,7 +520,7 @@ class SpatialNavigation { * @returns {boolean} */ isActiveElementInGroup() { - return Object.values(this.navigatableElements).some(group => group.includes(document.activeElement)); + return Object.values(this.navigatableElements).some(group => group.elements.includes(document.activeElement)); } /** From 2e2f830e5c0998c0326fa9ff9613cbc896a354f1 Mon Sep 17 00:00:00 2001 From: MartinBozhilov-coh Date: Mon, 7 Apr 2025 13:47:06 +0300 Subject: [PATCH 2/5] Increment npm version --- interaction-manager/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interaction-manager/package.json b/interaction-manager/package.json index 01a15e98..727701e8 100644 --- a/interaction-manager/package.json +++ b/interaction-manager/package.json @@ -1,6 +1,6 @@ { "name": "coherent-gameface-interaction-manager", - "version": "2.3.2", + "version": "2.3.3", "description": "Library for the most common UI interactions", "main": "dist/interaction-manager.js", "module": "esm/interaction-manager.js", From 24a7876e8529426a6ba0d56241f873fe9a05b192 Mon Sep 17 00:00:00 2001 From: MartinBozhilov-coh Date: Mon, 7 Apr 2025 14:10:58 +0300 Subject: [PATCH 3/5] Update method description --- .../src/lib_components/spatial-navigation.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/interaction-manager/src/lib_components/spatial-navigation.js b/interaction-manager/src/lib_components/spatial-navigation.js index f6855096..95b5c9b3 100644 --- a/interaction-manager/src/lib_components/spatial-navigation.js +++ b/interaction-manager/src/lib_components/spatial-navigation.js @@ -157,9 +157,9 @@ class SpatialNavigation { } /** - * Calculates the distance between the provided elements and return the max distance - * @param {HTMLElement[]} element - * @returns {number} - The max distance between the elements + * Recursively checks for overflow in the parent elements and sets the global overflow values + * @param {HTMLElement} element - The element to check for overflow + * @returns {HTMLElement|null} - Next element to check for overflow */ setOverflowValues(element) { if (!element) return null; // Base case: reached the root @@ -272,8 +272,8 @@ class SpatialNavigation { const currentArea = this.getCurrentArea(activeElement); if (!currentArea) return console.error('The active element is not in a focusable area!'); - const distance = currentArea.distance; - const focusableGroup = this.getFocusableGroup(activeElement, currentArea.elements, distance); + const { elements, distance } = currentArea; + const focusableGroup = this.getFocusableGroup(activeElement, elements, distance); const { x, y, width, height } = activeElement.getBoundingClientRect(); From 7b48f13ea8e96c2e4f6436812e8109d75363d46f Mon Sep 17 00:00:00 2001 From: MartinBozhilov-coh Date: Mon, 7 Apr 2025 15:05:55 +0300 Subject: [PATCH 4/5] Make overflow be navigatableArea property instead of global and fix focus bug --- .../src/lib_components/spatial-navigation.js | 68 ++++++++++++------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/interaction-manager/src/lib_components/spatial-navigation.js b/interaction-manager/src/lib_components/spatial-navigation.js index 95b5c9b3..18bd3c71 100644 --- a/interaction-manager/src/lib_components/spatial-navigation.js +++ b/interaction-manager/src/lib_components/spatial-navigation.js @@ -27,12 +27,11 @@ class SpatialNavigation { // eslint-disable-next-line require-jsdoc constructor() { this.enabled = false; - this.navigatableElements = { default: { elements: [], distance: 0 } }; + this.navigatableElements = { default: { elements: [], distance: 0, overflow: { x: 0, y: 0 } } }; this.registeredKeys = new Set(); this.clearCurrentActiveKeys = false; this.overlapPercentage = 0.5; this.lastFocusedElement = null; - this.overflow = { x: 0, y: 0 }; } /** @@ -64,7 +63,7 @@ class SpatialNavigation { if (!this.enabled) return; this.enabled = false; - this.navigatableElements = { default: { elements: [], distance: 0 } }; + this.navigatableElements = { default: { elements: [], distance: 0, overflow: { x: 0, y: 0 } } }; this.removeKeyActions(); this.overlapPercentage = 0.5; this.lastFocusedElement = null; @@ -110,8 +109,7 @@ class SpatialNavigation { domElements.forEach(this.makeFocusable); - this.navigatableElements.default.elements.push(...domElements); - this.navigatableElements.default.distance = this.getElementsDistance(this.navigatableElements.default.elements); + this.setNavigationAreaProperties('default', domElements); } /** @@ -136,8 +134,17 @@ class SpatialNavigation { this.navigatableElements[navArea.area] = { elements: [], distance: 0 }; } - this.navigatableElements[navArea.area].elements.push(...domElements); - this.navigatableElements[navArea.area].distance = this.getElementsDistance(domElements); + this.setNavigationAreaProperties(navArea.area, domElements); + } + + /** + * @param {string} area - The area to set the properties for + * @param {HTMLElement[]} domElements - The elements to be added to the area + */ + setNavigationAreaProperties(area, domElements) { + this.navigatableElements[area].elements.push(...domElements); + this.navigatableElements[area].distance = this.getElementsDistance(this.navigatableElements[area].elements); + this.navigatableElements[area].overflow = this.setOverflowValues(domElements[0].parentElement); } /** @@ -151,29 +158,25 @@ class SpatialNavigation { return Math.hypot(x, y); }); - this.setOverflowValues(elements[0].parentElement); - return Math.max(...distances); } /** - * Recursively checks for overflow in the parent elements and sets the global overflow values + * Recursively checks for overflow in the parent elements and sets the area overflow values * @param {HTMLElement} element - The element to check for overflow - * @returns {HTMLElement|null} - Next element to check for overflow + * @returns {{x: number, y: number}|HTMLElement} - Next element to check for overflow or object with the overflow values */ setOverflowValues(element) { - if (!element) return null; // Base case: reached the root + if (!element) return { x: 0, y: 0 }; const { scrollWidth, scrollHeight } = element; const overflowX = Math.max(0, scrollWidth - window.innerWidth); const overflowY = Math.max(0, scrollHeight - window.innerHeight); if (overflowX > 0 || overflowY > 0) { - this.overflow = { x: overflowX, y: overflowY }; - return; + return { x: overflowX, y: overflowY }; } - // Recursively check the parent element return this.setOverflowValues(element.parentElement); } @@ -227,12 +230,13 @@ class SpatialNavigation { * @param {number} focusedElement.x * @param {number} focusedElement.y * @param {number} distance + * @param {{x: number, y: number}} overflow * @returns {NavigationObject} */ - getClosestToEdge(direction, elements, focusedElement, distance) { + getClosestToEdge(direction, elements, focusedElement, distance, overflow) { let newDistance, oldDistance; - const bottomEdge = window.innerHeight + distance + this.overflow.y; - const rightEdge = window.innerWidth + distance + this.overflow.x; + const bottomEdge = window.innerHeight + distance + overflow.y; + const rightEdge = window.innerWidth + distance + overflow.x; return elements.reduce((acc, el) => { switch (direction) { @@ -272,7 +276,7 @@ class SpatialNavigation { const currentArea = this.getCurrentArea(activeElement); if (!currentArea) return console.error('The active element is not in a focusable area!'); - const { elements, distance } = currentArea; + const { elements, distance, overflow } = currentArea; const focusableGroup = this.getFocusableGroup(activeElement, elements, distance); const { x, y, width, height } = activeElement.getBoundingClientRect(); @@ -294,7 +298,9 @@ class SpatialNavigation { direction, currentAxisGroup, adjustedDimensions.x, adjustedDimensions.y); if (!nextFocusableElement) { - nextFocusableElement = this.getClosestToEdge(direction, currentAxisGroup, adjustedDimensions, distance); + nextFocusableElement = this.getClosestToEdge( + direction, currentAxisGroup, adjustedDimensions, distance, overflow + ); } if (nextFocusableElement) { @@ -481,12 +487,15 @@ class SpatialNavigation { * @returns {void} */ focusFirst(area = 'default') { - const navigatableElements = this.navigatableElements[area]; + const navigatableElements = this.navigatableElements[area].elements; if (!navigatableElements || navigatableElements.length === 0) { return console.error(`The area '${area}' you are trying to focus doesn't exist or the spatial navigation hasn't been initialized`); } - this.lastFocusedElement = navigatableElements.elements[0]; + this.lastFocusedElement = navigatableElements.find(el => !el.hasAttribute('disabled')); + if (!this.lastFocusedElement) { + return console.error(`The area '${area}' you are trying to focus doesn't have any focusable elements`); + } this.lastFocusedElement.focus(); } @@ -498,12 +507,23 @@ class SpatialNavigation { focusLast(area = 'default') { if (!this.enabled) return; - const navigatableElements = this.navigatableElements[area]; + const navigatableElements = this.navigatableElements[area].elements; if (!navigatableElements || navigatableElements.length === 0) { return console.error(`The area '${area}' you are trying to focus doesn't exist or the spatial navigation hasn't been initialized`); } - this.lastFocusedElement = navigatableElements.elements.slice(-1)[0]; + let element; + for (let i = navigatableElements.length - 1; i >= 0; i--) { + if (!navigatableElements[i].hasAttribute('disabled')) { + element = navigatableElements[i]; + break; + } + } + + this.lastFocusedElement = element; + if (!this.lastFocusedElement) { + return console.error(`The area '${area}' you are trying to focus doesn't have any focusable elements`); + } this.lastFocusedElement.focus(); } From 612437dae0015b9204ab2878b5b75ae29741f294 Mon Sep 17 00:00:00 2001 From: MartinBozhilov-coh Date: Mon, 7 Apr 2025 15:59:48 +0300 Subject: [PATCH 5/5] Resolve PR --- .../src/lib_components/spatial-navigation.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/interaction-manager/src/lib_components/spatial-navigation.js b/interaction-manager/src/lib_components/spatial-navigation.js index 18bd3c71..53186ad5 100644 --- a/interaction-manager/src/lib_components/spatial-navigation.js +++ b/interaction-manager/src/lib_components/spatial-navigation.js @@ -92,9 +92,9 @@ class SpatialNavigation { if (!this.navigatableElements[area]) return console.error(`The area '${area}' you are trying to remove doesn't exist`); - this.navigatableElements[area].forEach(element => element.removeAttribute('tabindex')); + this.navigatableElements[area].elements.forEach(element => element.removeAttribute('tabindex')); - this.navigatableElements[area].length = 0; + this.navigatableElements[area] = {}; } /** @@ -153,12 +153,11 @@ class SpatialNavigation { * @returns {number} - The max distance between the elements */ getElementsDistance(elements) { - const distances = elements.map((el) => { + return elements.reduce((acc, el) => { const { x, y } = el.getBoundingClientRect(); - return Math.hypot(x, y); - }); - - return Math.max(...distances); + const distance = Math.hypot(x, y); + return acc < distance ? distance : acc; + }, 0); } /**