Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CP Staging] fix(selection-list): scroll to top, cmd enter, tab key #25955

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/components/SelectionList/BaseSelectionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Button from '../Button';
import useLocalize from '../../hooks/useLocalize';
import Log from '../../libs/Log';
import OptionsListSkeletonView from '../OptionsListSkeletonView';
import useActiveElement from '../../hooks/useActiveElement';

const propTypes = {
...keyboardStatePropTypes,
Expand Down Expand Up @@ -49,6 +50,7 @@ function BaseSelectionList({
onConfirm,
showScrollIndicator = false,
showLoadingPlaceholder = false,
showConfirmButton = false,
isKeyboardShown = false,
}) {
const {translate} = useLocalize();
Expand All @@ -58,7 +60,7 @@ function BaseSelectionList({
const focusTimeoutRef = useRef(null);
const shouldShowTextInput = Boolean(textInputLabel);
const shouldShowSelectAll = Boolean(onSelectAll);
const shouldShowConfirmButton = Boolean(onConfirm);
const activeElement = useActiveElement();

/**
* Iterates through the sections and items inside each section, and builds 3 arrays along the way:
Expand Down Expand Up @@ -162,7 +164,7 @@ function BaseSelectionList({
}
}

listRef.current.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated});
listRef.current.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight});
};

const selectRow = (item, index) => {
Expand All @@ -176,6 +178,11 @@ function BaseSelectionList({
// 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);

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);
}
}
}

Expand Down Expand Up @@ -277,10 +284,18 @@ function BaseSelectionList({
};
}, [shouldDelayFocus, shouldShowTextInput]);

/** Selects row when pressing enter */
/** Selects row when pressing Enter */
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, {
captureOnInputs: true,
shouldBubble: () => !flattenedSections.allOptions[focusedIndex],
isActive: !activeElement,
});

/** Calls confirm action when pressing CTRL (CMD) + Enter */
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.CTRL_ENTER, onConfirm, {
captureOnInputs: true,
shouldBubble: () => !flattenedSections.allOptions[focusedIndex],
isActive: Boolean(onConfirm),
});

return (
Expand Down Expand Up @@ -371,7 +386,7 @@ function BaseSelectionList({
/>
</>
)}
{shouldShowConfirmButton && (
{showConfirmButton && (
<FixedFooter>
<Button
success
Expand Down
30 changes: 22 additions & 8 deletions src/components/SelectionList/CheckboxListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import PressableWithFeedback from '../Pressable/PressableWithFeedback';
import styles from '../../styles/styles';
import Text from '../Text';
import {checkboxListItemPropTypes} from './selectionListPropTypes';
import Checkbox from '../Checkbox';
import Avatar from '../Avatar';
import OfflineWithFeedback from '../OfflineWithFeedback';
import CONST from '../../CONST';
import * as StyleUtils from '../../styles/StyleUtils';
import Icon from '../Icon';
import * as Expensicons from '../Icon/Expensicons';
import themeColors from '../../styles/themes/default';

function CheckboxListItem({item, isFocused = false, onSelectRow, onDismissError = () => {}}) {
const hasError = !_.isEmpty(item.errors);
Expand All @@ -32,13 +35,24 @@ function CheckboxListItem({item, isFocused = false, onSelectRow, onDismissError
hoverStyle={styles.hoveredComponentBG}
focusStyle={styles.hoveredComponentBG}
>
<Checkbox
accessibilityLabel={item.text}
disabled={item.isDisabled}
isChecked={item.isSelected}
onPress={() => onSelectRow(item)}
style={item.isDisabled ? styles.buttonOpacityDisabled : {}}
/>
<View
style={[
StyleUtils.getCheckboxContainerStyle(20, 4),
item.isSelected && styles.checkedContainer,
item.isSelected && styles.borderColorFocus,
item.isDisabled && styles.cursorDisabled,
item.isDisabled && styles.buttonOpacityDisabled,
]}
>
{item.isSelected && (
<Icon
src={Expensicons.Checkmark}
fill={themeColors.textLight}
height={14}
width={14}
/>
)}
</View>
{Boolean(item.avatar) && (
<Avatar
containerStyles={styles.pl5}
Expand Down
3 changes: 3 additions & 0 deletions src/components/SelectionList/selectionListPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ const propTypes = {

/** Whether to show the loading placeholder */
showLoadingPlaceholder: PropTypes.bool,

/** Whether to show the default confirm button */
showConfirmButton: PropTypes.bool,
};

export {propTypes, radioListItemPropTypes, checkboxListItemPropTypes};
25 changes: 25 additions & 0 deletions src/hooks/useActiveElement/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {useEffect, useState} from 'react';

export default function useActiveElement() {
Comment on lines +2 to +3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB Can we have a comment for the file?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thiagobrez Please include in the follow-up issues if the PRs gets merged

const [active, setActive] = useState(document.activeElement);

const handleFocusIn = () => {
setActive(document.activeElement);
};

const handleFocusOut = () => {
setActive(null);
};

useEffect(() => {
document.addEventListener('focusin', handleFocusIn);
document.addEventListener('focusout', handleFocusOut);

return () => {
document.removeEventListener('focusin', handleFocusIn);
document.removeEventListener('focusout', handleFocusOut);
};
}, []);

return active;
}
3 changes: 3 additions & 0 deletions src/hooks/useActiveElement/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function useActiveElement() {
return null;
}
1 change: 1 addition & 0 deletions src/pages/workspace/WorkspaceInvitePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ function WorkspaceInvitePage(props) {
onChangeText={setSearchTerm}
headerMessage={headerMessage}
onSelectRow={toggleOption}
onConfirm={inviteUser}
showScrollIndicator
shouldDelayFocus
/>
Expand Down