feat(DIAM-49): collections by filtered artworks #12788

araujobarret · 2025-09-26T12:21:38Z

This PR resolves DIAM-49

Description

Adds support to collections made by artworks with filters on Explore By. Initially, I started with the idea of reusing the entire CollectionsByCategory, but that made the code very 🍝, including if annotations in GraphQL. That's why I've added a new Route for this purpose and tried to reuse what was possible.

PR Checklist

I have tested my changes on the following platforms:
- Android.
- iOS.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos at least on Android, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

feat: support artworks with filters as collections in Explore By

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.

ArtsyOpenSource · 2025-09-26T12:24:54Z

This PR contains the following changes:

Cross-platform user-facing changes (feat: support artworks with filters as collections in Explore By - araujobarret)

Generated by 🚫 dangerJS against 16b59e1

src/app/store/config/EnvironmentModel.tsx

nickskalkin

Looks good to me! Also attaching artsy/metaphysics#7054 to give more context for other reviewers

Asking @MounirDhahri to review too since he was working on adding Collect screen

src/app/Scenes/CollectionsByFilter/CollectionsByFilterBody.tsx

src/app/Scenes/CollectionsByFilter/CollectionsByFilterChips.tsx

MounirDhahri · 2025-09-26T14:31:02Z

Thanks for the tag @nickskalkin looking into it now

Copilot

Pull Request Overview

This PR adds support for collections made by artworks with filters in the Explore By section. The implementation creates a new CollectionsByFilter route that renders artworks with applied filters rather than curated marketing collections, avoiding code complexity in the existing CollectionsByCategory implementation.

Creates new CollectionsByFilter route and components for filtered artwork collections
Refactors existing components to support both marketing collections and filtered artworks
Adds comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/app/Scenes/CollectionsByFilter/*	New components for filtered artwork collections functionality
src/app/Scenes/Search/components/ExploreByCategory/ExploreByCategoryCard.tsx	Updated to use dynamic href from GraphQL instead of hardcoded path
src/app/Scenes/CollectionsByCategory/hooks/useCollectionsByCategoryParams.ts	Extracted shared route parameter logic for reuse
src/app/Scenes/CollectionsByCategory/*.tsx	Refactored to use shared parameter hook and updated component structure
src/app/Scenes/Collect/Collect.tsx	Enhanced to support dynamic title/subtitle parameters
src/app/Navigation/routes.tsx	Added new CollectionsByFilter route configuration

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/app/Scenes/CollectionsByFilter/CollectionsByFilterListItem.tsx

src/app/Scenes/CollectionsByCategory/CollectionsChips.tsx

Copilot · 2025-09-26T15:21:51Z

src/app/Scenes/CollectionsByFilter/CollectionsByFilterChips.tsx

+  const collections = extractNodes(data.chipsFilteredCollections)
+
+  const numRows = !isTablet() ? 3 : 2
+  const numColumns = Math.ceil(collections.length / 3)


Hard-coded magic number 3 should be replaced with the numRows variable for consistency and maintainability.

Suggested change

const numColumns = Math.ceil(collections.length / 3)

const numColumns = Math.ceil(collections.length / numRows)

Copilot · 2025-09-26T15:21:51Z

src/app/Scenes/CollectionsByFilter/CollectionsByFilterBody.tsx

+    return (
+      <>
+        <Separator borderColor="mono10" mt={4} />
+        {Array.from({ length: total - filtersForArtworksConnection.length }).map((_, i) => (


This calculation can result in a negative length if filtersForArtworksConnection.length exceeds total, which would cause an error. Add a Math.max(0, ...) wrapper to prevent negative values.

Suggested change

{Array.from({ length: total - filtersForArtworksConnection.length }).map((_, i) => (

{Array.from({ length: Math.max(0, total - filtersForArtworksConnection.length) }).map((_, i) => (

MounirDhahri

Although the code might look weird with all the ifs to support different queries returning the same content, I think it would still be less work than maintaining two components that do the same thing. Maybe we can look into consolidating both into a single source of true from MP.

I lack the full context of this, so I don't feel confident to approve this (because we already have the code) or reject this, so I will leave to you to decide on how to continue with this.

src/app/Scenes/CollectionsByFilter/CollectionsByFilter.tsx

MounirDhahri · 2025-09-26T15:15:45Z

src/app/Scenes/CollectionsByFilter/CollectionsByFilterBody.tsx

+  const header = useMemo(() => {
+    if (!data?.discoveryCategoryConnection) {
+      return null
+    }
+
+    return (
+      <>
+        <Flex gap={2}>
+          <Text variant="xl" px={2}>
+            {title}
+          </Text>
+          <Text px={2}>Explore collections by {title.toLowerCase()}</Text>
+
+          <CollectionsByFilterChips discoveryCategories={data.discoveryCategoryConnection} />
+        </Flex>
+
+        <Separator borderColor="mono10" my={4} />
+      </>
+    )
+  }, [data?.discoveryCategoryConnection, title])


can we extract this and other components to /Components

src/app/Scenes/CollectionsByFilter/CollectionsByFilterChips.tsx

araujobarret · 2025-09-29T14:14:55Z

@MounirDhahri, after your comment, it made sense to expand the MP query to return distinct types since we're dealing with 2 distinct results for the same query, and they need to be together(curated collections and fake collections(artworks with some filters)). I also had this question in mind. So, I will add a different query that supports this idea bc it's a breaking change.

MounirDhahri · 2025-09-30T13:23:15Z

Sorry for the late comment and thanks for looking further into this.

Assuming we are speaking of the new interface, since we are still not yet live with the app and are not using this anywhere, it should theoretically be safe to making a breaking change here

test: fix broken test and refactor the structure for a flatter test tree fix: remove test change test: fix explore by missing hrefs feat: remove extra route, incorporate the union type to render artworks with filter on the existing path

araujobarret · 2025-10-09T11:32:59Z

Alright, refactored the code and reused the existing route with the new Union type provided by MP artsy/metaphysics#7143

Noticed the prefetching of the Search screen was incomplete, added DiscoverSomethingNew and ExploreBy to it ⬆️

MounirDhahri

thank you! this looks great. Also thanks for moving the files to /components

araujobarret requested review from a team, iskounen and nickskalkin September 26, 2025 12:21

araujobarret self-assigned this Sep 26, 2025

artsy-peril bot added the Jira Synced label Sep 26, 2025

araujobarret changed the title ~~feat(DIAM-59): collections by filtered artworks~~ feat(DIAM-49): collections by filtered artworks Sep 26, 2025

nickskalkin requested a review from MounirDhahri September 26, 2025 13:25

nickskalkin reviewed Sep 26, 2025

View reviewed changes

src/app/store/config/EnvironmentModel.tsx Outdated Show resolved Hide resolved

nickskalkin reviewed Sep 26, 2025

View reviewed changes

MounirDhahri requested a review from Copilot September 26, 2025 15:20

Copilot AI reviewed Sep 26, 2025

View reviewed changes

MounirDhahri reviewed Sep 26, 2025

View reviewed changes

feat: collections by filtered artworks

6b45f8a

test: fix broken test and refactor the structure for a flatter test tree fix: remove test change test: fix explore by missing hrefs feat: remove extra route, incorporate the union type to render artworks with filter on the existing path

araujobarret force-pushed the araujobarret/dia-1383/explore-by-price-categories branch from 00836f1 to c0e52b2 Compare October 9, 2025 11:31

feat: prefetch discoverSomethingNew and ExploreBy on the Search screen

16b59e1

araujobarret force-pushed the araujobarret/dia-1383/explore-by-price-categories branch from c0e52b2 to 16b59e1 Compare October 9, 2025 11:35

MounirDhahri approved these changes Oct 10, 2025

View reviewed changes

araujobarret merged commit ce328b9 into main Oct 13, 2025
7 checks passed

araujobarret deleted the araujobarret/dia-1383/explore-by-price-categories branch October 13, 2025 13:20

	const numColumns = Math.ceil(collections.length / 3)
	const numColumns = Math.ceil(collections.length / numRows)

	{Array.from({ length: total - filtersForArtworksConnection.length }).map((_, i) => (
	{Array.from({ length: Math.max(0, total - filtersForArtworksConnection.length) }).map((_, i) => (

feat(DIAM-49): collections by filtered artworks #12788

feat(DIAM-49): collections by filtered artworks #12788

Uh oh!

Conversation

araujobarret commented Sep 26, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

Uh oh!

ArtsyOpenSource commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR contains the following changes:

Uh oh!

Uh oh!

nickskalkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MounirDhahri commented Sep 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

MounirDhahri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MounirDhahri Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

araujobarret commented Sep 29, 2025

Uh oh!

MounirDhahri commented Sep 30, 2025

Uh oh!

araujobarret commented Oct 9, 2025

Uh oh!

MounirDhahri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

araujobarret commented Sep 26, 2025 •

edited by atlassian bot

Loading

ArtsyOpenSource commented Sep 26, 2025 •

edited

Loading