refactor(DIA-1019): pass a component to FancySwiperCard instead of JSX #11439
Conversation
| const artworkCards: React.ReactNode[] = useMemo( | ||
| () => artworks.map((artwork, index) => <ArtQuizArtworkCard artwork={artwork} key={index} />), | ||
| [artworks] | ||
| ) |
There was a problem hiding this comment.
I memoized this because the artwork cards were being recalculated on each swipe and causing the screen to flicker. This stopped the flickering.
There was a problem hiding this comment.
Makes sense. I was gonna ask about performance but this makes sense!
| @@ -69,6 +59,10 @@ export const InfiniteDiscovery: React.FC = () => { | |||
| }) | |||
| }, []) | |||
|
|
|||
| const artworkCards: React.ReactNode[] = useMemo(() => { | |||
| return artworks.map((artwork, i) => <InfiniteDiscoveryArtworkCard artwork={artwork} key={i} />) | |||
| }, [artworks]) | |||
There was a problem hiding this comment.
Memoizing this mapping stopped the flickering on each swipe, but there is still a flicker when a new batch of artworks is retrieved. (You can see this flicker after the 3rd and 8th swipe in the Infinite Discovery video in the description.)
I need to find a way to not recalculate the existing ones...
| const { color } = useTheme() | ||
| const { width: screenWidth } = useScreenDimensions() | ||
|
|
||
| const src = !!artwork?.images?.[0]?.url ? artwork.images[0].url : undefined |
There was a problem hiding this comment.
#superminor - There's a couple simpler ways to do this kind of thing:
If its undefined, its undefined; no need to use a ternary and then assignment:
const src = artwork?.images?.[0]?.url
| const { width: screenWidth } = useScreenDimensions() | ||
|
|
||
| const src = !!artwork?.images?.[0]?.url ? artwork.images[0].url : undefined | ||
| const width = !!artwork?.images?.[0]?.width ? artwork.images[0].width : 0 |
There was a problem hiding this comment.
Ditto here -- can use the nullish operator:
const width = artwork?.images?.[0]?.width ?? 0
|
|
||
| const src = !!artwork?.images?.[0]?.url ? artwork.images[0].url : undefined | ||
| const width = !!artwork?.images?.[0]?.width ? artwork.images[0].width : 0 | ||
| const height = !!artwork?.images?.[0]?.height ? artwork.images[0].height : 0 |
| <Flex backgroundColor={color("white100")} width="100%" height={800}> | ||
| <EntityHeader | ||
| name={artwork?.artistNames ?? ""} | ||
| meta={artwork?.artists?.[0]?.formattedNationalityAndBirthday ?? undefined} |
There was a problem hiding this comment.
does this stuff not just fall back to undefined? Like do we need to explicitly define ?? undefined (or ?? "" -- ie, fallbacks ?
This PR refactors FancySwiper to receive an array of React components instead of JSX. I wanted to make this change because I will want to reuse the
useSaveArtworkhook to allow users to save artworks, and I can't use that hook for each artwork card unless it is inside of a component.InfiniteDiscovery.mp4
ArtTasteQuiz.mp4
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.