Conversation
bogdandobritoiu
requested review from
atn4z7,
onghwan and
ruijialin-avalabs
as code owners
bogdandobritoiu
requested review from
B0Y3R-AVA and
alexnicolae-ava
as code owners
There was a problem hiding this comment.
Pull request overview
Introduces a new ListScreenV2 component built on Shopify’s FlashList to improve list performance and scrolling/keyboard interactions, and migrates the Swap V2 token selector screen to use it.
Changes:
- Added
ListScreenV2(FlashList-based) with animated header behavior, modal grabber support, and custom empty/footer handling. - Updated
SelectSwapV2TokenScreento useListScreenV2, including providingestimatedItemSizeand updating thekeyExtractor. - Simplified the token selector empty/loading rendering now that the new list screen handles centering/layout.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core-mobile/app/new/features/swapV2/screens/SelectSwapV2TokenScreen.tsx | Switches Swap V2 token selection to the new FlashList-based list screen and tweaks list configuration. |
| packages/core-mobile/app/new/common/components/ListScreenV2.tsx | Adds a new reusable list-screen abstraction using FlashList with header/empty/footer + keyboard interaction logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| ...(props?.contentContainerStyle ?? {}), | ||
| paddingBottom: renderFooter ? footerHeight + 16 : 16 | ||
| } | ||
| }, [renderFooter, footerHeight, props?.contentContainerStyle]) |
There was a problem hiding this comment.
contentContainerStyle.paddingBottom doesn’t account for safe-area bottom inset or keyboard height. Compared to ListScreen, this can cause the last items to be obscured by the home indicator (when no footer) and/or the keyboard (when search is focused), conflicting with the PR’s keyboard-scrolling requirements. Consider computing paddingBottom similarly to ListScreen (keyboard height when visible, otherwise insets.bottom + 16, plus any footer height).
| return { | |
| ...(props?.contentContainerStyle ?? {}), | |
| paddingBottom: renderFooter ? footerHeight + 16 : 16 | |
| } | |
| }, [renderFooter, footerHeight, props?.contentContainerStyle]) | |
| const basePaddingBottom = keyboard.isVisible | |
| ? keyboard.height | |
| : insets.bottom + 16 | |
| const footerPaddingBottom = renderFooter ? footerHeight : 0 | |
| return { | |
| ...(props?.contentContainerStyle ?? {}), | |
| paddingBottom: basePaddingBottom + footerPaddingBottom | |
| } | |
| }, [ | |
| footerHeight, | |
| insets.bottom, | |
| keyboard.height, | |
| keyboard.isVisible, | |
| props?.contentContainerStyle, | |
| renderFooter | |
| ]) |
| offset: | ||
| event.nativeEvent.contentOffset.y > contentHeaderHeight | ||
| ? event.nativeEvent.contentOffset.y | ||
| : contentHeaderHeight, |
There was a problem hiding this comment.
In onScrollEndDrag, the conditional event.nativeEvent.contentOffset.y > contentHeaderHeight ? ... : contentHeaderHeight is unreachable because this block only runs when contentOffset.y < contentHeaderHeight. This can be simplified to always scroll to contentHeaderHeight, which makes the intent clearer and avoids dead code.
| offset: | |
| event.nativeEvent.contentOffset.y > contentHeaderHeight | |
| ? event.nativeEvent.contentOffset.y | |
| : contentHeaderHeight, | |
| offset: contentHeaderHeight, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const overrideProps = useMemo(() => { | ||
| return { | ||
| contentContainerStyle: { | ||
| ...contentContainerStyle, | ||
| minHeight | ||
| } | ||
| } | ||
| }, [contentContainerStyle, minHeight]) |
There was a problem hiding this comment.
The comment above says FlashList contentContainerStyle should not receive unsupported props like minHeight, but overrideProps later adds minHeight into a contentContainerStyle object. Either update the comment to explain why overrideProps.contentContainerStyle is safe/intentional, or avoid passing minHeight this way to prevent confusion and potential unsupported-style issues.
| <FlashList | ||
| data={data} | ||
| ref={scrollViewRef} | ||
| renderScrollComponent={RenderScrollComponent} | ||
| onScroll={onScrollEvent} | ||
| onScrollEndDrag={onScrollEndDrag} | ||
| keyboardDismissMode="interactive" | ||
| keyboardShouldPersistTaps="handled" | ||
| showsVerticalScrollIndicator={false} | ||
| overrideProps={overrideProps} | ||
| contentContainerStyle={contentContainerStyle} | ||
| {...props} |
There was a problem hiding this comment.
{...props} is spread after onScroll, onScrollEndDrag, contentContainerStyle, and overrideProps. If a caller passes any of these props, it will override the internal handlers/styles and can break the fading header behavior and keyboard/list layout assumptions. Consider moving {...props} earlier (before the internal props) or explicitly omitting the props you must control from the spread.
3 participants
Description
A new version of ListScreen with FlashList.
Simulator.Screen.Recording.-.26.-.2026-02-13.at.21.39.33.mov
Testing
Dev Testing (if applicable)
iOS - 7468
Android - 7469
Checklist
Please check all that apply (if applicable)