Skip to content

Adjust padding in new SelectionList #74362

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.

Sign up for GitHub

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

Merged
srikarparsi merged 6 commits into from
Nov 11, 2025
Merged

Adjust padding in new SelectionList #74362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
07f6812
Fix safe padding
zfurtak Nov 5, 2025
1f76b8f
Fix lint
zfurtak Nov 5, 2025
fe78a32
Fix lint
zfurtak Nov 5, 2025
18390df
Merge branch 'main' into @zfurtak/fix-padding
zfurtak Nov 6, 2025
b6d2ba8
Revert change
zfurtak Nov 6, 2025
56087c6
Adjust to review
zfurtak Nov 7, 2025
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
10 changes: 4 additions & 6 deletions src/components/SelectionList/BaseSelectionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ function BaseSelectionList<TItem extends ListItem>({
isLoadingNewOptions,
isRowMultilineSupported = false,
addBottomSafeAreaPadding,
includeSafeAreaPaddingBottom = true,
showListEmptyContent = true,
showLoadingPlaceholder,
showScrollIndicator = true,
Expand Down Expand Up @@ -100,10 +99,9 @@ function BaseSelectionList<TItem extends ListItem>({
[isSelected, selectedItems, canSelectMultiple],
);

const paddingBottomStyle = useMemo(
() => (!isKeyboardShown || !!footerContent) && includeSafeAreaPaddingBottom && safeAreaPaddingBottomStyle,
[footerContent, includeSafeAreaPaddingBottom, isKeyboardShown, safeAreaPaddingBottomStyle],
);
const paddingBottomStyle = useMemo(() => !isKeyboardShown && safeAreaPaddingBottomStyle, [isKeyboardShown, safeAreaPaddingBottomStyle]);

const hasFooter = !!footerContent || confirmButtonConfig?.showButton;

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Nov 6, 2025

Choose a reason for hiding this comment

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

❌ PERF-6 (docs)

This value derivation should be wrapped in useMemo with specific property dependencies to prevent unnecessary recalculations when unrelated confirmButtonConfig properties change.

Suggested fix:

const hasFooter = useMemo(() => footerContent ?? confirmButtonConfig?.showButton, [footerContent, confirmButtonConfig?.showButton]);

Copy link
Copy Markdown
Contributor

@aimane-chnaif aimane-chnaif Nov 7, 2025

Choose a reason for hiding this comment

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

I think it's fine. Not much performance benefit

const dataDetails = useMemo<DataDetailsType<TItem>>(() => {
const {disabledIndexes, disabledArrowKeyIndexes, selectedOptions} = data.reduce(
Expand Down Expand Up @@ -378,7 +376,7 @@ function BaseSelectionList<TItem extends ListItem>({

useImperativeHandle(ref, () => ({scrollAndHighlightItem, scrollToIndex}), [scrollAndHighlightItem, scrollToIndex]);
return (
<View style={[styles.flex1, !addBottomSafeAreaPadding && paddingBottomStyle, style?.containerStyle]}>
<View style={[styles.flex1, addBottomSafeAreaPadding && !hasFooter && paddingBottomStyle, style?.containerStyle]}>
{textInputComponent({shouldBeInsideList: false})}
{data.length === 0 ? (
renderListEmptyContent()
Expand Down
3 changes: 0 additions & 3 deletions src/components/SelectionList/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ type SelectionListProps<TItem extends ListItem> = {
/** Whether to add bottom safe area padding */
addBottomSafeAreaPadding?: boolean;

/** Whether to include padding bottom */
includeSafeAreaPaddingBottom?: boolean;

/** Whether to show the empty list content */
showListEmptyContent?: boolean;

Expand Down
1 change: 0 additions & 1 deletion src/pages/OnboardingEmployees/BaseOnboardingEmployees.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ function BaseOnboardingEmployees({shouldUseNativeStyles, route}: BaseOnboardingE
ListItem={RadioListItem}
footerContent={footerContent}
style={{listItemWrapperStyle: onboardingIsMediumOrLargerScreenWidth ? [styles.pl8, styles.pr8] : []}}
includeSafeAreaPaddingBottom={false}
/>
</ScreenWrapper>
);
Expand Down
Loading