From bb499b6b05a880cf8616d7161741e550aefcbe66 Mon Sep 17 00:00:00 2001 From: heswell Date: Mon, 23 Oct 2023 10:03:13 +0100 Subject: [PATCH] refactor Table navigation, preparing for row highlighting --- .../vuu-table/src/table-next/TableNext.tsx | 2 + .../src/table-next/useKeyboardNavigation.ts | 31 +++++-- .../vuu-table/src/table-next/useTableNext.ts | 3 + .../vuu-table/src/table/dataTableTypes.ts | 7 ++ .../src/common-hooks/navigationTypes.ts | 6 +- .../vuu-ui-controls/src/list/List.css | 5 +- .../vuu-ui-controls/src/list/List.tsx | 2 +- .../common-hooks/useKeyboardNavigation.ts | 83 +++++++++---------- .../vuu-ui-controls/src/list/useList.ts | 3 +- .../src/examples/Table/TableNext.examples.tsx | 30 +++++++ .../src/examples/UiControls/List.examples.tsx | 55 +++++++++++- 11 files changed, 163 insertions(+), 64 deletions(-) diff --git a/vuu-ui/packages/vuu-table/src/table-next/TableNext.tsx b/vuu-ui/packages/vuu-table/src/table-next/TableNext.tsx index b40f2cf57..f585ad55c 100644 --- a/vuu-ui/packages/vuu-table/src/table-next/TableNext.tsx +++ b/vuu-ui/packages/vuu-table/src/table-next/TableNext.tsx @@ -26,6 +26,7 @@ export const TableNext = forwardRef(function TableNext( config, dataSource, id: idProp, + navigationStyle = "cell", onAvailableColumnsChange, onConfigChange, onFeatureEnabled, @@ -69,6 +70,7 @@ export const TableNext = forwardRef(function TableNext( containerRef, dataSource, headerHeight, + navigationStyle, onAvailableColumnsChange, onConfigChange, onFeatureEnabled, diff --git a/vuu-ui/packages/vuu-table/src/table-next/useKeyboardNavigation.ts b/vuu-ui/packages/vuu-table/src/table-next/useKeyboardNavigation.ts index 73e750dae..e1b4ddf24 100644 --- a/vuu-ui/packages/vuu-table/src/table-next/useKeyboardNavigation.ts +++ b/vuu-ui/packages/vuu-table/src/table-next/useKeyboardNavigation.ts @@ -15,20 +15,33 @@ import { getTableCell, headerCellQuery, } from "./table-dom-utils"; +import { TableNavigationStyle } from "../table/dataTableTypes"; -const navigationKeys = new Set([ +const rowNavigationKeys = new Set([ "Home", "End", "PageUp", "PageDown", "ArrowDown", - "ArrowLeft", - "ArrowRight", "ArrowUp", ]); -export const isNavigationKey = (key: string): key is NavigationKey => { - return navigationKeys.has(key as NavigationKey); +const cellNavigationKeys = new Set(rowNavigationKeys); +cellNavigationKeys.add("ArrowLeft"); +cellNavigationKeys.add("ArrowRight"); + +export const isNavigationKey = ( + key: string, + navigationStyle: TableNavigationStyle +): key is NavigationKey => { + switch (navigationStyle) { + case "cell": + return cellNavigationKeys.has(key as NavigationKey); + case "row": + return rowNavigationKeys.has(key as NavigationKey); + default: + return false; + } }; type ArrowKey = "ArrowUp" | "ArrowDown" | "ArrowLeft" | "ArrowRight"; @@ -114,6 +127,7 @@ export interface NavigationHookProps { columnCount?: number; disableHighlightOnFocus?: boolean; label?: string; + navigationStyle: TableNavigationStyle; viewportRange: VuuRange; requestScroll?: ScrollRequestHandler; restoreLastFocus?: boolean; @@ -126,6 +140,7 @@ export const useKeyboardNavigation = ({ columnCount = 0, containerRef, disableHighlightOnFocus, + navigationStyle, requestScroll, rowCount = 0, viewportRowCount, @@ -233,7 +248,6 @@ NavigationHookProps) => { // click handler. const focusedCell = getFocusedCell(document.activeElement); if (focusedCell) { - console.log({ focusedCell }); focusedCellPos.current = getTableCellPos(focusedCell); } } @@ -242,7 +256,6 @@ NavigationHookProps) => { const navigateChildItems = useCallback( async (key: NavigationKey) => { - console.log(`navigate child items ${key}`); const [nextRowIdx, nextColIdx] = isPagingKey(key) ? await nextPageItemIdx(key, activeCellPos.current) : nextCellPos(key, activeCellPos.current, columnCount, rowCount); @@ -258,13 +271,13 @@ NavigationHookProps) => { const handleKeyDown = useCallback( (e: KeyboardEvent) => { - if (rowCount > 0 && isNavigationKey(e.key)) { + if (rowCount > 0 && isNavigationKey(e.key, navigationStyle)) { e.preventDefault(); e.stopPropagation(); void navigateChildItems(e.key); } }, - [rowCount, navigateChildItems] + [rowCount, navigationStyle, navigateChildItems] ); const handleClick = useCallback( diff --git a/vuu-ui/packages/vuu-table/src/table-next/useTableNext.ts b/vuu-ui/packages/vuu-table/src/table-next/useTableNext.ts index d97265299..3400fca92 100644 --- a/vuu-ui/packages/vuu-table/src/table-next/useTableNext.ts +++ b/vuu-ui/packages/vuu-table/src/table-next/useTableNext.ts @@ -67,6 +67,7 @@ export interface TableHookProps | "availableColumns" | "config" | "dataSource" + | "navigationStyle" | "onAvailableColumnsChange" | "onConfigChange" | "onFeatureEnabled" @@ -98,6 +99,7 @@ export const useTable = ({ containerRef, dataSource, headerHeight = 25, + navigationStyle, onAvailableColumnsChange, onConfigChange, onFeatureEnabled, @@ -408,6 +410,7 @@ export const useTable = ({ } = useKeyboardNavigation({ columnCount: columns.filter((c) => c.hidden !== true).length, containerRef, + navigationStyle, requestScroll, rowCount: dataSource?.size, viewportRange: range, diff --git a/vuu-ui/packages/vuu-table/src/table/dataTableTypes.ts b/vuu-ui/packages/vuu-table/src/table/dataTableTypes.ts index 2db9f107e..a196e163f 100644 --- a/vuu-ui/packages/vuu-table/src/table/dataTableTypes.ts +++ b/vuu-ui/packages/vuu-table/src/table/dataTableTypes.ts @@ -20,6 +20,8 @@ export type TableRowClickHandler = (row: VuuDataRow) => void; // TODO implement a Model object to represent a row data for better API export type TableRowSelectHandler = (row: DataSourceRow) => void; +export type TableNavigationStyle = "none" | "cell" | "row"; + export interface TableProps extends Omit, "onSelect"> { Row?: FC; @@ -32,6 +34,11 @@ export interface TableProps dataSource: DataSource; headerHeight?: number; height?: number; + /** + * Defined how focus navigation within data cells will be handled by table. + * Default is cell. + */ + navigationStyle?: TableNavigationStyle; /** * required if a fully featured column picker is to be available. * Available columns can be changed by the addition or removal of diff --git a/vuu-ui/packages/vuu-ui-controls/src/common-hooks/navigationTypes.ts b/vuu-ui/packages/vuu-ui-controls/src/common-hooks/navigationTypes.ts index c6b041c06..7a6349a1a 100644 --- a/vuu-ui/packages/vuu-ui-controls/src/common-hooks/navigationTypes.ts +++ b/vuu-ui/packages/vuu-ui-controls/src/common-hooks/navigationTypes.ts @@ -1,14 +1,12 @@ import { FocusEvent, KeyboardEvent, RefObject } from "react"; -import { CollectionItem } from "./collectionTypes"; -export interface NavigationProps { +export interface NavigationProps { cycleFocus?: boolean; defaultHighlightedIndex?: number; disableHighlightOnFocus?: boolean; focusOnHighlight?: boolean; focusVisible?: number; highlightedIndex?: number; - indexPositions: CollectionItem[]; itemCount: number; onHighlight?: (idx: number) => void; onKeyboardNavigation?: (evt: KeyboardEvent, idx: number) => void; @@ -16,7 +14,7 @@ export interface NavigationProps { viewportItemCount: number; } -export interface NavigationHookProps extends NavigationProps { +export interface NavigationHookProps extends NavigationProps { containerRef: RefObject; label?: string; selected?: string[]; diff --git a/vuu-ui/packages/vuu-ui-controls/src/list/List.css b/vuu-ui/packages/vuu-ui-controls/src/list/List.css index 9b9c87477..9a4e0b348 100644 --- a/vuu-ui/packages/vuu-ui-controls/src/list/List.css +++ b/vuu-ui/packages/vuu-ui-controls/src/list/List.css @@ -36,12 +36,13 @@ overflow: auto; } -.vuuListItemHeader { +.vuuListHeader { --saltList-item-background: var(--list-item-header-background); color: var(--list-item-header-color); + font-weight: 600; } -.vuuListItemHeader[data-sticky="true"] { +.vuuListHeader[data-sticky="true"] { --saltList-item-background: var(--list-background); position: sticky; top: 0; diff --git a/vuu-ui/packages/vuu-ui-controls/src/list/List.tsx b/vuu-ui/packages/vuu-ui-controls/src/list/List.tsx index c1e7904f1..8e03994ce 100644 --- a/vuu-ui/packages/vuu-ui-controls/src/list/List.tsx +++ b/vuu-ui/packages/vuu-ui-controls/src/list/List.tsx @@ -203,12 +203,12 @@ export const List = forwardRef(function List< focusVisible: collapsibleHeaders && appliedFocusVisible === idx.value, })} aria-expanded={expanded} - data-idx={collapsibleHeaders ? idx.value : undefined} data-index={collapsibleHeaders ? idx.value : undefined} data-highlighted={idx.value === highlightedIndex || undefined} data-sticky={stickyHeaders} data-selectable={false} id={headerId} + itemHeight={getItemHeight(idx.value)} key={`header-${idx.value}`} label={title} // role="presentation" diff --git a/vuu-ui/packages/vuu-ui-controls/src/list/common-hooks/useKeyboardNavigation.ts b/vuu-ui/packages/vuu-ui-controls/src/list/common-hooks/useKeyboardNavigation.ts index 6474b99c6..17ac6213e 100644 --- a/vuu-ui/packages/vuu-ui-controls/src/list/common-hooks/useKeyboardNavigation.ts +++ b/vuu-ui/packages/vuu-ui-controls/src/list/common-hooks/useKeyboardNavigation.ts @@ -18,13 +18,12 @@ import { PageUp, } from "./keyUtils"; import { - CollectionItem, NavigationHookProps, NavigationHookResult, getFirstSelectedItem, hasSelection, } from "../../common-hooks"; -import { getElementByDataIndex } from "@finos/vuu-utils"; +import { getElementByDataIndex, isValidNumber } from "@finos/vuu-utils"; export const LIST_FOCUS_VISIBLE = -2; @@ -46,18 +45,20 @@ function nextItemIdx(count: number, key: string, idx: number) { } } -const getIndexOfSelectedItem = ( - items: CollectionItem[], - selected?: string[] -) => { +const getIndexOfSelectedItem = (selected?: string[]) => { const selectedItemId = Array.isArray(selected) ? getFirstSelectedItem(selected) : undefined; if (selectedItemId) { - return items.findIndex((item) => item.id === selectedItemId); - } else { - return -1; + const el = document.getElementById(selectedItemId) as HTMLElement; + if (el) { + const index = parseInt(el.dataset.index ?? "-1"); + if (isValidNumber(index)) { + return index; + } + } } + return -1; }; const getStartIdx = ( @@ -137,24 +138,29 @@ const pageUp = async ( } }; -const isLeaf = (item: CollectionItem): boolean => - !item.header && !item.childNodes; -const isFocusable = (item: CollectionItem) => - isLeaf(item) || item.expanded !== undefined; +// const isLeaf = (item: CollectionItem): boolean => +// !item.header && !item.childNodes; +const isLeaf = (element?: HTMLElement) => element !== undefined; +// const isFocusable = (item: CollectionItem) => +// isLeaf(item) || item.expanded !== undefined; +// TODO read dom element and check for leaf item or toggleable group +const isFocusable = (container: HTMLElement, index: number) => { + const targetEl = getElementByDataIndex(container, index); + return isLeaf(targetEl); +}; -export const useKeyboardNavigation = ({ +export const useKeyboardNavigation = ({ containerRef, defaultHighlightedIndex = -1, disableHighlightOnFocus, highlightedIndex: highlightedIndexProp, - indexPositions, itemCount, onHighlight, onKeyboardNavigation, restoreLastFocus, selected, viewportItemCount, -}: NavigationHookProps): NavigationHookResult => { +}: NavigationHookProps): NavigationHookResult => { const lastFocus = useRef(-1); const [, forceRender] = useState({}); const [highlightedIndex, setHighlightedIdx, isControlledHighlighting] = @@ -198,41 +204,43 @@ export const useKeyboardNavigation = ({ const nextFocusableItemIdx = useCallback( (key = ArrowDown, idx: number = key === ArrowDown ? -1 : itemCount) => { + //TODO we don't seem to have selectedhere first time after selection if (itemCount === 0) { return -1; } else { - const indexOfSelectedItem = getIndexOfSelectedItem( - indexPositions, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - selected - ); + const isEnd = key === "End"; + const isHome = key === "Home"; // The start index is generally the highlightedIdx (passed in as idx). // We don't need it for Home and End navigation. // Special case where we have selection, but no highlighting - begin // navigation from selected item. + const indexOfSelectedItem = + isEnd || isHome || idx === -1 ? -1 : getIndexOfSelectedItem(selected); const startIdx = getStartIdx(key, idx, indexOfSelectedItem, itemCount); - let nextIdx = nextItemIdx(itemCount, key, startIdx); + + const { current: container } = containerRef; // Guard against returning zero, when first item is a header or group if ( nextIdx === 0 && key === ArrowUp && - !isFocusable(indexPositions[0]) + container && + !isFocusable(container, 0) ) { return idx; } while ( - (((key === ArrowDown || key === Home) && nextIdx < itemCount) || - ((key === ArrowUp || key === End) && nextIdx > 0)) && - !isFocusable(indexPositions[nextIdx]) + (((key === ArrowDown || isHome) && nextIdx < itemCount) || + ((key === ArrowUp || isEnd) && nextIdx > 0)) && + container && + !isFocusable(container, nextIdx) ) { nextIdx = nextItemIdx(itemCount, key, nextIdx); } return nextIdx; } }, - [indexPositions, itemCount, selected] + [containerRef, itemCount, selected] ); // does this belong here or should it be a method passed in? @@ -247,7 +255,7 @@ export const useKeyboardNavigation = ({ } else { // If mouse wan't used, then keyboard must have been keyboardNavigation.current = true; - if (indexPositions.length === 0) { + if (itemCount === 0) { setHighlightedIndex(LIST_FOCUS_VISIBLE); } else if (highlightedIndex !== -1) { // We need to force a render here. We're not changing the highlightedIdx, but we want to @@ -258,12 +266,7 @@ export const useKeyboardNavigation = ({ if (lastFocus.current !== -1) { setHighlightedIndex(lastFocus.current); } else { - const selectedItemIdx = getIndexOfSelectedItem( - indexPositions, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - selected - ); + const selectedItemIdx = getIndexOfSelectedItem(selected); if (selectedItemIdx !== -1) { setHighlightedIndex(selectedItemIdx); } else { @@ -271,12 +274,7 @@ export const useKeyboardNavigation = ({ } } } else if (hasSelection(selected)) { - const selectedItemIdx = getIndexOfSelectedItem( - indexPositions, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - selected - ); + const selectedItemIdx = getIndexOfSelectedItem(selected); setHighlightedIndex(selectedItemIdx); } else if (disableHighlightOnFocus !== true) { setHighlightedIndex(nextFocusableItemIdx()); @@ -285,7 +283,7 @@ export const useKeyboardNavigation = ({ }, [ disableHighlightOnFocus, highlightedIndex, - indexPositions, + itemCount, nextFocusableItemIdx, restoreLastFocus, selected, @@ -318,7 +316,6 @@ export const useKeyboardNavigation = ({ const handleKeyDown = useCallback( (e: KeyboardEvent) => { - console.log("handleKeyDown"); if (itemCount > 0 && isNavigationKey(e)) { e.preventDefault(); e.stopPropagation(); diff --git a/vuu-ui/packages/vuu-ui-controls/src/list/useList.ts b/vuu-ui/packages/vuu-ui-controls/src/list/useList.ts index e4f7742fe..096153c8d 100644 --- a/vuu-ui/packages/vuu-ui-controls/src/list/useList.ts +++ b/vuu-ui/packages/vuu-ui-controls/src/list/useList.ts @@ -132,12 +132,11 @@ export const useList = ({ }, setHighlightedIndex, ...keyboardHook - } = useKeyboardNavigation({ + } = useKeyboardNavigation({ containerRef: scrollContainer, defaultHighlightedIndex, disableHighlightOnFocus, highlightedIndex: highlightedIndexProp, - indexPositions: dataHook.data, itemCount: dataHook.data.length, label, onHighlight, diff --git a/vuu-ui/showcase/src/examples/Table/TableNext.examples.tsx b/vuu-ui/showcase/src/examples/Table/TableNext.examples.tsx index b9f727918..83bbbe215 100644 --- a/vuu-ui/showcase/src/examples/Table/TableNext.examples.tsx +++ b/vuu-ui/showcase/src/examples/Table/TableNext.examples.tsx @@ -51,6 +51,36 @@ export const DefaultTableNextArrayData = () => { }; DefaultTableNextArrayData.displaySequence = displaySequence++; +export const NavigationStyle = () => { + const { + typeaheadHook: _, + config: configProp, + ...props + } = useTableConfig({ + rangeChangeRowset: "full", + table: { module: "SIMUL", table: "instruments" }, + }); + + const [config, setConfig] = useState(configProp); + + const handleConfigChange = useCallback((config: TableConfig) => { + setConfig(config); + }, []); + + return ( + + ); +}; +NavigationStyle.displaySequence = displaySequence++; + export const EditableTableNextArrayData = () => { const { config, dataSource } = useTableConfig({ columnConfig: { diff --git a/vuu-ui/showcase/src/examples/UiControls/List.examples.tsx b/vuu-ui/showcase/src/examples/UiControls/List.examples.tsx index 69b69c7dc..27ea54be0 100644 --- a/vuu-ui/showcase/src/examples/UiControls/List.examples.tsx +++ b/vuu-ui/showcase/src/examples/UiControls/List.examples.tsx @@ -20,17 +20,17 @@ import { useRef, useState, } from "react"; -import { usa_states } from "./List.data"; +import { usa_states, usa_states_cities } from "./List.data"; let displaySequence = 1; export const DefaultList = () => { const handleSelect = useCallback((evt, selected) => { - console.log(`handleSelect`, { selected }); + console.log(`handleSelect ${selected}`); }, []); const handleSelectionChange = useCallback( (evt, selected) => { - console.log(`handleSelectionChange`, { selected }); + console.log(`handleSelectionChange ${selected}`); }, [] ); @@ -419,3 +419,52 @@ export const DefaultSelectedWithinViewport = () => { ); }; DefaultSelectedWithinViewport.displaySequence = displaySequence++; + +export const GroupedList = () => { + const handleSelect = useCallback((evt, selected) => { + console.log(`handleSelect ${selected}`); + }, []); + const handleSelectionChange = useCallback( + (evt, selected) => { + console.log(`handleSelectionChange ${selected}`); + }, + [] + ); + + return ( + + ); +}; +GroupedList.displaySequence = displaySequence++; + +export const GroupedListCollapsibleHeaders = () => { + const handleSelect = useCallback((evt, selected) => { + console.log(`handleSelect ${selected}`); + }, []); + const handleSelectionChange = useCallback( + (evt, selected) => { + console.log(`handleSelectionChange ${selected}`); + }, + [] + ); + + return ( + + ); +}; +GroupedListCollapsibleHeaders.displaySequence = displaySequence++;