From 51a5d7cd4f8bc795d1840b4af95ec017945edb9f Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Tue, 15 Nov 2022 17:48:16 +0100 Subject: [PATCH] Retain list item focus while scrolling Moves focus away from unmounted list items to the grid such that keyboard navigation still works after scrolling Co-Authored-By: Mark Hicken <849930+markhicken@users.noreply.github.com> --- app/src/ui/lib/list/list-row.tsx | 30 ++++++++++++++- app/src/ui/lib/list/list.tsx | 64 +++++++++++++++++++++++++------- 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/app/src/ui/lib/list/list-row.tsx b/app/src/ui/lib/list/list-row.tsx index dc0c762b56..d1e219913b 100644 --- a/app/src/ui/lib/list/list-row.tsx +++ b/app/src/ui/lib/list/list-row.tsx @@ -24,7 +24,7 @@ interface IListRowProps { readonly selected?: boolean /** callback to fire when the DOM element is created */ - readonly onRef?: (element: HTMLDivElement | null) => void + readonly onRowRef?: (index: number, element: HTMLDivElement | null) => void /** callback to fire when the row receives a mouseover event */ readonly onRowMouseOver: (index: number, e: React.MouseEvent) => void @@ -41,6 +41,18 @@ interface IListRowProps { /** callback to fire when the row receives a keyboard event */ readonly onRowKeyDown: (index: number, e: React.KeyboardEvent) => void + /** called when the row (or any of its descendants) receives focus */ + readonly onRowFocus?: ( + index: number, + e: React.FocusEvent + ) => void + + /** called when the row (and all of its descendants) loses focus */ + readonly onRowBlur?: ( + index: number, + e: React.FocusEvent + ) => void + /** * Whether or not this list row is going to be selectable either through * keyboard navigation, pointer clicks, or both. This is used to determine @@ -53,6 +65,10 @@ interface IListRowProps { } export class ListRow extends React.Component { + private onRef = (elem: HTMLDivElement | null) => { + this.props.onRowRef?.(this.props.rowIndex, elem) + } + private onRowMouseOver = (e: React.MouseEvent) => { this.props.onRowMouseOver(this.props.rowIndex, e) } @@ -73,6 +89,14 @@ export class ListRow extends React.Component { this.props.onRowKeyDown(this.props.rowIndex, e) } + private onFocus = (e: React.FocusEvent) => { + this.props.onRowFocus?.(this.props.rowIndex, e) + } + + private onBlur = (e: React.FocusEvent) => { + this.props.onRowBlur?.(this.props.rowIndex, e) + } + public render() { const selected = this.props.selected const className = classNames( @@ -102,13 +126,15 @@ export class ListRow extends React.Component { role={role} className={className} tabIndex={this.props.tabIndex} - ref={this.props.onRef} + ref={this.onRef} onMouseOver={this.onRowMouseOver} onMouseDown={this.onRowMouseDown} onMouseUp={this.onRowMouseUp} onClick={this.onRowClick} onKeyDown={this.onRowKeyDown} style={style} + onFocus={this.onFocus} + onBlur={this.onBlur} > {this.props.children} diff --git a/app/src/ui/lib/list/list.tsx b/app/src/ui/lib/list/list.tsx index c3aa3bdba2..bed01393b2 100644 --- a/app/src/ui/lib/list/list.tsx +++ b/app/src/ui/lib/list/list.tsx @@ -269,6 +269,8 @@ export class List extends React.Component { private fakeScroll: HTMLDivElement | null = null private focusRow = -1 + private readonly rowRefs = new Map() + /** * The style prop for our child Grid. We keep this here in order * to not create a new object on each render and thus forcing @@ -567,6 +569,15 @@ export class List extends React.Component { } } + private onFocusWithinChanged = (focusWithin: boolean) => { + // So the grid lost focus (we manually focus the grid if the focused list + // item is unmounted) so we mustn't attempt to refocus the previously + // focused list item if it scrolls back into view. + if (!focusWithin) { + this.focusRow = -1 + } + } + private toggleSelection = (event: React.KeyboardEvent) => { this.props.selectedRows.forEach(row => { if (!this.props.onRowClick) { @@ -586,6 +597,16 @@ export class List extends React.Component { }) } + private onRowFocus = (index: number, e: React.FocusEvent) => { + this.focusRow = index + } + + private onRowBlur = (index: number, e: React.FocusEvent) => { + if (this.focusRow === index) { + this.focusRow = -1 + } + } + private onRowMouseOver = (row: number, event: React.MouseEvent) => { if (this.props.selectOnHover && this.canSelectRow(row)) { if (!this.props.selectedRows.includes(row)) { @@ -595,7 +616,7 @@ export class List extends React.Component { // more importantly `scrollRowToVisible` automatically manages focus so // using it here allows us to piggy-back on its focus-preserving magic // even though we could theoretically live without scrolling - this.scrollRowToVisible(row) + this.scrollRowToVisible(row, this.props.focusOnHover !== false) } } } @@ -717,10 +738,14 @@ export class List extends React.Component { this.scrollRowToVisible(row) } - private scrollRowToVisible(row: number) { + private scrollRowToVisible(row: number, moveFocus = true) { if (this.grid !== null) { this.grid.scrollToCell({ rowIndex: row, columnIndex: 0 }) - this.focusRow = row + + if (moveFocus) { + this.focusRow = row + this.rowRefs.get(row)?.focus({ preventScroll: true }) + } } } @@ -801,12 +826,27 @@ export class List extends React.Component { } } - private onFocusedItemRef = (element: HTMLDivElement | null) => { - if (this.props.focusOnHover !== false && element !== null) { - element.focus() + private onRowRef = (rowIndex: number, element: HTMLDivElement | null) => { + if (element === null) { + this.rowRefs.delete(rowIndex) + } else { + this.rowRefs.set(rowIndex, element) } - this.focusRow = -1 + if (rowIndex === this.focusRow) { + // The currently focused row is going being unmounted so we'll move focus + // programmatically to the grid so that keyboard navigation still works + if (element === null) { + const grid = ReactDOM.findDOMNode(this.grid) + if (grid instanceof HTMLElement) { + grid.focus({ preventScroll: true }) + } + } else { + // A previously focused row is being mounted again, we'll move focus + // back to it + element.focus({ preventScroll: true }) + } + } } private getCustomRowClassNames = (rowIndex: number) => { @@ -833,17 +873,12 @@ export class List extends React.Component { const selected = this.props.selectedRows.indexOf(rowIndex) !== -1 const customClasses = this.getCustomRowClassNames(rowIndex) - const focused = rowIndex === this.focusRow - // An unselectable row shouldn't be focusable let tabIndex: number | undefined = undefined if (selectable) { tabIndex = selected && this.props.selectedRows[0] === rowIndex ? 0 : -1 } - // We only need to keep a reference to the focused element - const ref = focused ? this.onFocusedItemRef : undefined - const row = this.props.rowRenderer(rowIndex) const element = @@ -867,7 +902,7 @@ export class List extends React.Component { { onRowMouseDown={this.onRowMouseDown} onRowMouseUp={this.onRowMouseUp} onRowMouseOver={this.onRowMouseOver} + onRowFocus={this.onRowFocus} + onRowBlur={this.onRowBlur} style={params.style} tabIndex={tabIndex} children={element} @@ -975,6 +1012,7 @@ export class List extends React.Component {