Improve keyboard navigation for data viewer filter dialogs (#4620)

- If focus is on the column drop down, then pressing a key will immediately open the search sub-dialog and use that key as an initial search input. - Updating the search string will always place the cursor on the first available item - Hitting "ArrowDown" or "Tab" from within the search input will shift focus to the list and allow keyboard navigation as expected - Hitting "Enter" or "Space" when navigating the list will confirm the selection I've also added a new style to the filter widgets to use monospace font for the values. This makes them more legible in my opinion, particularly with values which contain "l" "1" and "I".
posit-dev · Sep 30, 2024 · 1570041 · 1570041
1 parent 659e073
commit 1570041
Show file tree

Hide file tree

Showing 9 changed files with 142 additions and 56 deletions.
diff --git a/...nents/dataExplorerPanel/components/addEditRowFilterModalPopup/components/columnSearch.tsx b/...nents/dataExplorerPanel/components/addEditRowFilterModalPopup/components/columnSearch.tsx
@@ -8,7 +8,7 @@ import 'vs/css!./columnSearch';
 
 // React.
 import * as React from 'react';
-import { useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports
+import { useEffect, useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports
 
 // Other dependencies.
 import { localize } from 'vs/nls';
@@ -19,7 +19,10 @@ import { positronClassNames } from 'vs/base/common/positronUtilities';
  */
 interface ColumnSearchProps {
 	initialSearchText?: string;
+	focus?: boolean;
 	onSearchTextChanged: (searchText: string) => void;
+	onNavigateOut?: (searchText: string) => void;
+	onConfirmSearch?: (searchText: string) => void;
 }
 
 /**
@@ -31,10 +34,33 @@ export const ColumnSearch = (props: ColumnSearchProps) => {
 	// Reference hooks.
 	const inputRef = useRef<HTMLInputElement>(undefined!);
 
+	useEffect(() => {
+		if (!props.focus) { return; }
+		inputRef.current.focus();
+	}, [inputRef, props.focus]);
+
 	// State hooks.
 	const [focused, setFocused] = useState(false);
 	const [searchText, setSearchText] = useState(props.initialSearchText ?? '');
 
+	const handleOnKeyDown = (evt: React.KeyboardEvent<HTMLInputElement>) => {
+		switch (evt.code) {
+			case 'ArrowDown':
+			case 'Tab':
+				if (!props.onNavigateOut) { break; }
+				evt.stopPropagation();
+				evt.preventDefault();
+				props.onNavigateOut?.(evt.currentTarget.value);
+				break;
+			case 'Enter':
+				if (!props.onConfirmSearch) { break; }
+				evt.stopPropagation();
+				evt.preventDefault();
+				props.onConfirmSearch?.(evt.currentTarget.value);
+				break;
+		}
+	};
+
 	// Render.
 	return (
 		<div className='column-search-container'>
@@ -45,6 +71,7 @@ export const ColumnSearch = (props: ColumnSearchProps) => {
 					className='text-input'
 					placeholder={(() => localize('positron.searchPlacehold', "search"))()}
 					value={searchText}
+					onKeyDown={handleOnKeyDown}
 					onFocus={() => setFocused(true)}
 					onBlur={() => setFocused(false)}
 					onChange={e => {

diff --git a/...Panel/components/addEditRowFilterModalPopup/components/columnSelectorDataGridInstance.tsx b/...Panel/components/addEditRowFilterModalPopup/components/columnSelectorDataGridInstance.tsx
@@ -139,6 +139,14 @@ export class ColumnSelectorDataGridInstance extends DataGridInstance {
 		return ROW_HEIGHT;
 	}
 
+	selectItem(rowIndex: number): void {
+		// Get the column schema for the row index.
+		const columnSchema = this._columnSchemaCache.getColumnSchema(rowIndex);
+		if (!columnSchema) { return; }
+
+		this._onDidSelectColumnEmitter.fire(columnSchema);
+	}
+
 	/**
 	 * Gets a cell.
 	 * @param columnIndex The column index.
@@ -190,6 +198,13 @@ export class ColumnSelectorDataGridInstance extends DataGridInstance {
 			// Set the search text and fetch data.
 			this._searchText = searchText;
 			await this.fetchData();
+
+			// select the first available row after fetching so that users cat hit "enter"
+			// to make an immediate confirmation on what they were searching for
+			if (this.visibleRows) {
+				this.showCursor();
+				this.setCursorRow(0);
+			}
 		}
 	}
 

diff --git a/...plorerPanel/components/addEditRowFilterModalPopup/components/columnSelectorModalPopup.tsx b/...plorerPanel/components/addEditRowFilterModalPopup/components/columnSelectorModalPopup.tsx
@@ -28,6 +28,8 @@ interface ColumnSelectorModalPopupProps {
 	readonly renderer: PositronModalReactRenderer;
 	readonly columnSelectorDataGridInstance: ColumnSelectorDataGridInstance;
 	readonly anchorElement: HTMLElement;
+	readonly searchInput?: string;
+	readonly focusInput?: boolean;
 	readonly onItemHighlighted: (columnSchema: ColumnSchema) => void;
 	readonly onItemSelected: (columnSchema: ColumnSchema) => void;
 }
@@ -43,7 +45,9 @@ export const ColumnSelectorModalPopup = (props: ColumnSelectorModalPopupProps) =
 
 	// Main useEffect.
 	useEffect(() => {
+		if (props.focusInput) { return; }
 		// Drive focus into the data grid so the user can immediately navigate.
+		props.columnSelectorDataGridInstance.setCursorPosition(0, 0);
 		positronDataGridRef.current.focus();
 	}, []);
 
@@ -60,6 +64,14 @@ export const ColumnSelectorModalPopup = (props: ColumnSelectorModalPopupProps) =
 		return () => disposableStore.dispose();
 	}, [props, props.columnSelectorDataGridInstance]);
 
+	const onKeyDown = (evt: React.KeyboardEvent) => {
+		if (evt.code === 'Enter' || evt.code === 'Space') {
+			evt.preventDefault();
+			evt.stopPropagation();
+			props.columnSelectorDataGridInstance.selectItem(props.columnSelectorDataGridInstance.cursorRowIndex);
+		}
+	};
+
 	// Render.
 	return (
 		<PositronModalPopup
@@ -72,17 +84,29 @@ export const ColumnSelectorModalPopup = (props: ColumnSelectorModalPopupProps) =
 			focusableElementSelectors='input[type="text"],div[id=column-positron-data-grid]'
 			keyboardNavigationStyle='dialog'
 		>
-			<div className='column-selector'>
+			<div className='column-selector' >
 				<div className='column-selector-search'>
 					<ColumnSearch
+						initialSearchText={props.searchInput}
+						focus={props.focusInput}
 						onSearchTextChanged={async searchText => {
 							await props.columnSelectorDataGridInstance.setSearchText(
 								searchText !== '' ? searchText : undefined
 							);
 						}}
+						onNavigateOut={() => {
+							positronDataGridRef.current.focus();
+							props.columnSelectorDataGridInstance.showCursor();
+						}}
+						onConfirmSearch={() => {
+							props.columnSelectorDataGridInstance.selectItem(props.columnSelectorDataGridInstance.cursorColumnIndex);
+						}}
 					/>
 				</div>
-				<div className='column-selector-data-grid' style={{ height: 400 }}>
+				<div
+					className='column-selector-data-grid' style={{ height: 400 }}
+					onKeyDown={onKeyDown}
+				>
 					<PositronDataGrid
 						configurationService={props.configurationService}
 						layoutService={props.renderer.layoutService}

diff --git a/...ExplorerPanel/components/addEditRowFilterModalPopup/components/dropDownColumnSelector.tsx b/...ExplorerPanel/components/addEditRowFilterModalPopup/components/dropDownColumnSelector.tsx
@@ -8,7 +8,7 @@ import 'vs/css!./dropDownColumnSelector';
 
 // React.
 import * as React from 'react';
-import { useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports
+import { useCallback, useEffect, useRef, useState } from 'react'; // eslint-disable-line no-duplicate-imports
 
 // Other dependencies.
 import * as DOM from 'vs/base/browser/dom';
@@ -47,50 +47,66 @@ export const DropDownColumnSelector = (props: DropDownColumnSelectorProps) => {
 
 	// State hooks.
 	const [title, _setTitle] = useState(props.title);
+	const [selectedColumnSchema, setSelectedColumnSchema] = useState<ColumnSchema | undefined>(props.selectedColumnSchema);
 
-	// // State hooks.
-	const [selectedColumnSchema, setSelectedColumnSchema] =
-		useState<ColumnSchema | undefined>(props.selectedColumnSchema);
+	const onPressed = useCallback((focusInput?: boolean) => {
+		// Create the renderer.
+		const renderer = new PositronModalReactRenderer({
+			keybindingService: props.keybindingService,
+			layoutService: props.layoutService,
+			container: props.layoutService.getContainer(DOM.getWindow(ref.current)),
+			disableCaptures: true, // permits the usage of the enter key where applicable
+			onDisposed: () => {
+				ref.current.focus();
+			}
+		});
+
+		// Create the column selector data grid instance.
+		const columnSelectorDataGridInstance = new ColumnSelectorDataGridInstance(
+			props.dataExplorerClientInstance
+		);
+
+		// Show the drop down list box modal popup.
+		renderer.render(
+			<ColumnSelectorModalPopup
+				configurationService={props.configurationService}
+				renderer={renderer}
+				columnSelectorDataGridInstance={columnSelectorDataGridInstance}
+				anchorElement={ref.current}
+				focusInput={focusInput}
+				onItemHighlighted={columnSchema => {
+					console.log(`onItemHighlighted ${columnSchema.column_name}`);
+				}}
+				onItemSelected={columnSchema => {
+					renderer.dispose();
+					setSelectedColumnSchema(columnSchema);
+					props.onSelectedColumnSchemaChanged(columnSchema);
+				}}
+			/>
+		);
+	}, [props]);
+
+	const onKeyDown = useCallback((evt: KeyboardEvent) => {
+		// eliminate key events for anything that isn't a single-character key or whitespaces
+		if (evt.key.trim().length !== 1) { return; }
+		// don't consume event here; the input will pick it up
+		onPressed(true);
+	}, [onPressed]);
+
+	useEffect(() => {
+		const el = ref.current;
+		el.addEventListener('keydown', onKeyDown);
+		return () => {
+			el.removeEventListener('keydown', onKeyDown);
+		};
+	}, [ref, onKeyDown]);
 
 	// Render.
 	return (
 		<Button
 			ref={ref}
 			className='drop-down-column-selector'
-			onPressed={() => {
-				// Create the renderer.
-				const renderer = new PositronModalReactRenderer({
-					keybindingService: props.keybindingService,
-					layoutService: props.layoutService,
-					container: props.layoutService.getContainer(DOM.getWindow(ref.current)),
-					onDisposed: () => {
-						ref.current.focus();
-					}
-				});
-
-				// Create the column selector data grid instance.
-				const columnSelectorDataGridInstance = new ColumnSelectorDataGridInstance(
-					props.dataExplorerClientInstance
-				);
-
-				// Show the drop down list box modal popup.
-				renderer.render(
-					<ColumnSelectorModalPopup
-						configurationService={props.configurationService}
-						renderer={renderer}
-						columnSelectorDataGridInstance={columnSelectorDataGridInstance}
-						anchorElement={ref.current}
-						onItemHighlighted={columnSchema => {
-							console.log(`onItemHighlighted ${columnSchema.column_name}`);
-						}}
-						onItemSelected={columnSchema => {
-							renderer.dispose();
-							setSelectedColumnSchema(columnSchema);
-							props.onSelectedColumnSchemaChanged(columnSchema);
-						}}
-					/>
-				);
-			}}
+			onPressed={() => onPressed()}
 		>
 			{!selectedColumnSchema ?
 				(<div className='title'>{title}</div>) :

diff --git a/...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css b/...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.css
@@ -43,6 +43,10 @@
 	font-weight: 600;
 }
 
+.row-filter-widget .title .column-value {
+	font-family: var(--monaco-monospace-font), monospace;
+}
+
 .row-filter-widget .title .space-before::before {
 	content: " ";
 	white-space: pre;

diff --git a/...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx b/...lorer/components/dataExplorerPanel/components/rowFilterBar/components/rowFilterWidget.tsx
@@ -92,37 +92,37 @@ export const RowFilterWidget = forwardRef<HTMLButtonElement, RowFilterWidgetProp
 			return <>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>{props.rowFilter.operatorText}</span>
-				<span>{props.rowFilter.value}</span>
+				<span className='column-value'>{props.rowFilter.value}</span>
 			</>;
 		} else if (props.rowFilter instanceof RowFilterDescriptorSearch) {
 			return <>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>{props.rowFilter.operatorText}</span>
-				<span>{props.rowFilter.value}</span>
+				<span className='column-value'>&quot;{props.rowFilter.value}&quot;</span>
 			</>;
 		} else if (props.rowFilter instanceof RowFilterDescriptorIsBetween) {
 			return <>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>&gt;=</span>
-				<span>{props.rowFilter.lowerLimit}</span>
+				<span className='column-value'>{props.rowFilter.lowerLimit}</span>
 				<span className='space-before space-after'>
 					{localize('positron.dataExplorer.rowFilterWidget.and', "and")}
 				</span>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>&lt;=</span>
-				<span>{props.rowFilter.upperLimit}</span>
+				<span className='column-value'>{props.rowFilter.upperLimit}</span>
 			</>;
 		} else if (props.rowFilter instanceof RowFilterDescriptorIsNotBetween) {
 			return <>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>&lt;</span>
-				<span>{props.rowFilter.lowerLimit}</span>
+				<span className='column-value'>{props.rowFilter.lowerLimit}</span>
 				<span className='space-before space-after'>
 					{localize('positron.dataExplorer.rowFilterWidget.and', "and")}
 				</span>
 				<span className='column-name'>{props.rowFilter.schema.column_name}</span>
 				<span className='space-before space-after'>&gt;</span>
-				<span>{props.rowFilter.upperLimit}</span>
+				<span className='column-value'>{props.rowFilter.upperLimit}</span>
 			</>;
 		} else {
 			// This indicates a bug.

diff --git a/src/vs/workbench/browser/positronDataGrid/components/dataGridWaffle.tsx b/src/vs/workbench/browser/positronDataGrid/components/dataGridWaffle.tsx
@@ -144,8 +144,6 @@ export const DataGridWaffle = forwardRef<HTMLDivElement>((_: unknown, ref) => {
 		switch (e.code) {
 			// Space key.
 			case 'Space': {
-				// Consume the event.
-				consumeEvent();
 
 				// Make sure the cursor is showing.
 				if (context.instance.showCursor()) {
@@ -154,6 +152,9 @@ export const DataGridWaffle = forwardRef<HTMLDivElement>((_: unknown, ref) => {
 
 				// If selection is enabled, process the key.
 				if (context.instance.selection) {
+					// Consume the event only if there's an action supported for it
+					consumeEvent();
+
 					if (e.ctrlKey && !e.shiftKey) {
 						context.instance.selectColumn(context.instance.cursorColumnIndex);
 					} else if (e.shiftKey && !e.ctrlKey) {

diff --git a/src/vs/workbench/browser/positronDataGrid/positronDataGrid.tsx b/src/vs/workbench/browser/positronDataGrid/positronDataGrid.tsx
@@ -8,7 +8,7 @@ import 'vs/css!./positronDataGrid';
 
 // React.
 import * as React from 'react';
-import { forwardRef, useRef } from 'react'; // eslint-disable-line no-duplicate-imports
+import { forwardRef } from 'react'; // eslint-disable-line no-duplicate-imports
 
 // Other dependencies.
 import { DataGridWaffle } from 'vs/workbench/browser/positronDataGrid/components/dataGridWaffle';
@@ -27,14 +27,12 @@ interface PositronDataGridProps extends PositronDataGridConfiguration {
  * @returns The rendered component.
  */
 export const PositronDataGrid = forwardRef<HTMLDivElement, PositronDataGridProps>((props, ref) => {
-	// Reference hooks.
-	const dataGridWaffleRef = useRef<HTMLDivElement>(undefined!);
 
 	// Render.
 	return (
 		<PositronDataGridContextProvider {...props}>
-			<div ref={ref} id={props.id} className='data-grid'>
-				<DataGridWaffle ref={dataGridWaffleRef} />
+			<div id={props.id} className='data-grid'>
+				<DataGridWaffle ref={ref} />
 			</div>
 		</PositronDataGridContextProvider>
 	);

diff --git a/src/vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer.tsx b/src/vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer.tsx
@@ -51,6 +51,7 @@ export interface PositronModalReactRendererOptions {
 	readonly container: HTMLElement;
 	readonly parent?: HTMLElement;
 	readonly onDisposed?: () => void;
+	readonly disableCaptures?: boolean;
 }
 
 /**
@@ -321,14 +322,14 @@ export class PositronModalReactRenderer extends Disposable {
 		};
 
 		// Add global keydown, mousedown, and resize event listeners.
-		window.addEventListener(KEYDOWN, keydownHandler, true);
+		window.addEventListener(KEYDOWN, keydownHandler, renderer._options.disableCaptures ? false : true);
 		window.addEventListener(MOUSEDOWN, mousedownHandler, true);
 		window.addEventListener(RESIZE, resizeHandler, false);
 
 		// Return the cleanup function that removes our event listeners.
 		PositronModalReactRenderer._unbindCallback = () => {
 			// Remove keydown, mousedown, and resize event listeners.
-			window.removeEventListener(KEYDOWN, keydownHandler, true);
+			window.removeEventListener(KEYDOWN, keydownHandler, renderer._options.disableCaptures ? false : true);
 			window.removeEventListener(MOUSEDOWN, mousedownHandler, true);
 			window.removeEventListener(RESIZE, resizeHandler, false);
 		};