Refactor keyboard grid navigation for JSDialog dropdowns

- Made `KeyboardGridNavigation` callable directly from JSDialog dropdowns, removing the need for explicit bindings. - Separated grid and list selection logic into dedicated files. - Streamlined navigation logic to handle only navigation, simplifying callback management. -Refactored navigation to focus solely on movement logic, removing selection handling. - Simplified callback management by decoupling selection logic, improving maintainability. Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I8e34ddaf44a79eecf57a428f7ed90c6a399d93c2
CollaboraOnline · Nov 27, 2024 · 0c6b3b5 · 0c6b3b5
1 parent 601bee1
commit 0c6b3b5
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 29 deletions.
diff --git a/browser/src/control/jsdialog/Util.KeyboardGridNavigation.ts b/browser/src/control/jsdialog/Util.KeyboardGridNavigation.ts
@@ -19,18 +19,8 @@ function handleKeyboardNavigation(
 	event: KeyboardEvent,
 	currentElement: HTMLElement,
 	parentContainer: HTMLElement,
-	handleSelection?: any,
-	builder?: any,
-	widgetData?: any,
 ) {
 	switch (event.key) {
-		case 'Enter':
-		case ' ':
-			if (handleSelection && !(currentElement instanceof HTMLSelectElement)) {
-				handleSelection(currentElement, builder, widgetData);
-				event.preventDefault();
-			}
-			break;
 		case 'ArrowRight':
 			moveFocus(parentContainer, currentElement, 'next', 'horizontal');
 			event.preventDefault();
@@ -93,6 +83,16 @@ function moveFocus(
 		} else {
 			targetRow--;
 		}
+
+		// If the target column does not exist in the target row, move to the last column in the target row
+		const elementsInTargetRow = focusableElements.filter(
+			(el) => getRowColumn(el)[0] === targetRow,
+		);
+		if (elementsInTargetRow.length > 0) {
+			if (targetColumn >= elementsInTargetRow.length) {
+				targetColumn = elementsInTargetRow.length - 1;
+			}
+		}
 	}
 
 	const targetElement = focusableElements.find(
@@ -103,10 +103,10 @@ function moveFocus(
 	if (targetElement) {
 		targetElement.focus();
 	} else {
-		// Determine the element to pass to FindFocusableElement based on axis and direction
+		// Handle edge cases by moving to adjacent sibling elements if no exact match is found
 		const potentialCurrentElement =
 			axis === 'vertical' ? parentContainer : currentElement;
-		// if vertical or
+
 		const nextElement =
 			direction === 'next'
 				? JSDialog.FindFocusableElement(
@@ -130,22 +130,14 @@ function getRowColumn(element: HTMLElement): [number, number] {
 	return [row, column];
 }
 
-JSDialog.KeyboardGridNavigation = function (
-	container: HTMLElement,
-	selectionHandler?: any,
-	builder?: any,
-	widgetData?: any,
-) {
+JSDialog.KeyboardGridNavigation = function (container: HTMLElement) {
 	container.addEventListener('keydown', (event: KeyboardEvent) => {
 		const activeElement = document.activeElement as HTMLElement;
 		if (!JSDialog.IsTextInputField(activeElement)) {
 			handleKeyboardNavigation(
 				event,
 				activeElement,
 				activeElement.parentElement,
-				selectionHandler,
-				builder,
-				widgetData,
 			);
 		}
 	});

diff --git a/browser/src/control/jsdialog/Widget.ColorPicker.ts b/browser/src/control/jsdialog/Widget.ColorPicker.ts
@@ -117,6 +117,15 @@ function createColor(
 			widgetData, // Pass the widget data
 		);
 	});
+	color.addEventListener('keydown', (event: KeyboardEvent) => {
+		if (event.code === 'Enter') {
+			handleColorSelection(
+				event.target as HTMLElement, // The clicked element
+				builder, // Pass the builder object
+				widgetData, // Pass the widget data
+			);
+		}
+	});
 	return color;
 }
 
@@ -377,14 +386,6 @@ JSDialog.colorPicker = function (
 		);
 	});
 
-	// Apply keyboard navigation to the color picker widget
-	JSDialog.KeyboardGridNavigation(
-		container,
-		handleColorSelection,
-		builder,
-		data,
-	);
-
 	JSDialog.MakeFocusCycle(container);
 
 	return false;