Skip to content

Commit

Permalink
Merge pull request #27246 from thiagobrez/thiagobrez/selection-list-h…
Browse files Browse the repository at this point in the history
…ighlight-jump

fix(selection-list): highlight on lists with 1 section
  • Loading branch information
cristipaval committed Sep 25, 2023
2 parents db4d85b + ec40451 commit 38030f8
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/components/Button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ class Button extends Component {
]}
nativeID={this.props.nativeID}
accessibilityLabel={this.props.accessibilityLabel}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
hoverDimmingValue={1}
>
{this.renderContent()}
Expand Down
39 changes: 27 additions & 12 deletions src/components/SelectionList/BaseSelectionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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));

Expand Down Expand Up @@ -168,23 +171,35 @@ function BaseSelectionList({
listRef.current.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight});
};

const selectRow = (item, index) => {
/**
* Logic to run when a row is selected, either with click/press or keyboard hotkeys.
*
* @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) => {
// 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), 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);
Expand All @@ -197,7 +212,7 @@ function BaseSelectionList({
return;
}

selectRow(focusedOption, focusedIndex);
selectRow(focusedOption);
};

/**
Expand Down Expand Up @@ -254,7 +269,7 @@ function BaseSelectionList({
<UserListItem
item={item}
isFocused={isItemFocused}
onSelectRow={() => selectRow(item, index)}
onSelectRow={() => selectRow(item, true)}
onDismissError={onDismissError}
showTooltip={showTooltip}
/>
Expand All @@ -266,7 +281,7 @@ function BaseSelectionList({
item={item}
isFocused={isItemFocused}
isDisabled={isDisabled}
onSelectRow={() => selectRow(item, index)}
onSelectRow={() => selectRow(item, true)}
/>
);
};
Expand All @@ -290,7 +305,7 @@ function BaseSelectionList({
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, {
captureOnInputs: true,
shouldBubble: () => !flattenedSections.allOptions[focusedIndex],
isActive: !activeElement && isFocused,
isActive: !shouldDisableHotkeys && isFocused,
});

/** Calls confirm action when pressing CTRL (CMD) + Enter */
Expand Down
1 change: 0 additions & 1 deletion src/components/SelectionList/RadioListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ function RadioListItem({item, isFocused = false, isDisabled = false, onSelectRow
accessibilityRole="button"
hoverDimmingValue={1}
hoverStyle={styles.hoveredComponentBG}
focusStyle={styles.hoveredComponentBG}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
>
<View style={[styles.flex1, styles.justifyContentBetween, styles.sidebarLinkInner, styles.optionRow, styles.userSelectNone, isFocused && styles.sidebarLinkActive]}>
Expand Down
1 change: 0 additions & 1 deletion src/components/SelectionList/UserListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ function UserListItem({item, isFocused = false, showTooltip, onSelectRow, onDism
accessibilityState={{checked: item.isSelected}}
hoverDimmingValue={1}
hoverStyle={styles.hoveredComponentBG}
focusStyle={styles.hoveredComponentBG}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
>
<View style={styles.checkboxPressable}>
Expand Down
6 changes: 6 additions & 0 deletions src/hooks/useActiveElement/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {useEffect, useState} from 'react';

/**
* Listens for the focusin and focusout events and sets the DOM activeElement to the state.
* On native, we just return null.
*
* @return {Element} the active element in the DOM
*/
export default function useActiveElement() {
const [active, setActive] = useState(document.activeElement);

Expand Down
5 changes: 5 additions & 0 deletions src/hooks/useActiveElement/index.native.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* Native doesn't have the DOM, so we just return null.
*
* @return {null}
*/
export default function useActiveElement() {
return null;
}

0 comments on commit 38030f8

Please sign in to comment.