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

feat(ExploreBy): address bugs, layout issues and relay fetching issues #11008

Merged
merged 2 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
86 changes: 54 additions & 32 deletions src/app/Scenes/CollectionsByCategory/Body.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Flex, Skeleton, SkeletonText, Text, useSpace } from "@artsy/palette-mobile"
import { Flex, Separator, Skeleton, SkeletonText, Text, useSpace } from "@artsy/palette-mobile"
import { useRoute } from "@react-navigation/native"
import { FlashList } from "@shopify/flash-list"
import { BodyCollectionsByCategoryQuery } from "__generated__/BodyCollectionsByCategoryQuery.graphql"
import { BodyCollectionsByCategory_marketingCollection$key } from "__generated__/BodyCollectionsByCategory_marketingCollection.graphql"
import { CollectionsChips_marketingCollections$key } from "__generated__/CollectionsChips_marketingCollections.graphql"
import { BodyCollectionsByCategory_viewer$key } from "__generated__/BodyCollectionsByCategory_viewer.graphql"
import {
CollectionRailPlaceholder,
CollectionRailWithSuspense,
Expand All @@ -16,41 +16,56 @@ import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { graphql, useLazyLoadQuery, useFragment } from "react-relay"

interface BodyProps {
marketingCollections: BodyCollectionsByCategory_marketingCollection$key &
CollectionsChips_marketingCollections$key
viewer: BodyCollectionsByCategory_viewer$key
}

export const Body: React.FC<BodyProps> = ({ marketingCollections: _marketingCollections }) => {
export const Body: React.FC<BodyProps> = ({ viewer }) => {
const space = useSpace()
const marketingCollections = useFragment(fragment, _marketingCollections)
const data = useFragment(fragment, viewer)
const { params } = useRoute<CollectionsByCategoriesRouteProp>()
const category = params.props.category

if (!marketingCollections) {
if (!data?.marketingCollections) {
return null
}

return (
<Flex px={2} gap={space(4)}>
<Text variant="xl">{category}</Text>
<Flex gap={space(4)}>
<Text variant="xl" px={2}>
{category}
</Text>

<Flex gap={space(1)}>
<Flex px={2} gap={space(2)}>
<Text>Explore collections with {category}</Text>
<CollectionsChips marketingCollections={_marketingCollections} />
{/* TODO: fix typings broken by some unknown reason here, prob related to @plural */}
<CollectionsChips marketingCollections={data.marketingCollections as any} />
</Flex>

{marketingCollections.map((collection) => {
const slug = collection?.slug ?? ""
return <CollectionRailWithSuspense key={`artwork_rail_${slug}`} slug={slug} />
})}
<Separator borderColor="black10" />

<FlashList
estimatedItemSize={ESTIMATED_ITEM_SIZE}
data={data.marketingCollections}
keyExtractor={(item) => `artwork_rail_${item?.slug}`}
renderItem={({ item }) => {
return <CollectionRailWithSuspense slug={item?.slug ?? ""} />
}}
ItemSeparatorComponent={() => <Separator borderColor="black10" my={4} />}
/>
</Flex>
)
}

const ESTIMATED_ITEM_SIZE = 390

const fragment = graphql`
fragment BodyCollectionsByCategory_marketingCollection on MarketingCollection
@relay(plural: true) {
slug @required(action: NONE)
fragment BodyCollectionsByCategory_viewer on Viewer
@argumentDefinitions(category: { type: "String" }) {
marketingCollections(category: $category, first: 20) {
...CollectionsChips_marketingCollections

slug @required(action: NONE)
}
}
`

Expand All @@ -59,42 +74,49 @@ const BodyPlaceholder: React.FC = () => {

return (
<Skeleton>
<Flex px={2} gap={space(2)}>
<SkeletonText variant="xl">Category</SkeletonText>
<Flex gap={space(4)}>
<Flex px={2}>
<SkeletonText variant="xl">Category</SkeletonText>
</Flex>

<Flex gap={space(1)}>
<Flex gap={space(1)} px={2}>
<SkeletonText>Category description text</SkeletonText>

<CollectionsChipsPlaceholder />
</Flex>

<Separator borderColor="black10" />

<CollectionRailPlaceholder />
</Flex>
</Skeleton>
)
}

const query = graphql`
query BodyCollectionsByCategoryQuery($category: String!) {
marketingCollections(category: $category, first: 20) {
...BodyCollectionsByCategory_marketingCollection
...CollectionsChips_marketingCollections
query BodyCollectionsByCategoryQuery($category: String!) @cacheable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joeyAghion added the @cacheable here since, in this first part we have no custom-user-data being fetched, we only use to display the chips collections list and to get each collection slug.

viewer {
...BodyCollectionsByCategory_viewer @arguments(category: $category)
}
}
`

export const BodyWithSuspense = withSuspense({
Component: () => {
const { params } = useRoute<CollectionsByCategoriesRouteProp>()
const data = useLazyLoadQuery<BodyCollectionsByCategoryQuery>(query, {
category: params.props.entityID,
})

if (!data.marketingCollections) {
const data = useLazyLoadQuery<BodyCollectionsByCategoryQuery>(
Copy link
Member

Choose a reason for hiding this comment

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

👍

query,
{
category: params.props.entityID,
},
{ fetchPolicy: "store-and-network" }
)

if (!data.viewer) {
return <BodyPlaceholder />
}

return <Body marketingCollections={data.marketingCollections} />
return <Body viewer={data.viewer} />
},
LoadingFallback: BodyPlaceholder,
ErrorFallback: NoFallback,
Expand Down
60 changes: 36 additions & 24 deletions src/app/Scenes/CollectionsByCategory/CollectionRail.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
ArrowRightIcon,
Flex,
Separator,
Skeleton,
SkeletonText,
Spacer,
Expand All @@ -10,12 +11,12 @@ import { CollectionRailCollectionsByCategoryQuery } from "__generated__/Collecti
import { CollectionRail_marketingCollection$key } from "__generated__/CollectionRail_marketingCollection.graphql"
import { ArtworkRail, ArtworkRailPlaceholder } from "app/Components/ArtworkRail/ArtworkRail"
import { SectionTitle } from "app/Components/SectionTitle"
import { navigate } from "app/system/navigation/navigate"
import { ElementInView } from "app/utils/ElementInView"
import { extractNodes } from "app/utils/extractNodes"
import { withSuspense, NoFallback } from "app/utils/hooks/withSuspense"
import { useClientQuery } from "app/utils/useClientQuery"
import { FC, useState } from "react"
import { graphql, useFragment } from "react-relay"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"

interface CollectionRailProps {
collection: CollectionRail_marketingCollection$key
Expand All @@ -31,20 +32,29 @@ export const CollectionRail: FC<CollectionRailProps> = ({ collection: _collectio
const artworks = extractNodes(collection.artworksConnection)

return (
<Flex>
<SectionTitle
title={collection.title}
onPress={() => {}}
RightButtonContent={() => <ArrowRightIcon />}
<Flex px={2}>
<Flex justifyContent="center">
<SectionTitle
title={collection.title}
titleVariant="md"
onPress={() => navigate(`/collection/${collection.slug}`)}
/>
</Flex>
<ArtworkRail
onPress={(artwork) => navigate(artwork.href ?? "")}
artworks={artworks}
ListHeaderComponent={null}
showSaveIcon
showPartnerName
/>
<ArtworkRail artworks={artworks} ListHeaderComponent={null} />
</Flex>
)
}

const fragment = graphql`
fragment CollectionRail_marketingCollection on MarketingCollection {
title @required(action: NONE)
slug @required(action: NONE)

artworksConnection(first: 10) {
counts @required(action: NONE) {
Expand All @@ -66,23 +76,26 @@ const fragment = graphql`
export const CollectionRailPlaceholder: FC = () => {
return (
<Skeleton>
<Flex justifyContent="space-between" flexDirection="row">
<SkeletonText>Category title</SkeletonText>
<ArrowRightIcon />
</Flex>

<Spacer y={1} />

<Flex flexDirection="row">
<ArtworkRailPlaceholder />
<Flex px={2}>
<Flex justifyContent="space-between" flexDirection="row">
<SkeletonText variant="md">Category title</SkeletonText>
<ArrowRightIcon />
</Flex>

<Spacer y={1} />

<Flex flexDirection="row">
<ArtworkRailPlaceholder />
<Separator borderColor="black10" my={2} />
</Flex>
</Flex>
</Skeleton>
)
}

const query = graphql`
query CollectionRailCollectionsByCategoryQuery($slug: String!) {
marketingCollection(slug: $slug) {
query CollectionRailCollectionsByCategoryQuery($slug: String!, $isVisible: Boolean!) {
marketingCollection(slug: $slug) @include(if: $isVisible) {
...CollectionRail_marketingCollection

title
Expand All @@ -99,10 +112,9 @@ export const CollectionRailWithSuspense = withSuspense<CollectionRailWithSuspens
Component: ({ slug }) => {
const { height } = useScreenDimensions()
const [isVisible, setIsVisible] = useState(false)
const { data, loading } = useClientQuery<CollectionRailCollectionsByCategoryQuery>({
query,
variables: { slug },
skip: !isVisible,
const data = useLazyLoadQuery<CollectionRailCollectionsByCategoryQuery>(query, {
slug,
isVisible,
})

const handleOnVisible = () => {
Expand All @@ -111,7 +123,7 @@ export const CollectionRailWithSuspense = withSuspense<CollectionRailWithSuspens
}
}

if (loading || !data?.marketingCollection || !isVisible) {
if (!data?.marketingCollection || !isVisible) {
// We don't need to overfetch all rails at once, fetch as they become closer to be visible
return (
<ElementInView onVisible={handleOnVisible} visibilityMargin={-height}>
Expand Down
7 changes: 3 additions & 4 deletions src/app/Scenes/CollectionsByCategory/__tests__/Body.tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ jest.mock("app/Scenes/CollectionsByCategory/CollectionRail", () => ({

describe("Body", () => {
const { renderWithRelay } = setupTestWrapper<BodyHomeViewSectionCardsTestQuery>({
Component: ({ marketingCollections }) => <Body marketingCollections={marketingCollections} />,
Component: Body,
query: graphql`
query BodyHomeViewSectionCardsTestQuery {
marketingCollections(category: "category", first: 20) @required(action: NONE) {
...BodyCollectionsByCategory_marketingCollection
...CollectionsChips_marketingCollections
viewer @required(action: NONE) {
...BodyCollectionsByCategory_viewer @arguments(category: "category")
}
}
`,
Expand Down
13 changes: 4 additions & 9 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionCards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { useFeatureFlag } from "app/utils/hooks/useFeatureFlag"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useClientQuery } from "app/utils/useClientQuery"
import React from "react"
import { isTablet } from "react-native-device-info"
import { graphql, useFragment } from "react-relay"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"

interface HomeViewSectionCardsProps {
section: HomeViewSectionCards_section$key
Expand Down Expand Up @@ -107,9 +106,9 @@ const fragment = graphql`
`

const query = graphql`
query HomeViewSectionCardsQuery($id: String!) {
query HomeViewSectionCardsQuery($id: String!, $isEnabled: Boolean!) {
homeView {
section(id: $id) {
section(id: $id) @include(if: $isEnabled) {
...HomeViewSectionCards_section
}
}
Expand Down Expand Up @@ -147,11 +146,7 @@ export const HomeViewSectionCardsQueryRenderer = withSuspense<
>({
Component: ({ sectionID }) => {
const isEnabled = useFeatureFlag("AREnableMarketingCollectionsCategories")
const { data } = useClientQuery<HomeViewSectionCardsQuery>({
query,
variables: { id: sectionID },
skip: !isEnabled,
})
const data = useLazyLoadQuery<HomeViewSectionCardsQuery>(query, { id: sectionID, isEnabled })

if (!data?.homeView.section || !isEnabled) {
return null
Expand Down
14 changes: 6 additions & 8 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionCardsChips.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { useFeatureFlag } from "app/utils/hooks/useFeatureFlag"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useClientQuery } from "app/utils/useClientQuery"
import { FlatList, ScrollView } from "react-native"
import { isTablet } from "react-native-device-info"
import { graphql, useFragment } from "react-relay"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"

interface HomeViewSectionCardsChipsProps {
section: HomeViewSectionCardsChips_section$key
Expand Down Expand Up @@ -132,9 +131,9 @@ const HomeViewSectionCardsChipsPlaceholder: React.FC = () => {
}

const query = graphql`
query HomeViewSectionCardsChipsQuery($id: String!) {
query HomeViewSectionCardsChipsQuery($id: String!, $isEnabled: Boolean!) {
homeView {
section(id: $id) {
section(id: $id) @include(if: $isEnabled) {
...HomeViewSectionCardsChips_section
}
}
Expand All @@ -144,10 +143,9 @@ const query = graphql`
export const HomeViewSectionCardsChipsQueryRenderer: React.FC<SectionSharedProps> = withSuspense({
Component: ({ sectionID, index, ...flexProps }) => {
const isEnabled = useFeatureFlag("AREnableMarketingCollectionsCategories")
const { data } = useClientQuery<HomeViewSectionCardsChipsQuery>({
query,
variables: { id: sectionID },
skip: !isEnabled,
const data = useLazyLoadQuery<HomeViewSectionCardsChipsQuery>(query, {
id: sectionID,
isEnabled,
})

if (!data?.homeView.section || !isEnabled) {
Expand Down