From 1c1efa8d4000b3beb18d802ce82531941782b13a Mon Sep 17 00:00:00 2001 From: chiuam <67026167+chiuam@users.noreply.github.com> Date: Fri, 29 Jul 2022 12:51:26 -0400 Subject: [PATCH 1/2] Revert "[0.66] Flatlist keyboard navigation: Mouse can move selection (#1269)" This reverts commit 6496462f5a4204dcf5670cad32bd31f512a799b9. --- Libraries/Lists/FlatList.js | 13 ------------- Libraries/Lists/VirtualizedList.js | 4 ---- .../js/examples/FlatList/FlatListExample.js | 6 +----- 3 files changed, 1 insertion(+), 22 deletions(-) diff --git a/Libraries/Lists/FlatList.js b/Libraries/Lists/FlatList.js index 604ee53041852c..bda4951e24687b 100644 --- a/Libraries/Lists/FlatList.js +++ b/Libraries/Lists/FlatList.js @@ -369,19 +369,6 @@ class FlatList extends React.PureComponent, void> { } } - // [TODO(macOS GH#750) - /** - * Move selection to the specified index - * - * @platform macos - */ - selectRowAtIndex(index: number) { - if (this._listRef) { - this._listRef.selectRowAtIndex(index); - } - } - // ]TODO(macOS GH#750) - /** * Provides a handle to the underlying scroll responder. */ diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 4f44a655a68606..e04fc2179c0155 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -592,10 +592,6 @@ class VirtualizedList extends React.PureComponent { this.scrollToOffset({offset: newOffset}); } } - - selectRowAtIndex(rowIndex: number) { - this._selectRowAtIndex(rowIndex); - } // ]TODO(macOS GH#774) recordInteraction() { diff --git a/packages/rn-tester/js/examples/FlatList/FlatListExample.js b/packages/rn-tester/js/examples/FlatList/FlatListExample.js index 5b979d404079f7..ffd38c288343d4 100644 --- a/packages/rn-tester/js/examples/FlatList/FlatListExample.js +++ b/packages/rn-tester/js/examples/FlatList/FlatListExample.js @@ -61,7 +61,7 @@ type State = {| fadingEdgeLength: number, onPressDisabled: boolean, textSelectable: boolean, - enableSelectionOnKeyPress: boolean, // TODO(macOS GH#774) + enableSelectionOnKeyPress: boolean, // TODO(macOS GH#774)] |}; class FlatListExample extends React.PureComponent { @@ -303,10 +303,6 @@ class FlatListExample extends React.PureComponent { _pressItem = (key: string) => { this._listRef && this._listRef.recordInteraction(); const index = Number(key); - // [TODO(macOS GH#774) - if (this.state.enableSelectionOnKeyPress) { - this._listRef && this._listRef.selectRowAtIndex(index); - } // ]TODO(macOS GH#774) const itemState = pressItem(this.state.data[index]); this.setState(state => ({ ...state, From 460a15552579a4340c88f65094a62266cfba97f4 Mon Sep 17 00:00:00 2001 From: chiuam <67026167+chiuam@users.noreply.github.com> Date: Fri, 29 Jul 2022 13:44:15 -0400 Subject: [PATCH 2/2] revert keyboard navigation in flatlists --- Libraries/Components/ScrollView/ScrollView.js | 40 +++++- Libraries/Lists/VirtualizedList.js | 119 ++++++------------ React/Views/ScrollView/RCTScrollView.m | 57 ++++----- React/Views/UIView+React.m | 28 ++--- .../js/components/ListExampleShared.js | 26 +--- .../js/examples/FlatList/FlatListExample.js | 19 +-- 6 files changed, 111 insertions(+), 178 deletions(-) diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index 95214edcef6045..541245effac1c2 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -1201,10 +1201,42 @@ class ScrollView extends React.Component { nativeEvent.contentOffset.y + nativeEvent.layoutMeasurement.height, }); - } else if (key === 'HOME') { - this.scrollTo({x: 0, y: 0}); - } else if (key === 'END') { - this.scrollToEnd({animated: true}); + } else if (key === 'LEFT_ARROW') { + this._handleScrollByKeyDown(event, { + x: + nativeEvent.contentOffset.x + + -(this.props.horizontalLineScroll !== undefined + ? this.props.horizontalLineScroll + : kMinScrollOffset), + y: nativeEvent.contentOffset.y, + }); + } else if (key === 'RIGHT_ARROW') { + this._handleScrollByKeyDown(event, { + x: + nativeEvent.contentOffset.x + + (this.props.horizontalLineScroll !== undefined + ? this.props.horizontalLineScroll + : kMinScrollOffset), + y: nativeEvent.contentOffset.y, + }); + } else if (key === 'DOWN_ARROW') { + this._handleScrollByKeyDown(event, { + x: nativeEvent.contentOffset.x, + y: + nativeEvent.contentOffset.y + + (this.props.verticalLineScroll !== undefined + ? this.props.verticalLineScroll + : kMinScrollOffset), + }); + } else if (key === 'UP_ARROW') { + this._handleScrollByKeyDown(event, { + x: nativeEvent.contentOffset.x, + y: + nativeEvent.contentOffset.y + + -(this.props.verticalLineScroll !== undefined + ? this.props.verticalLineScroll + : kMinScrollOffset), + }); } } } diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index e04fc2179c0155..1422f27a444fd5 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -588,7 +588,7 @@ class VirtualizedList extends React.PureComponent { const newOffset = Math.min(contentLength, visTop + (frameEnd - visEnd)); this.scrollToOffset({offset: newOffset}); } else if (frame.offset < visTop) { - const newOffset = Math.min(frame.offset, visTop - frame.length); + const newOffset = Math.max(0, visTop - frame.length); this.scrollToOffset({offset: newOffset}); } } @@ -882,13 +882,7 @@ class VirtualizedList extends React.PureComponent { index={ii} inversionStyle={inversionStyle} item={item} - // [TODO(macOS GH#774) - isSelected={ - this.props.enableSelectionOnKeyPress && - this.state.selectedRowIndex === ii - ? true - : false - } // TODO(macOS GH#774)] + isSelected={this.state.selectedRowIndex === ii ? true : false} // TODO(macOS GH#774) key={key} prevCellKey={prevCellKey} onUpdateSeparators={this._onUpdateSeparators} @@ -1329,12 +1323,10 @@ class VirtualizedList extends React.PureComponent { // $FlowFixMe[prop-missing] Invalid prop usage { // $FlowFixMe Invalid prop usage ); } @@ -1515,13 +1507,6 @@ class VirtualizedList extends React.PureComponent { return rowAbove; }; - _selectRowAtIndex = rowIndex => { - this.setState(state => { - return {selectedRowIndex: rowIndex}; - }); - return rowIndex; - }; - _selectRowBelowIndex = rowIndex => { if (this.props.getItemCount) { const {data} = this.props; @@ -1536,14 +1521,14 @@ class VirtualizedList extends React.PureComponent { } }; - _handleKeyDown = (event: ScrollEvent) => { + _handleKeyDown = (e: ScrollEvent) => { if (this.props.onScrollKeyDown) { - this.props.onScrollKeyDown(event); + this.props.onScrollKeyDown(e); } else { if (Platform.OS === 'macos') { // $FlowFixMe Cannot get e.nativeEvent because property nativeEvent is missing in Event - const nativeEvent = event.nativeEvent; - const key = nativeEvent.key; + const event = e.nativeEvent; + const key = event.key; let prevIndex = -1; let newIndex = -1; @@ -1551,66 +1536,46 @@ class VirtualizedList extends React.PureComponent { prevIndex = this.state.selectedRowIndex; } - // const {data, getItem} = this.props; - if (key === 'UP_ARROW') { - newIndex = this._selectRowAboveIndex(prevIndex); - this._handleSelectionChange(prevIndex, newIndex); - } else if (key === 'DOWN_ARROW') { + const {data, getItem} = this.props; + if (key === 'DOWN_ARROW') { newIndex = this._selectRowBelowIndex(prevIndex); - this._handleSelectionChange(prevIndex, newIndex); + this.ensureItemAtIndexIsVisible(newIndex); + + if (prevIndex !== newIndex) { + const item = getItem(data, newIndex); + if (this.props.onSelectionChanged) { + this.props.onSelectionChanged({ + previousSelection: prevIndex, + newSelection: newIndex, + item: item, + }); + } + } + } else if (key === 'UP_ARROW') { + newIndex = this._selectRowAboveIndex(prevIndex); + this.ensureItemAtIndexIsVisible(newIndex); + + if (prevIndex !== newIndex) { + const item = getItem(data, newIndex); + if (this.props.onSelectionChanged) { + this.props.onSelectionChanged({ + previousSelection: prevIndex, + newSelection: newIndex, + item: item, + }); + } + } } else if (key === 'ENTER') { if (this.props.onSelectionEntered) { - const item = this.props.getItem(this.props.data, prevIndex); + const item = getItem(data, prevIndex); if (this.props.onSelectionEntered) { this.props.onSelectionEntered(item); } } - } else if (key === 'OPTION_UP') { - newIndex = this._selectRowAtIndex(0); - this._handleSelectionChange(prevIndex, newIndex); - } else if (key === 'OPTION_DOWN') { - newIndex = this._selectRowAtIndex(this.state.last); - this._handleSelectionChange(prevIndex, newIndex); - } else if (key === 'PAGE_UP') { - const maxY = - event.nativeEvent.contentSize.height - - event.nativeEvent.layoutMeasurement.height; - const newOffset = Math.min( - maxY, - nativeEvent.contentOffset.y + -nativeEvent.layoutMeasurement.height, - ); - this.scrollToOffset({animated: true, offset: newOffset}); - } else if (key === 'PAGE_DOWN') { - const maxY = - event.nativeEvent.contentSize.height - - event.nativeEvent.layoutMeasurement.height; - const newOffset = Math.min( - maxY, - nativeEvent.contentOffset.y + nativeEvent.layoutMeasurement.height, - ); - this.scrollToOffset({animated: true, offset: newOffset}); - } else if (key === 'HOME') { - this.scrollToOffset({animated: true, offset: 0}); - } else if (key === 'END') { - this.scrollToEnd({animated: true}); } } } }; - - _handleSelectionChange = (prevIndex, newIndex) => { - this.ensureItemAtIndexIsVisible(newIndex); - if (prevIndex !== newIndex) { - const item = this.props.getItem(this.props.data, newIndex); - if (this.props.onSelectionChanged) { - this.props.onSelectionChanged({ - previousSelection: prevIndex, - newSelection: newIndex, - item: item, - }); - } - } - }; // ]TODO(macOS GH#774) _renderDebugOverlay() { @@ -2217,7 +2182,6 @@ class CellRenderer extends React.Component< return React.createElement(ListItemComponent, { item, index, - isSelected, separators: this._separators, }); } @@ -2294,7 +2258,6 @@ class CellRenderer extends React.Component< {itemSeparator} ); - // TODO(macOS GH#774)] return ( diff --git a/React/Views/ScrollView/RCTScrollView.m b/React/Views/ScrollView/RCTScrollView.m index 807025f884e331..5ae6d4bd4e7dfc 100644 --- a/React/Views/ScrollView/RCTScrollView.m +++ b/React/Views/ScrollView/RCTScrollView.m @@ -1180,22 +1180,16 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager #if TARGET_OS_OSX // [TODO(macOS GH#774) -- (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode modifierFlags:(NSEventModifierFlags)modifierFlags +- (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode { switch (keyCode) { case 36: return @"ENTER"; - case 115: - return @"HOME"; - case 116: return @"PAGE_UP"; - case 119: - return @"END"; - case 121: return @"PAGE_DOWN"; @@ -1206,18 +1200,10 @@ - (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode modifierFlags:(NSEventModi return @"RIGHT_ARROW"; case 125: - if (modifierFlags & NSEventModifierFlagOption) { - return @"OPTION_DOWN"; - } else { - return @"DOWN_ARROW"; - } + return @"DOWN_ARROW"; case 126: - if (modifierFlags & NSEventModifierFlagOption) { - return @"OPTION_UP"; - } else { - return @"UP_ARROW"; - } + return @"UP_ARROW"; } return @""; } @@ -1225,25 +1211,24 @@ - (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode modifierFlags:(NSEventModi - (void)keyDown:(UIEvent*)theEvent { // Don't emit a scroll event if tab was pressed while the scrollview is first responder - if (!(self == [[self window] firstResponder] && theEvent.keyCode == 48)) { - NSString *keyCommand = [self keyCommandFromKeyCode:theEvent.keyCode modifierFlags:theEvent.modifierFlags]; - if (![keyCommand isEqual: @""]) { - RCT_SEND_SCROLL_EVENT(onScrollKeyDown, (@{ @"key": keyCommand})); - } else { - [super keyDown:theEvent]; - } - } - - // AX: if a tab key was pressed and the first responder is currently clipped by the scroll view, - // automatically scroll to make the view visible to make it navigable via keyboard. - if ([theEvent keyCode] == 48) { //tab key - id firstResponder = [[self window] firstResponder]; - if ([firstResponder isKindOfClass:[NSView class]] && - [firstResponder isDescendantOf:[_scrollView documentView]]) { - NSView *view = (NSView*)firstResponder; - NSRect visibleRect = ([view superview] == [_scrollView documentView]) ? NSInsetRect(view.frame, -1, -1) : - [view convertRect:view.frame toView:_scrollView.documentView]; - [[_scrollView documentView] scrollRectToVisible:visibleRect]; + if (self == [[self window] firstResponder] && + theEvent.keyCode != 48) { + NSString *keyCommand = [self keyCommandFromKeyCode:theEvent.keyCode]; + RCT_SEND_SCROLL_EVENT(onScrollKeyDown, (@{ @"key": keyCommand})); + } else { + [super keyDown:theEvent]; + + // AX: if a tab key was pressed and the first responder is currently clipped by the scroll view, + // automatically scroll to make the view visible to make it navigable via keyboard. + if ([theEvent keyCode] == 48) { //tab key + id firstResponder = [[self window] firstResponder]; + if ([firstResponder isKindOfClass:[NSView class]] && + [firstResponder isDescendantOf:[_scrollView documentView]]) { + NSView *view = (NSView*)firstResponder; + NSRect visibleRect = ([view superview] == [_scrollView documentView]) ? NSInsetRect(view.frame, -1, -1) : + [view convertRect:view.frame toView:_scrollView.documentView]; + [[_scrollView documentView] scrollRectToVisible:visibleRect]; + } } } } diff --git a/React/Views/UIView+React.m b/React/Views/UIView+React.m index 604790b3de9ba4..14a58f1cf79f7b 100644 --- a/React/Views/UIView+React.m +++ b/React/Views/UIView+React.m @@ -285,37 +285,29 @@ - (void)setReactIsFocusNeeded:(BOOL)isFocusNeeded - (void)reactFocus { -#if TARGET_OS_OSX // [TODO(macOS GH#774) - if (![[self window] makeFirstResponder:self]) { -#else - if (![self becomeFirstResponder]) { -#endif //// TODO(macOS GH#774)] - self.reactIsFocusNeeded = YES; - } + if (![self becomeFirstResponder]) { + self.reactIsFocusNeeded = YES; + } } - (void)reactFocusIfNeeded { - if (self.reactIsFocusNeeded) { -#if TARGET_OS_OSX // [TODO(macOS GH#774) - if ([[self window] makeFirstResponder:self]) { -#else - if ([self becomeFirstResponder]) { -#endif // TODO(macOS GH#774)] - self.reactIsFocusNeeded = NO; - } - } + if (self.reactIsFocusNeeded) { + if ([self becomeFirstResponder]) { + self.reactIsFocusNeeded = NO; + } + } } - (void)reactBlur { -#if TARGET_OS_OSX // [TODO(macOS GH#774) +#if TARGET_OS_OSX // TODO(macOS GH#774) if (self == [[self window] firstResponder]) { [[self window] makeFirstResponder:[[self window] nextResponder]]; } #else [self resignFirstResponder]; -#endif // TODO(macOS GH#774)] +#endif } #pragma mark - Layout diff --git a/packages/rn-tester/js/components/ListExampleShared.js b/packages/rn-tester/js/components/ListExampleShared.js index 3986ca6455849b..2312ee69c22f93 100644 --- a/packages/rn-tester/js/components/ListExampleShared.js +++ b/packages/rn-tester/js/components/ListExampleShared.js @@ -22,7 +22,6 @@ const { Text, TextInput, View, - PlatformColor, // TODO(macOS GH#774) } = require('react-native'); export type Item = { @@ -58,22 +57,13 @@ class ItemComponent extends React.PureComponent<{ onPress: (key: string) => void, onShowUnderlay?: () => void, onHideUnderlay?: () => void, - textSelectable?: ?boolean, - isSelected?: ?Boolean, // TODO(macOS GH#774) ... }> { _onPress = () => { this.props.onPress(this.props.item.key); }; render(): React.Node { - // [TODO(macOS GH#774) - const { - fixedHeight, - horizontal, - item, - textSelectable, - isSelected, - } = this.props; // TODO(macOS GH#774)] + const {fixedHeight, horizontal, item} = this.props; const itemHash = Math.abs(hashCode(item.title)); const imgSource = THUMB_URLS[itemHash % THUMB_URLS.length]; return ( @@ -87,12 +77,10 @@ class ItemComponent extends React.PureComponent<{ styles.row, horizontal && {width: HORIZ_WIDTH}, fixedHeight && {height: ITEM_HEIGHT}, - isSelected && styles.selectedItem, // TODO(macOS GH#774) ]}> {!item.noImage && } {item.title} - {item.text} @@ -362,16 +350,6 @@ const styles = StyleSheet.create({ text: { flex: 1, }, - // [TODO(macOS GH#774) - selectedItem: { - backgroundColor: PlatformColor('selectedContentBackgroundColor'), - }, - selectedItemText: { - // This was the closest UI Element color that looked right... - // https://developer.apple.com/documentation/appkit/nscolor/ui_element_colors - color: PlatformColor('selectedMenuItemTextColor'), - }, - // [TODO(macOS GH#774)] }); module.exports = { diff --git a/packages/rn-tester/js/examples/FlatList/FlatListExample.js b/packages/rn-tester/js/examples/FlatList/FlatListExample.js index ffd38c288343d4..4606d68090c628 100644 --- a/packages/rn-tester/js/examples/FlatList/FlatListExample.js +++ b/packages/rn-tester/js/examples/FlatList/FlatListExample.js @@ -59,9 +59,6 @@ type State = {| empty: boolean, useFlatListItemComponent: boolean, fadingEdgeLength: number, - onPressDisabled: boolean, - textSelectable: boolean, - enableSelectionOnKeyPress: boolean, // TODO(macOS GH#774)] |}; class FlatListExample extends React.PureComponent { @@ -77,9 +74,6 @@ class FlatListExample extends React.PureComponent { empty: false, useFlatListItemComponent: false, fadingEdgeLength: 0, - onPressDisabled: false, - textSelectable: true, - enableSelectionOnKeyPress: false, // TODO(macOS GH#774) }; _onChangeFilterText = filterText => { @@ -172,13 +166,6 @@ class FlatListExample extends React.PureComponent { this.state.useFlatListItemComponent, this._setBooleanValue('useFlatListItemComponent'), )} - {/* [TODO(macOS GH#774) */} - {renderSmallSwitchOption( - 'Keyboard Navigation', - this.state.enableSelectionOnKeyPress, - this._setBooleanValue('enableSelectionOnKeyPress'), - )} - {/* TODO(macOS GH#774)] */} {Platform.OS === 'android' && ( { } @@ -261,8 +247,7 @@ class FlatListExample extends React.PureComponent { /* $FlowFixMe[invalid-computed-prop] (>=0.111.0 site=react_native_fb) * This comment suppresses an error found when Flow v0.111 was deployed. * To see the error, delete this comment and run Flow. */ - [flatListPropKey]: props => { - const {item, separators, isSelected} = props; // TODO(macOS GH#774) + [flatListPropKey]: ({item, separators}) => { return ( { onPress={this._pressItem} onShowUnderlay={separators.highlight} onHideUnderlay={separators.unhighlight} - textSelectable={this.state.textSelectable} - isSelected={isSelected} // TODO(macOS GH#774) /> ); },