From 34760eb7852235bee549808bb6cb38d7eb19ae78 Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Tue, 12 Sep 2023 13:08:23 +0100 Subject: [PATCH 1/6] fix(selection-list): highlight on lists with 1 section --- .../SelectionList/BaseSelectionList.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index b76ded8a542f..1b45f0d42e5c 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -167,15 +167,13 @@ function BaseSelectionList({ listRef.current.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight}); }; - const selectRow = (item, index) => { + const selectRow = (item) => { // In single-selection lists we don't care about updating the focused index, because the list is closed after selecting an item if (canSelectMultiple) { - if (sections.length === 1) { - // If the list has only 1 section (e.g. Workspace Members list), we always focus the next available item - const nextAvailableIndex = _.findIndex(flattenedSections.allOptions, (option, i) => i > index && !option.isDisabled); - setFocusedIndex(nextAvailableIndex); - } else { - // If the list has multiple sections (e.g. Workspace Invite list), we focus the first one after all the selected (selected items are always at the top) + if (sections.length > 1) { + // If the list has only 1 section (e.g. Workspace Members list), we do nothing. + // If the list has multiple sections (e.g. Workspace Invite list), we focus the first one after all the selected (selected items are always at the top). + const selectedOptionsCount = item.isSelected ? flattenedSections.selectedOptions.length - 1 : flattenedSections.selectedOptions.length + 1; setFocusedIndex(selectedOptionsCount); @@ -196,7 +194,7 @@ function BaseSelectionList({ return; } - selectRow(focusedOption, focusedIndex); + selectRow(focusedOption); }; /** @@ -250,7 +248,7 @@ function BaseSelectionList({ selectRow(item, index)} + onSelectRow={() => selectRow(item)} onDismissError={onDismissError} /> ); @@ -261,7 +259,7 @@ function BaseSelectionList({ item={item} isFocused={isFocused} isDisabled={isDisabled} - onSelectRow={() => selectRow(item, index)} + onSelectRow={() => selectRow(item)} /> ); }; From 67c2a5f7fe67c2dfe2574af00056dc07885418a2 Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Tue, 12 Sep 2023 13:59:40 +0100 Subject: [PATCH 2/6] chore: make function call concise --- src/components/SelectionList/BaseSelectionList.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 1b45f0d42e5c..230de8e75dcc 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -248,7 +248,7 @@ function BaseSelectionList({ selectRow(item)} + onSelectRow={selectRow} onDismissError={onDismissError} /> ); @@ -259,7 +259,7 @@ function BaseSelectionList({ item={item} isFocused={isFocused} isDisabled={isDisabled} - onSelectRow={() => selectRow(item)} + onSelectRow={selectRow} /> ); }; From cc0c7e2d1eabc828a0abb61fc9cbebc8692cc1e2 Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Thu, 14 Sep 2023 11:29:26 +0100 Subject: [PATCH 3/6] fix(selection-list): remove active item highlight --- src/components/SelectionList/RadioListItem.js | 1 - src/components/SelectionList/UserListItem.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/components/SelectionList/RadioListItem.js b/src/components/SelectionList/RadioListItem.js index 92e3e84b66c8..4adf582b30d4 100644 --- a/src/components/SelectionList/RadioListItem.js +++ b/src/components/SelectionList/RadioListItem.js @@ -17,7 +17,6 @@ function RadioListItem({item, isFocused = false, isDisabled = false, onSelectRow accessibilityRole="button" hoverDimmingValue={1} hoverStyle={styles.hoveredComponentBG} - focusStyle={styles.hoveredComponentBG} > diff --git a/src/components/SelectionList/UserListItem.js b/src/components/SelectionList/UserListItem.js index dd90fc750510..f701b1873cbe 100644 --- a/src/components/SelectionList/UserListItem.js +++ b/src/components/SelectionList/UserListItem.js @@ -62,7 +62,6 @@ function UserListItem({item, isFocused = false, showTooltip, onSelectRow, onDism accessibilityState={{checked: item.isSelected}} hoverDimmingValue={1} hoverStyle={styles.hoveredComponentBG} - focusStyle={styles.hoveredComponentBG} > Date: Thu, 14 Sep 2023 16:15:15 +0100 Subject: [PATCH 4/6] fix(selection-list): remove active item highlight --- src/components/Button/index.js | 1 + src/components/SelectionList/BaseSelectionList.js | 5 ++++- src/components/SelectionList/RadioListItem.js | 1 - src/components/SelectionList/UserListItem.js | 1 - src/hooks/useActiveElement/index.js | 6 ++++++ src/hooks/useActiveElement/index.native.js | 5 +++++ 6 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/components/Button/index.js b/src/components/Button/index.js index bfde528a4750..0d19691d1256 100644 --- a/src/components/Button/index.js +++ b/src/components/Button/index.js @@ -305,6 +305,7 @@ class Button extends Component { ]} nativeID={this.props.nativeID} accessibilityLabel={this.props.accessibilityLabel} + accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON} hoverDimmingValue={1} > {this.renderContent()} diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index 6699e56e54f6..895272ba18ef 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -135,6 +135,9 @@ function BaseSelectionList({ }; }, [canSelectMultiple, sections]); + // Disable `Enter` hotkey if the active element is a button or checkbox + const shouldDisableHotkeys = activeElement && [CONST.ACCESSIBILITY_ROLE.BUTTON, CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role); + // If `initiallyFocusedOptionKey` is not passed, we fall back to `-1`, to avoid showing the highlight on the first member const [focusedIndex, setFocusedIndex] = useState(() => _.findIndex(flattenedSections.allOptions, (option) => option.keyForList === initiallyFocusedOptionKey)); @@ -287,7 +290,7 @@ function BaseSelectionList({ useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, { captureOnInputs: true, shouldBubble: () => !flattenedSections.allOptions[focusedIndex], - isActive: !activeElement, + isActive: !shouldDisableHotkeys, }); /** Calls confirm action when pressing CTRL (CMD) + Enter */ diff --git a/src/components/SelectionList/RadioListItem.js b/src/components/SelectionList/RadioListItem.js index 92e3e84b66c8..4adf582b30d4 100644 --- a/src/components/SelectionList/RadioListItem.js +++ b/src/components/SelectionList/RadioListItem.js @@ -17,7 +17,6 @@ function RadioListItem({item, isFocused = false, isDisabled = false, onSelectRow accessibilityRole="button" hoverDimmingValue={1} hoverStyle={styles.hoveredComponentBG} - focusStyle={styles.hoveredComponentBG} > diff --git a/src/components/SelectionList/UserListItem.js b/src/components/SelectionList/UserListItem.js index dd90fc750510..f701b1873cbe 100644 --- a/src/components/SelectionList/UserListItem.js +++ b/src/components/SelectionList/UserListItem.js @@ -62,7 +62,6 @@ function UserListItem({item, isFocused = false, showTooltip, onSelectRow, onDism accessibilityState={{checked: item.isSelected}} hoverDimmingValue={1} hoverStyle={styles.hoveredComponentBG} - focusStyle={styles.hoveredComponentBG} > Date: Thu, 21 Sep 2023 15:41:18 +0100 Subject: [PATCH 5/6] fix(selection-list): remove highlight when pressing --- .../SelectionList/BaseSelectionList.js | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index e0e4f8740cff..fb9d27323735 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -171,21 +171,35 @@ function BaseSelectionList({ listRef.current.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight}); }; - const selectRow = (item) => { + /** + * Logic to run when a row is selected, either with click/press or keyboard hotkeys. + * + * @param item - the list item + * @param {Boolean} shouldUnfocusRow - flag to decide if we should unfocus all rows. True when selecting a row with click or press (not keyboard) + */ + const selectRow = (item, shouldUnfocusRow = false) => { // In single-selection lists we don't care about updating the focused index, because the list is closed after selecting an item if (canSelectMultiple) { if (sections.length > 1) { // If the list has only 1 section (e.g. Workspace Members list), we do nothing. - // If the list has multiple sections (e.g. Workspace Invite list), we focus the first one after all the selected (selected items are always at the top). - + // If the list has multiple sections (e.g. Workspace Invite list), and `shouldUnfocusRow` is false, + // we focus the first one after all the selected (selected items are always at the top). const selectedOptionsCount = item.isSelected ? flattenedSections.selectedOptions.length - 1 : flattenedSections.selectedOptions.length + 1; - setFocusedIndex(selectedOptionsCount); + + if (!shouldUnfocusRow) { + setFocusedIndex(selectedOptionsCount); + } if (!item.isSelected) { // If we're selecting an item, scroll to it's position at the top, so we can see it scrollToIndex(Math.max(selectedOptionsCount - 1, 0), true); } } + + if (shouldUnfocusRow) { + // Unfocus all rows when selecting row with click/press + setFocusedIndex(-1); + } } onSelectRow(item); @@ -255,7 +269,7 @@ function BaseSelectionList({ selectRow(item, true)} onDismissError={onDismissError} showTooltip={showTooltip} /> @@ -267,7 +281,7 @@ function BaseSelectionList({ item={item} isFocused={isItemFocused} isDisabled={isDisabled} - onSelectRow={selectRow} + onSelectRow={() => selectRow(item, true)} /> ); }; From ec40451e00d3239e35eb02b18d0f78d945e030a9 Mon Sep 17 00:00:00 2001 From: Thiago Brezinski Date: Thu, 21 Sep 2023 18:01:26 +0100 Subject: [PATCH 6/6] chore: fix jsdoc --- src/components/SelectionList/BaseSelectionList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SelectionList/BaseSelectionList.js b/src/components/SelectionList/BaseSelectionList.js index fb9d27323735..f1de4a04e78d 100644 --- a/src/components/SelectionList/BaseSelectionList.js +++ b/src/components/SelectionList/BaseSelectionList.js @@ -174,7 +174,7 @@ function BaseSelectionList({ /** * Logic to run when a row is selected, either with click/press or keyboard hotkeys. * - * @param item - the list item + * @param {Object} item - the list item * @param {Boolean} shouldUnfocusRow - flag to decide if we should unfocus all rows. True when selecting a row with click or press (not keyboard) */ const selectRow = (item, shouldUnfocusRow = false) => {