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

fix: Workspace switcher - Selection highlight moves to second item when searching for workspace. #37577

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
17 changes: 11 additions & 6 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ function BaseSelectionList<TItem extends ListItem>(
listHeaderWrapperStyle,
isRowMultilineSupported = false,
textInputRef,
shouldShowTextInput = true,
textInputIconLeft,
contentContainerStyles,
}: BaseSelectionListProps<TItem>,
ref: ForwardedRef<SelectionListHandle>,
) {
Expand All @@ -76,7 +79,7 @@ function BaseSelectionList<TItem extends ListItem>(
const listRef = useRef<RNSectionList<TItem, Section<TItem>>>(null);
const innerTextInputRef = useRef<RNTextInput | null>(null);
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const shouldShowTextInput = !!textInputLabel;
const showTextInput = !!textInputLabel || shouldShowTextInput;
const shouldShowSelectAll = !!onSelectAll;
const activeElementRole = useActiveElementRole();
const isFocused = useIsFocused();
Expand Down Expand Up @@ -240,15 +243,15 @@ function BaseSelectionList<TItem extends ListItem>(

onSelectRow(item);

if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
if (showTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
innerTextInputRef.current.focus();
}
};

const selectAllRow = () => {
onSelectAll?.();

if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
if (showTextInput && shouldPreventDefaultFocusOnSelectRow && innerTextInputRef.current) {
innerTextInputRef.current.focus();
}
};
Expand Down Expand Up @@ -374,7 +377,7 @@ function BaseSelectionList<TItem extends ListItem>(
/** Focuses the text input when the component comes into focus and after any navigation animations finish. */
useFocusEffect(
useCallback(() => {
if (shouldShowTextInput) {
if (showTextInput) {
focusTimeoutRef.current = setTimeout(() => {
if (!innerTextInputRef.current) {
return;
Expand All @@ -388,7 +391,7 @@ function BaseSelectionList<TItem extends ListItem>(
}
clearTimeout(focusTimeoutRef.current);
};
}, [shouldShowTextInput]),
}, [showTextInput]),
);

const prevTextInputValue = usePrevious(textInputValue);
Expand Down Expand Up @@ -470,7 +473,7 @@ function BaseSelectionList<TItem extends ListItem>(
<SafeAreaConsumer>
{({safeAreaPaddingBottomStyle}) => (
<View style={[styles.flex1, !isKeyboardShown && safeAreaPaddingBottomStyle, containerStyle]}>
{shouldShowTextInput && (
{showTextInput && (
<View style={[styles.ph4, styles.pb3]}>
<TextInput
ref={(element) => {
Expand Down Expand Up @@ -498,6 +501,7 @@ function BaseSelectionList<TItem extends ListItem>(
blurOnSubmit={!!flattenedSections.allOptions.length}
isLoading={isLoadingNewOptions}
testID="selection-list-text-input"
iconLeft={textInputIconLeft}
/>
</View>
)}
Expand Down Expand Up @@ -549,6 +553,7 @@ function BaseSelectionList<TItem extends ListItem>(
onLayout={onSectionListLayout}
style={(!maxToRenderPerBatch || isInitialSectionListRender) && styles.opacity0}
ListFooterComponent={ShowMoreButtonInstance}
contentContainerStyle={contentContainerStyles}
/>
{children}
</>
Expand Down
10 changes: 10 additions & 0 deletions src/components/SelectionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type CONST from '@src/CONST';
import type {Errors, Icon, PendingAction} from '@src/types/onyx/OnyxCommon';
import type {ReceiptErrors} from '@src/types/onyx/Transaction';
import type ChildrenProps from '@src/types/utils/ChildrenProps';
import type IconAsset from '@src/types/utils/IconAsset';
import type RadioListItem from './RadioListItem';
import type TableListItem from './TableListItem';
import type UserListItem from './UserListItem';
Expand Down Expand Up @@ -172,6 +173,9 @@ type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {
/** Sections for the section list */
sections: Array<SectionListData<TItem, Section<TItem>>> | typeof CONST.EMPTY_ARRAY;

/** Content container styles for OptionsList */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** Content container styles for OptionsList */
/** Content container styles for section list */

contentContainerStyles?: StyleProp<ViewStyle> | undefined;

/** Default renderer for every item in the list */
ListItem: typeof RadioListItem | typeof UserListItem | typeof TableListItem;

Expand Down Expand Up @@ -297,6 +301,12 @@ type BaseSelectionListProps<TItem extends ListItem> = Partial<ChildrenProps> & {

/** Ref for textInput */
textInputRef?: MutableRefObject<TextInput | null>;

/** Left icon to display in TextInput */
textInputIconLeft?: IconAsset | null;

/** If false, the text input will not be shown at all. Defaults to true */
shouldShowTextInput?: boolean;
};

type SelectionListHandle = {
Expand Down
54 changes: 33 additions & 21 deletions src/pages/WorkspaceSwitcherPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import {MagnifyingGlass} from '@components/Icon/Expensicons';
import OptionRow from '@components/OptionRow';
import OptionsSelector from '@components/OptionsSelector';
// import OptionsSelector from '@components/OptionsSelector';
import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import UserListItem from '@components/SelectionList/UserListItem';
import Text from '@components/Text';
import Tooltip from '@components/Tooltip';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useAutoFocusInput from '@hooks/useAutoFocusInput';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useTheme from '@hooks/useTheme';
Expand Down Expand Up @@ -59,7 +60,6 @@ function WorkspaceSwitcherPage({policies}: WorkspaceSwitcherPageProps) {
const {isOffline} = useNetwork();
const [selectedOption, setSelectedOption] = useState<SimpleWorkspaceItem>();
const [searchTerm, setSearchTerm] = useState('');
const {inputCallbackRef} = useAutoFocusInput();
const {translate} = useLocalize();
const {activeWorkspaceID, setActiveWorkspaceID} = useActiveWorkspace();

Expand Down Expand Up @@ -141,8 +141,9 @@ function WorkspaceSwitcherPage({policies}: WorkspaceSwitcherPageProps) {
boldStyle: hasUnreadData(policy?.id),
keyForList: policy?.id,
isPolicyAdmin: PolicyUtils.isPolicyAdmin(policy),
isSelected: selectedOption?.policyID === policy?.id,
}));
}, [policies, getIndicatorTypeForPolicy, hasUnreadData, isOffline]);
}, [policies, getIndicatorTypeForPolicy, hasUnreadData, isOffline, selectedOption?.policyID]);

const filteredAndSortedUserWorkspaces = useMemo(
() =>
Expand Down Expand Up @@ -237,7 +238,8 @@ function WorkspaceSwitcherPage({policies}: WorkspaceSwitcherPageProps) {
</View>

{usersWorkspaces.length > 0 ? (
<OptionsSelector
<>
{/* <OptionsSelector
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
// @ts-expect-error TODO: remove this comment once OptionsSelector (https://github.com/Expensify/App/issues/25125) is migrated to TS
placeholder={translate('workspace.switcher.placeholder')}
ref={inputCallbackRef}
Expand All @@ -259,27 +261,37 @@ function WorkspaceSwitcherPage({policies}: WorkspaceSwitcherPageProps) {
textIconLeft={MagnifyingGlass}
// Null is to avoid selecting unfocused option when Global selected, undefined is to focus selected workspace
initiallyFocusedOptionKey={!activeWorkspaceID ? null : undefined}
/>
/> */}

<SelectionList
textInputPlaceholder={translate('workspace.switcher.placeholder')} // In place of placeholder prop of OptionsSelector
// ref={inputCallbackRef} Not required in SelectionList
sections={[usersWorkspacesSectionData]}
textInputValue={searchTerm} // In place of value prop of OptionsSelector
shouldShowTextInput={usersWorkspaces.length >= CONST.WORKSPACE_SWITCHER.MINIMUM_WORKSPACES_TO_SHOW_SEARCH} // New prop added
onChangeText={setSearchTerm}
// selectedOptions={selectedOption ? [selectedOption] : []} > Not in SelectionList > Not required > I added isSelected key to sections
onSelectRow={selectPolicy}
shouldPreventDefaultFocusOnSelectRow
headerMessage={headerMessage}
// highlightSelectedOptions > To add checkmark to seleted option > Not in SelectionList > Not required > BaseListItem adds the checkmark by default if item has isSelected
// shouldShowOptions > To show loader if false > Not in SelectionList > Not required since isLoading is false by default in SelectionList
// autoFocus={false} > Not in SelectionList > Not required > SelectionList handles text input focus
canSelectMultiple={false} // In place of canSelectMultipleOptions prop of OptionsSelector
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
// shouldShowSubscript={false} > Not in SelectionList > Also removed from OptionsSelector so no change needed for this
shouldShowTooltips={false} // In place of showTitleTooltip prop of OptionsSelector
contentContainerStyles={[styles.pt0, styles.mt0]} // New prop added
textInputIconLeft={MagnifyingGlass} // New prop added
initiallyFocusedOptionKey={activeWorkspaceID}
ListItem={UserListItem}
/>
</>
) : (
<WorkspaceCardCreateAWorkspace />
)}
</>
),
[
inputCallbackRef,
setSearchTerm,
searchTerm,
selectPolicy,
selectedOption,
styles,
theme.textSupporting,
translate,
usersWorkspaces.length,
usersWorkspacesSectionData,
activeWorkspaceID,
theme.icon,
headerMessage,
],
[setSearchTerm, searchTerm, selectPolicy, styles, theme.textSupporting, translate, usersWorkspaces.length, usersWorkspacesSectionData, activeWorkspaceID, theme.icon, headerMessage],
);

useEffect(() => {
Expand Down
Loading