From e97433328e617113bf65255bb23cb0f377984133 Mon Sep 17 00:00:00 2001 From: Saad Najmi Date: Tue, 19 Jul 2022 15:13:32 -0700 Subject: [PATCH 1/2] Keyboard navigation in Flatlist (#1258) * add pull yml * match handleOpenURLNotification event payload with iOS (#755) (#2) Co-authored-by: Ryan Linton * [pull] master from microsoft:master (#11) * Deprecated api (#853) * Remove deprecated/unused context param * Update a few Mac deprecated APIs * Packing RN dependencies, hermes and ignoring javadoc failure, (#852) * Ignore javadoc failure * Bringing few more changes from 0.63-stable * Fixing a patch in engine selection * Fixing a patch in nuget spec * Fixing the output directory of nuget pack * Packaging dependencies in the nuget * Fix onMouseEnter/onMouseLeave callbacks not firing on Pressable (#855) * add pull yml * match handleOpenURLNotification event payload with iOS (#755) (#2) Co-authored-by: Ryan Linton * fix mouse evetns on pressable * delete extra yml from this branch * Add macOS tags * reorder props to have onMouseEnter/onMouseLeave always be before onPress Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com> Co-authored-by: Ryan Linton * Grammar fixes. (#856) Updates simple grammar issues. Co-authored-by: Nick Trescases <42704557+ntre@users.noreply.github.com> Co-authored-by: Anandraj Co-authored-by: Saad Najmi Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com> Co-authored-by: Ryan Linton Co-authored-by: Muhammad Hamza Zaman * wip * wip * more wip * Home/End/OptionUp/OptionDown work * ensureItemAtIndexIsVisible works * Home/End work * Initial cleanup for PR * More cleanup * More cleanup * Make it a real prop * No need for client code * Don't move keyboard focus with selection * Update tags * Fix flow errors * Update colors, make ScrollView focusable * prettier * undo change * Fix flow errors * Clean up code + handle page up/down with new prop Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com> Co-authored-by: Ryan Linton Co-authored-by: Nick Trescases <42704557+ntre@users.noreply.github.com> Co-authored-by: Anandraj Co-authored-by: Muhammad Hamza Zaman --- 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 | 27 +++- .../js/examples/FlatList/FlatList-basic.js | 14 ++- 6 files changed, 173 insertions(+), 112 deletions(-) diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index be3b718ab0f284..599c20142af87e 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -1206,42 +1206,10 @@ class ScrollView extends React.Component { nativeEvent.contentOffset.y + nativeEvent.layoutMeasurement.height, }); - } 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), - }); + } else if (key === 'HOME') { + this.scrollTo({x: 0, y: 0}); + } else if (key === 'END') { + this.scrollToEnd({animated: true}); } } } diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 678f936c56aa28..4dd797dfeb8a6e 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.max(0, visTop - frame.length); + const newOffset = Math.min(frame.offset, visTop - frame.length); this.scrollToOffset({offset: newOffset}); } } @@ -884,7 +884,13 @@ class VirtualizedList extends React.PureComponent { index={ii} inversionStyle={inversionStyle} item={item} - isSelected={this.state.selectedRowIndex === ii ? true : false} // TODO(macOS GH#774) + // [TODO(macOS GH#774) + isSelected={ + this.props.enableSelectionOnKeyPress && + this.state.selectedRowIndex === ii + ? true + : false + } // TODO(macOS GH#774)] key={key} prevCellKey={prevCellKey} onUpdateSeparators={this._onUpdateSeparators} @@ -1322,10 +1328,12 @@ class VirtualizedList extends React.PureComponent { // $FlowFixMe[prop-missing] Invalid prop usage { // $FlowFixMe Invalid prop usage ); } @@ -1506,6 +1514,13 @@ 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; @@ -1520,14 +1535,14 @@ class VirtualizedList extends React.PureComponent { } }; - _handleKeyDown = (e: ScrollEvent) => { + _handleKeyDown = (event: ScrollEvent) => { if (this.props.onScrollKeyDown) { - this.props.onScrollKeyDown(e); + this.props.onScrollKeyDown(event); } else { if (Platform.OS === 'macos') { // $FlowFixMe Cannot get e.nativeEvent because property nativeEvent is missing in Event - const event = e.nativeEvent; - const key = event.key; + const nativeEvent = event.nativeEvent; + const key = nativeEvent.key; let prevIndex = -1; let newIndex = -1; @@ -1535,46 +1550,66 @@ class VirtualizedList extends React.PureComponent { prevIndex = this.state.selectedRowIndex; } - const {data, getItem} = this.props; - if (key === 'DOWN_ARROW') { - newIndex = this._selectRowBelowIndex(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 === 'UP_ARROW') { + // const {data, getItem} = this.props; + 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, - }); - } - } + this._handleSelectionChange(prevIndex, newIndex); + } else if (key === 'DOWN_ARROW') { + newIndex = this._selectRowBelowIndex(prevIndex); + this._handleSelectionChange(prevIndex, newIndex); } else if (key === 'ENTER') { if (this.props.onSelectionEntered) { - const item = getItem(data, prevIndex); + const item = this.props.getItem(this.props.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() { @@ -2172,6 +2207,7 @@ class CellRenderer extends React.Component< return React.createElement(ListItemComponent, { item, index, + isSelected, separators: this._separators, }); } @@ -2248,6 +2284,7 @@ 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 9ff6b14f66c3d3..19ec9dca0f8acf 100644 --- a/React/Views/ScrollView/RCTScrollView.m +++ b/React/Views/ScrollView/RCTScrollView.m @@ -1258,16 +1258,22 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager #if TARGET_OS_OSX // [TODO(macOS GH#774) -- (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode +- (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode modifierFlags:(NSEventModifierFlags)modifierFlags { switch (keyCode) { case 36: return @"ENTER"; + case 115: + return @"HOME"; + case 116: return @"PAGE_UP"; + case 119: + return @"END"; + case 121: return @"PAGE_DOWN"; @@ -1278,10 +1284,18 @@ - (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode return @"RIGHT_ARROW"; case 125: - return @"DOWN_ARROW"; + if (modifierFlags & NSEventModifierFlagOption) { + return @"OPTION_DOWN"; + } else { + return @"DOWN_ARROW"; + } case 126: - return @"UP_ARROW"; + if (modifierFlags & NSEventModifierFlagOption) { + return @"OPTION_UP"; + } else { + return @"UP_ARROW"; + } } return @""; } @@ -1289,24 +1303,25 @@ - (NSString*)keyCommandFromKeyCode:(NSInteger)keyCode - (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]; - 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 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]; } } } diff --git a/React/Views/UIView+React.m b/React/Views/UIView+React.m index 33d53bc4f7b617..6b6173c90b4411 100644 --- a/React/Views/UIView+React.m +++ b/React/Views/UIView+React.m @@ -282,29 +282,37 @@ - (void)setReactIsFocusNeeded:(BOOL)isFocusNeeded - (void)reactFocus { - if (![self becomeFirstResponder]) { - self.reactIsFocusNeeded = YES; - } +#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; + } } - (void)reactFocusIfNeeded { - if (self.reactIsFocusNeeded) { - if ([self becomeFirstResponder]) { - self.reactIsFocusNeeded = NO; - } - } + 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; + } + } } - (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 +#endif // TODO(macOS GH#774)] } #pragma mark - Layout diff --git a/packages/rn-tester/js/components/ListExampleShared.js b/packages/rn-tester/js/components/ListExampleShared.js index 1acad0e6f5a5e5..e83f313e2e9d09 100644 --- a/packages/rn-tester/js/components/ListExampleShared.js +++ b/packages/rn-tester/js/components/ListExampleShared.js @@ -22,6 +22,7 @@ const { Text, TextInput, View, + PlatformColor, // TODO(macOS GH#774) } = require('react-native'); export type Item = { @@ -58,13 +59,21 @@ class ItemComponent extends React.PureComponent<{ onShowUnderlay?: () => void, onHideUnderlay?: () => void, textSelectable?: ?boolean, + isSelected?: ?Boolean, // TODO(macOS GH#774) ... }> { _onPress = () => { this.props.onPress(this.props.item.key); }; render(): React.Node { - const {fixedHeight, horizontal, item, textSelectable} = this.props; + // [TODO(macOS GH#774) + const { + fixedHeight, + horizontal, + item, + textSelectable, + isSelected, + } = this.props; // TODO(macOS GH#774)] const itemHash = Math.abs(hashCode(item.title)); const imgSource = THUMB_URLS[itemHash % THUMB_URLS.length]; return ( @@ -78,10 +87,12 @@ 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} @@ -352,6 +363,16 @@ 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/FlatList-basic.js b/packages/rn-tester/js/examples/FlatList/FlatList-basic.js index 259daccc7b45ac..7338f50e3620b4 100644 --- a/packages/rn-tester/js/examples/FlatList/FlatList-basic.js +++ b/packages/rn-tester/js/examples/FlatList/FlatList-basic.js @@ -63,6 +63,7 @@ type State = {| fadingEdgeLength: number, onPressDisabled: boolean, textSelectable: boolean, + enableSelectionOnKeyPress: boolean, // TODO(macOS GH#774)] |}; class FlatListExample extends React.PureComponent { @@ -80,6 +81,7 @@ class FlatListExample extends React.PureComponent { fadingEdgeLength: 0, onPressDisabled: false, textSelectable: true, + enableSelectionOnKeyPress: false, // TODO(macOS GH#774) }; _onChangeFilterText = filterText => { @@ -182,6 +184,13 @@ 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' && ( { } @@ -276,7 +286,8 @@ 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]: ({item, separators}) => { + [flatListPropKey]: props => { + const {item, separators, isSelected} = props; // TODO(macOS GH#774) return ( { onShowUnderlay={separators.highlight} onHideUnderlay={separators.unhighlight} textSelectable={this.state.textSelectable} + isSelected={isSelected} // TODO(macOS GH#774) /> ); }, From b5710f4215284c3bddc202ac1b0133e7e3ff2861 Mon Sep 17 00:00:00 2001 From: Saad Najmi Date: Tue, 19 Jul 2022 16:12:14 -0700 Subject: [PATCH 2/2] yarn lint --fix --- .../rn-tester/js/components/ListExampleShared.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/rn-tester/js/components/ListExampleShared.js b/packages/rn-tester/js/components/ListExampleShared.js index e83f313e2e9d09..96957402bdfdaf 100644 --- a/packages/rn-tester/js/components/ListExampleShared.js +++ b/packages/rn-tester/js/components/ListExampleShared.js @@ -67,13 +67,8 @@ class ItemComponent extends React.PureComponent<{ }; render(): React.Node { // [TODO(macOS GH#774) - const { - fixedHeight, - horizontal, - item, - textSelectable, - isSelected, - } = this.props; // TODO(macOS GH#774)] + const {fixedHeight, horizontal, item, textSelectable, isSelected} = + this.props; // TODO(macOS GH#774)] const itemHash = Math.abs(hashCode(item.title)); const imgSource = THUMB_URLS[itemHash % THUMB_URLS.length]; return ( @@ -88,8 +83,7 @@ class ItemComponent extends React.PureComponent<{ horizontal && {width: HORIZ_WIDTH}, fixedHeight && {height: ITEM_HEIGHT}, isSelected && styles.selectedItem, // TODO(macOS GH#774) - ]} - > + ]}> {!item.noImage && }