-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Replace OptionsSelector with SelectionList - part 1 #38994
Merged
danieldoglas
merged 21 commits into
Expensify:main
from
software-mansion-labs:filip-solecki/remove-options-selector-1
Apr 5, 2024
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
daeb721
Replace OptionsSelector with SelectionList
filip-solecki 8c372c5
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki 966d18e
Fix lint
filip-solecki 5849045
Fix failing tests
filip-solecki 908363b
CR fixes
filip-solecki 46bcacc
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki df13200
Merge fix
filip-solecki 8c0297b
Fix isSelected state in EditReportFieldDropdownPage
filip-solecki 50bc061
review fixes
filip-solecki b55a085
Remove autofocus
filip-solecki c8a23e4
Fix isSelected state for dropdown
filip-solecki 1d14541
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki d652ee0
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki c261ba1
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki 9c11da7
CR fixes
filip-solecki 3275787
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki 7f50fe9
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki 62528f3
Add textInputAutoFocus prop
filip-solecki aaeaa22
Merge branch 'main' into filip-solecki/remove-options-selector-1
filip-solecki f044d83
Remove autoFocus on WorkspaceSwitcher
filip-solecki 3c3e295
revert unnecessary change
filip-solecki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,8 +69,11 @@ function BaseSelectionList<TItem extends ListItem>( | |
listHeaderWrapperStyle, | ||
isRowMultilineSupported = false, | ||
textInputRef, | ||
textInputIconLeft, | ||
sectionTitleStyles, | ||
headerMessageStyle, | ||
shouldHideListOnInitialRender = true, | ||
textInputAutoFocus = true, | ||
}: BaseSelectionListProps<TItem>, | ||
ref: ForwardedRef<SelectionListHandle>, | ||
) { | ||
|
@@ -79,7 +82,7 @@ function BaseSelectionList<TItem extends ListItem>( | |
const listRef = useRef<RNSectionList<TItem, SectionWithIndexOffset<TItem>>>(null); | ||
const innerTextInputRef = useRef<RNTextInput | null>(null); | ||
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
const shouldShowTextInput = !!textInputLabel; | ||
const shouldShowTextInput = !!textInputLabel || !!textInputIconLeft; | ||
const shouldShowSelectAll = !!onSelectAll; | ||
const activeElementRole = useActiveElementRole(); | ||
const isFocused = useIsFocused(); | ||
|
@@ -310,7 +313,7 @@ function BaseSelectionList<TItem extends ListItem>( | |
// We do this so that we can reference the height in `getItemLayout` – | ||
// we need to know the heights of all list items up-front in order to synchronously compute the layout of any given list item. | ||
// So be aware that if you adjust the content of the section header (for example, change the font size), you may need to adjust this explicit height as well. | ||
<View style={[styles.optionsListSectionHeader, styles.justifyContentCenter]}> | ||
<View style={[styles.optionsListSectionHeader, styles.justifyContentCenter, sectionTitleStyles]}> | ||
<Text style={[styles.ph4, styles.textLabelSupporting]}>{section.title}</Text> | ||
</View> | ||
); | ||
|
@@ -377,6 +380,9 @@ function BaseSelectionList<TItem extends ListItem>( | |
/** Focuses the text input when the component comes into focus and after any navigation animations finish. */ | ||
useFocusEffect( | ||
useCallback(() => { | ||
if (!textInputAutoFocus) { | ||
return; | ||
} | ||
if (shouldShowTextInput) { | ||
focusTimeoutRef.current = setTimeout(() => { | ||
if (!innerTextInputRef.current) { | ||
|
@@ -391,7 +397,7 @@ function BaseSelectionList<TItem extends ListItem>( | |
} | ||
clearTimeout(focusTimeoutRef.current); | ||
}; | ||
}, [shouldShowTextInput]), | ||
}, [shouldShowTextInput, textInputAutoFocus]), | ||
); | ||
|
||
const prevTextInputValue = usePrevious(textInputValue); | ||
|
@@ -494,8 +500,12 @@ function BaseSelectionList<TItem extends ListItem>( | |
return; | ||
} | ||
|
||
// eslint-disable-next-line no-param-reassign | ||
textInputRef.current = element as RNTextInput; | ||
if (typeof textInputRef === 'function') { | ||
textInputRef(element as RNTextInput); | ||
} else { | ||
// eslint-disable-next-line no-param-reassign | ||
textInputRef.current = element as RNTextInput; | ||
} | ||
}} | ||
label={textInputLabel} | ||
accessibilityLabel={textInputLabel} | ||
|
@@ -508,14 +518,18 @@ function BaseSelectionList<TItem extends ListItem>( | |
inputMode={inputMode} | ||
selectTextOnFocus | ||
spellCheck={false} | ||
iconLeft={textInputIconLeft} | ||
onSubmitEditing={selectFocusedOption} | ||
blurOnSubmit={!!flattenedSections.allOptions.length} | ||
isLoading={isLoadingNewOptions} | ||
testID="selection-list-text-input" | ||
/> | ||
</View> | ||
)} | ||
{!!headerMessage && ( | ||
|
||
{/* If we are loading new options we will avoid showing any header message. This is mostly because one of the header messages says there are no options. */} | ||
{/* This is misleading because we might be in the process of loading fresh options from the server. */} | ||
{!isLoadingNewOptions && headerMessage && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {!isLoadingNewOptions && !!headerMessage && ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
<View style={headerMessageStyle ?? [styles.ph5, styles.pb5]}> | ||
<Text style={[styles.textLabel, styles.colorMuted]}>{headerMessage}</Text> | ||
</View> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.