From b2251989a5a18304e55708b2514eea1cb477b12c Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Tue, 5 Sep 2023 15:53:50 +0200 Subject: [PATCH 1/5] Revert "Revert "Merge pull request #17225 from desktop/jc-live-container-chill"" This reverts commit a58b635bd8f80605c59ce6801ee32697b60fd1d5. --- app/src/ui/lib/filter-list.tsx | 67 ++++++++++++++++++------- app/src/ui/lib/section-filter-list.tsx | 69 +++++++++++++++++++------- 2 files changed, 98 insertions(+), 38 deletions(-) diff --git a/app/src/ui/lib/filter-list.tsx b/app/src/ui/lib/filter-list.tsx index 19ed4cbead..76917eb488 100644 --- a/app/src/ui/lib/filter-list.tsx +++ b/app/src/ui/lib/filter-list.tsx @@ -173,6 +173,7 @@ interface IFilterListState { readonly rows: ReadonlyArray> readonly selectedRow: number readonly filterValue: string + readonly filterValueChanged: boolean } /** @@ -193,23 +194,31 @@ export class FilterList extends React.Component< IFilterListProps, IFilterListState > { + public static getDerivedStateFromProps( + props: IFilterListProps, + state: IFilterListState + ) { + return createStateUpdate(props, state) + } + private list: List | null = null private filterTextBox: TextBox | null = null public constructor(props: IFilterListProps) { super(props) - this.state = createStateUpdate(props) - } - - public componentWillMount() { - if (this.props.filterTextBox !== undefined) { - this.filterTextBox = this.props.filterTextBox + if (props.filterTextBox !== undefined) { + this.filterTextBox = props.filterTextBox } - } - public componentWillReceiveProps(nextProps: IFilterListProps) { - this.setState(createStateUpdate(nextProps)) + const filterValue = (props.filterText || '').toLowerCase() + + this.state = { + rows: new Array>(), + selectedRow: -1, + filterValue, + filterValueChanged: filterValue.length > 0, + } } public componentDidUpdate( @@ -276,6 +285,23 @@ export class FilterList extends React.Component< ) } + public renderLiveContainer() { + if (!this.state.filterValueChanged) { + return null + } + + const itemRows = this.state.rows.filter(row => row.kind === 'item') + const resultsPluralized = itemRows.length === 1 ? 'result' : 'results' + const screenReaderMessage = `${itemRows.length} ${resultsPluralized}` + + return ( + + ) + } + public renderFilterRow() { if (this.props.hideFilterRow === true) { return null @@ -290,16 +316,10 @@ export class FilterList extends React.Component< } public render() { - const itemRows = this.state.rows.filter(row => row.kind === 'item') - const resultsPluralized = itemRows.length === 1 ? 'result' : 'results' - const screenReaderMessage = `${itemRows.length} ${resultsPluralized}` - return (
- + {this.renderLiveContainer()} + {this.props.renderPreList ? this.props.renderPreList() : null} {this.renderFilterRow()} @@ -577,7 +597,8 @@ export function getText( } function createStateUpdate( - props: IFilterListProps + props: IFilterListProps, + state: IFilterListState ) { const flattenedRows = new Array>() const filter = (props.filterText || '').toLowerCase() @@ -618,7 +639,15 @@ function createStateUpdate( selectedRow = flattenedRows.findIndex(i => i.kind === 'item') } - return { rows: flattenedRows, selectedRow, filterValue: filter } + // Stay true if already set, otherwise become true if the filter has content + const filterValueChanged = state.filterValueChanged ? true : filter.length > 0 + + return { + rows: flattenedRows, + selectedRow, + filterValue: filter, + filterValueChanged, + } } function getItemFromRowIndex( diff --git a/app/src/ui/lib/section-filter-list.tsx b/app/src/ui/lib/section-filter-list.tsx index f6b3401dec..e064ff9f49 100644 --- a/app/src/ui/lib/section-filter-list.tsx +++ b/app/src/ui/lib/section-filter-list.tsx @@ -166,6 +166,7 @@ interface IFilterListState { readonly rows: ReadonlyArray>> readonly selectedRow: RowIndexPath readonly filterValue: string + readonly filterValueChanged: boolean // Indices of groups in the filtered list readonly groups: ReadonlyArray } @@ -174,23 +175,32 @@ interface IFilterListState { export class SectionFilterList< T extends IFilterListItem > extends React.Component, IFilterListState> { + public static getDerivedStateFromProps( + props: ISectionFilterListProps, + state: IFilterListState + ) { + return createStateUpdate(props, state) + } + private list: SectionList | null = null private filterTextBox: TextBox | null = null public constructor(props: ISectionFilterListProps) { super(props) - this.state = createStateUpdate(props) - } - - public componentWillMount() { - if (this.props.filterTextBox !== undefined) { - this.filterTextBox = this.props.filterTextBox + if (props.filterTextBox !== undefined) { + this.filterTextBox = props.filterTextBox } - } - public componentWillReceiveProps(nextProps: ISectionFilterListProps) { - this.setState(createStateUpdate(nextProps)) + const filterValue = (props.filterText || '').toLowerCase() + + this.state = { + rows: new Array>>(), + selectedRow: InvalidRowIndexPath, + filterValue, + filterValueChanged: filterValue.length > 0, + groups: [], + } } public componentDidUpdate( @@ -257,6 +267,23 @@ export class SectionFilterList< ) } + public renderLiveContainer() { + if (!this.state.filterValueChanged) { + return null + } + + const itemRows = this.state.rows.flat().filter(row => row.kind === 'item') + const resultsPluralized = itemRows.length === 1 ? 'result' : 'results' + const screenReaderMessage = `${itemRows.length} ${resultsPluralized}` + + return ( + + ) + } + public renderFilterRow() { if (this.props.hideFilterRow === true) { return null @@ -271,16 +298,10 @@ export class SectionFilterList< } public render() { - const itemRows = this.state.rows.flat().filter(row => row.kind === 'item') - const resultsPluralized = itemRows.length === 1 ? 'result' : 'results' - const screenReaderMessage = `${itemRows.length} ${resultsPluralized}` - return (
- + {this.renderLiveContainer()} + {this.props.renderPreList ? this.props.renderPreList() : null} {this.renderFilterRow()} @@ -614,7 +635,8 @@ function getFirstVisibleRow( } function createStateUpdate( - props: ISectionFilterListProps + props: ISectionFilterListProps, + state: IFilterListState ) { const rows = new Array>>() const filter = (props.filterText || '').toLowerCase() @@ -664,7 +686,16 @@ function createStateUpdate( selectedRow = getFirstVisibleRow(rows) } - return { rows: rows, selectedRow, filterValue: filter, groups: groupIndices } + // Stay true if already set, otherwise become true if the filter has content + const filterValueChanged = state.filterValueChanged ? true : filter.length > 0 + + return { + rows: rows, + selectedRow, + filterValue: filter, + filterValueChanged, + groups: groupIndices, + } } function getItemFromRowIndex( From e7607116b1898073596afd80e7c8adbeab4f5c33 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 30 Aug 2023 23:25:03 +0200 Subject: [PATCH 2/5] Fix keyboard navigation of section filter list --- app/src/ui/lib/section-filter-list.tsx | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/app/src/ui/lib/section-filter-list.tsx b/app/src/ui/lib/section-filter-list.tsx index e064ff9f49..bb49ed95dc 100644 --- a/app/src/ui/lib/section-filter-list.tsx +++ b/app/src/ui/lib/section-filter-list.tsx @@ -175,13 +175,6 @@ interface IFilterListState { export class SectionFilterList< T extends IFilterListItem > extends React.Component, IFilterListState> { - public static getDerivedStateFromProps( - props: ISectionFilterListProps, - state: IFilterListState - ) { - return createStateUpdate(props, state) - } - private list: SectionList | null = null private filterTextBox: TextBox | null = null @@ -192,15 +185,11 @@ export class SectionFilterList< this.filterTextBox = props.filterTextBox } - const filterValue = (props.filterText || '').toLowerCase() + this.state = createStateUpdate(props, null) + } - this.state = { - rows: new Array>>(), - selectedRow: InvalidRowIndexPath, - filterValue, - filterValueChanged: filterValue.length > 0, - groups: [], - } + public componentWillReceiveProps(nextProps: ISectionFilterListProps) { + this.setState(createStateUpdate(nextProps, this.state)) } public componentDidUpdate( @@ -636,7 +625,7 @@ function getFirstVisibleRow( function createStateUpdate( props: ISectionFilterListProps, - state: IFilterListState + state: IFilterListState | null ) { const rows = new Array>>() const filter = (props.filterText || '').toLowerCase() @@ -687,7 +676,9 @@ function createStateUpdate( } // Stay true if already set, otherwise become true if the filter has content - const filterValueChanged = state.filterValueChanged ? true : filter.length > 0 + const filterValueChanged = state?.filterValueChanged + ? true + : filter.length > 0 return { rows: rows, From cfb53226b4ae2afaa438ba3b69de472b8ba3606c Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 30 Aug 2023 23:31:28 +0200 Subject: [PATCH 3/5] Fix keyboard navigation of flat filter list --- app/src/ui/lib/filter-list.tsx | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/app/src/ui/lib/filter-list.tsx b/app/src/ui/lib/filter-list.tsx index 76917eb488..17bc6af842 100644 --- a/app/src/ui/lib/filter-list.tsx +++ b/app/src/ui/lib/filter-list.tsx @@ -194,13 +194,6 @@ export class FilterList extends React.Component< IFilterListProps, IFilterListState > { - public static getDerivedStateFromProps( - props: IFilterListProps, - state: IFilterListState - ) { - return createStateUpdate(props, state) - } - private list: List | null = null private filterTextBox: TextBox | null = null @@ -211,14 +204,11 @@ export class FilterList extends React.Component< this.filterTextBox = props.filterTextBox } - const filterValue = (props.filterText || '').toLowerCase() + this.state = createStateUpdate(props, null) + } - this.state = { - rows: new Array>(), - selectedRow: -1, - filterValue, - filterValueChanged: filterValue.length > 0, - } + public componentWillReceiveProps(nextProps: IFilterListProps) { + this.setState(createStateUpdate(nextProps, this.state)) } public componentDidUpdate( @@ -598,7 +588,7 @@ export function getText( function createStateUpdate( props: IFilterListProps, - state: IFilterListState + state: IFilterListState | null ) { const flattenedRows = new Array>() const filter = (props.filterText || '').toLowerCase() @@ -640,7 +630,9 @@ function createStateUpdate( } // Stay true if already set, otherwise become true if the filter has content - const filterValueChanged = state.filterValueChanged ? true : filter.length > 0 + const filterValueChanged = state?.filterValueChanged + ? true + : filter.length > 0 return { rows: flattenedRows, From 2fd0eb8328dfa6642b0447a5665d9c327a90a1fc Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 31 Aug 2023 09:58:08 +0200 Subject: [PATCH 4/5] In selectable (flat) lists without selected items, set first item as next tab stop --- app/src/ui/lib/list/list.tsx | 133 ++++++++++++++++++------------- app/src/ui/lib/list/selection.ts | 9 +++ 2 files changed, 85 insertions(+), 57 deletions(-) diff --git a/app/src/ui/lib/list/list.tsx b/app/src/ui/lib/list/list.tsx index 659d173e9e..283c139cc2 100644 --- a/app/src/ui/lib/list/list.tsx +++ b/app/src/ui/lib/list/list.tsx @@ -711,6 +711,16 @@ export class List extends React.Component { return this.props.canSelectRow ? this.props.canSelectRow(rowIndex) : true } + private getFirstSelectableRowIndexPath(): number | null { + for (let i = 0; i < this.props.rowCount; i++) { + if (this.canSelectRow(i)) { + return i + } + } + + return null + } + private addSelection(direction: SelectionDirection, source: SelectionSource) { if (this.props.selectedRows.length === 0) { return this.moveSelection(direction, source) @@ -955,66 +965,73 @@ export class List extends React.Component { return customClasses.length === 0 ? undefined : customClasses.join(' ') } - private renderRow = (params: IRowRendererParams) => { - const rowIndex = params.rowIndex - const selectable = this.canSelectRow(rowIndex) - const selected = this.props.selectedRows.indexOf(rowIndex) !== -1 - const customClasses = this.getCustomRowClassNames(rowIndex) + private getRowRenderer = (firstSelectableRowIndex: number | null) => { + return (params: IRowRendererParams) => { + const { selectedRows } = this.props + const rowIndex = params.rowIndex + const selectable = this.canSelectRow(rowIndex) + const selected = selectedRows.indexOf(rowIndex) !== -1 + const customClasses = this.getCustomRowClassNames(rowIndex) - // An unselectable row shouldn't be focusable - let tabIndex: number | undefined = undefined - if (selectable) { - tabIndex = selected && this.props.selectedRows[0] === rowIndex ? 0 : -1 - } + // An unselectable row shouldn't be focusable + let tabIndex: number | undefined = undefined + if (selectable) { + tabIndex = + (selected && selectedRows[0] === rowIndex) || + (selectedRows.length === 0 && firstSelectableRowIndex === rowIndex) + ? 0 + : -1 + } - const row = this.props.rowRenderer(rowIndex) + const row = this.props.rowRenderer(rowIndex) - const element = - this.props.insertionDragType !== undefined ? ( - - {row} - - ) : ( - row + const element = + this.props.insertionDragType !== undefined ? ( + + {row} + + ) : ( + row + ) + + const id = this.getRowId(rowIndex) + + const ariaLabel = + this.props.getRowAriaLabel !== undefined + ? this.props.getRowAriaLabel(rowIndex) + : undefined + + return ( + ) - - const id = this.getRowId(rowIndex) - - const ariaLabel = - this.props.getRowAriaLabel !== undefined - ? this.props.getRowAriaLabel(rowIndex) - : undefined - - return ( - - ) + } } public render() { @@ -1132,7 +1149,9 @@ export class List extends React.Component { } rowCount={this.props.rowCount} rowHeight={this.props.rowHeight} - cellRenderer={this.renderRow} + cellRenderer={this.getRowRenderer( + this.getFirstSelectableRowIndexPath() + )} onScroll={this.onScroll} scrollTop={this.props.setScrollTop} overscanRowCount={4} diff --git a/app/src/ui/lib/list/selection.ts b/app/src/ui/lib/list/selection.ts index 0c288d7390..023159f69e 100644 --- a/app/src/ui/lib/list/selection.ts +++ b/app/src/ui/lib/list/selection.ts @@ -62,12 +62,21 @@ export interface ISelectAllSource { readonly kind: 'select-all' } +/** + * Interface describing a user initiated selection change event originating + * when focusing the list when it has no selection. + */ +export interface IFocusSource { + readonly kind: 'focus' +} + /** A type union of possible sources of a selection changed event */ export type SelectionSource = | IMouseClickSource | IHoverSource | IKeyboardSource | ISelectAllSource + | IFocusSource /** * Determine the next selectable row, given the direction and a starting From 899fa8f50c0cc30782f9bed50049ade895bd71dc Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 31 Aug 2023 09:58:19 +0200 Subject: [PATCH 5/5] In selectable (section) lists without selected items, set first item as next tab stop --- app/src/ui/lib/list/section-list-selection.ts | 9 ++++ app/src/ui/lib/list/section-list.tsx | 41 ++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/app/src/ui/lib/list/section-list-selection.ts b/app/src/ui/lib/list/section-list-selection.ts index 87e1473bdd..3040c0526e 100644 --- a/app/src/ui/lib/list/section-list-selection.ts +++ b/app/src/ui/lib/list/section-list-selection.ts @@ -71,12 +71,21 @@ export interface ISelectAllSource { readonly kind: 'select-all' } +/** + * Interface describing a user initiated selection change event originating + * when focusing the list when it has no selection. + */ +export interface IFocusSource { + readonly kind: 'focus' +} + /** A type union of possible sources of a selection changed event */ export type SelectionSource = | IMouseClickSource | IHoverSource | IKeyboardSource | ISelectAllSource + | IFocusSource /** * Determine the next selectable row, given the direction and a starting diff --git a/app/src/ui/lib/list/section-list.tsx b/app/src/ui/lib/list/section-list.tsx index 1c814482ed..da7728a162 100644 --- a/app/src/ui/lib/list/section-list.tsx +++ b/app/src/ui/lib/list/section-list.tsx @@ -742,6 +742,11 @@ export class SectionList extends React.Component< // focused list item if it scrolls back into view. if (!focusWithin) { this.focusRow = InvalidRowIndexPath + } else if (this.props.selectedRows.length === 0) { + const firstSelectableRowIndexPath = this.getFirstSelectableRowIndexPath() + if (firstSelectableRowIndexPath !== null) { + this.moveSelectionTo(firstSelectableRowIndexPath, { kind: 'focus' }) + } } } @@ -810,6 +815,22 @@ export class SectionList extends React.Component< return this.props.canSelectRow ? this.props.canSelectRow(rowIndex) : true } + private getFirstSelectableRowIndexPath(): RowIndexPath | null { + const { rowCount } = this.props + + for (let section = 0; section < rowCount.length; section++) { + const rowCountInSection = rowCount[section] + for (let row = 0; row < rowCountInSection; row++) { + const indexPath = { section, row } + if (this.canSelectRow(indexPath)) { + return indexPath + } + } + } + + return null + } + private addSelection(direction: SelectionDirection, source: SelectionSource) { if (this.props.selectedRows.length === 0) { return this.moveSelection(direction, source) @@ -1110,8 +1131,12 @@ export class SectionList extends React.Component< return customClasses.length === 0 ? undefined : customClasses.join(' ') } - private getRowRenderer = (section: number) => { + private getRowRenderer = ( + section: number, + firstSelectableRowIndexPath: RowIndexPath | null + ) => { return (params: IRowRendererParams) => { + const { selectedRows } = this.props const indexPath: RowIndexPath = { section: section, row: params.rowIndex, @@ -1119,16 +1144,17 @@ export class SectionList extends React.Component< const selectable = this.canSelectRow(indexPath) const selected = - this.props.selectedRows.findIndex(r => - rowIndexPathEquals(r, indexPath) - ) !== -1 + selectedRows.findIndex(r => rowIndexPathEquals(r, indexPath)) !== -1 const customClasses = this.getCustomRowClassNames(indexPath) // An unselectable row shouldn't be focusable let tabIndex: number | undefined = undefined if (selectable) { tabIndex = - selected && rowIndexPathEquals(this.props.selectedRows[0], indexPath) + (selected && rowIndexPathEquals(selectedRows[0], indexPath)) || + (selectedRows.length === 0 && + firstSelectableRowIndexPath !== null && + rowIndexPathEquals(firstSelectableRowIndexPath, indexPath)) ? 0 : -1 } @@ -1303,7 +1329,10 @@ export class SectionList extends React.Component< columnCount={1} rowCount={this.props.rowCount[section]} rowHeight={this.getRowHeight(section)} - cellRenderer={this.getRowRenderer(section)} + cellRenderer={this.getRowRenderer( + section, + this.getFirstSelectableRowIndexPath() + )} scrollTop={relativeScrollTop} overscanRowCount={4} style={{ ...params.style, width: '100%' }}