feat: allow using updated filters + sorts in sale screen#13267
Open
brainbicycle wants to merge 11 commits intomainfrom
Open
feat: allow using updated filters + sorts in sale screen#13267brainbicycle wants to merge 11 commits intomainfrom
brainbicycle wants to merge 11 commits intomainfrom
Conversation
…/masonry-new-sale-lot
…or nested scroll support MasonryInfiniteScrollArtworkGrid was causing infinite scroll issues when used inside the parent FlatList (nested scroll context). Reverted to using the deprecated InfiniteScrollArtworksGridContainer which properly handles nested scrolling via ParentAwareScrollView and isCloseToBottom helper. Changes: - Replace MasonryInfiniteScrollArtworkGrid with InfiniteScrollArtworksGridContainer - Remove custom renderItem, handleOnEndReached, ListFooterComponent callbacks - Add InfiniteScrollArtworksGrid_connection fragment spread to GraphQL queries - Adapt loadMore function signature to handle callback parameter - Apply to both SaleLotsListLegacy and SaleLotsListNew implementations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…o results The New implementation was checking filtered counts instead of unfiltered counts to determine if the sale has any artworks. This caused the entire lots section to disappear when filters resulted in zero matches, instead of showing the FilteredArtworkGridZeroState with "Clear Filters" button. Changes: - Use unfilteredSaleArtworksConnection.counts.total for initial null check - Remove underscore prefix from unfilteredSaleArtworksConnection parameter - Remove debug console.logs - Now matches Legacy implementation behavior This ensures: - Sales with 0 artworks (unfiltered) → return null (hide section) - Sales with artworks but 0 filtered results → show zero state with clear filters Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add LegacySaleArtworks filter mode to conditionally hide filters that are not supported by the saleArtworksConnection query when the AREnableArtworksConnectionForAuction2 feature flag is disabled. When flag is OFF (legacy): - Only show: Sort, View As, Estimate Range - Plus aggregations: Artists, Medium - Hide: Colors, Framed, Price Range (unsupported) When flag is ON (new): - Show all filters (no change to existing behavior) This prevents users from selecting filters that would be silently ignored by the legacy query. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove the temporary demonstration test file that was used to verify the hooks migration. The test cases are already covered by the comprehensive SaleLotsListHooks.tests.tsx file. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add testID="sale-artworks-grid" to the InfiniteScrollArtworksGridContainer wrapper in both legacy and new implementations. Update tests to check for this testID instead of masonry-specific assertions. This provides a cleaner, more explicit way to verify the grid component renders correctly. All tests passing (6 passed). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
brainbicycle
commented
Feb 19, 2026
| filterOptionToDisplayConfigMap.estimateRange, | ||
| filterOptionToDisplayConfigMap.sort, | ||
| filterOptionToDisplayConfigMap.viewAs, | ||
| ] |
Contributor
Author
There was a problem hiding this comment.
this has been a silent bug for more than 2 years since we first introduced this feature flag 😬. Showing filters in the modal that are unsupported so just don't do anything.
brainbicycle
commented
Feb 19, 2026
| /> | ||
| ) : ( | ||
| <Flex px={2} testID="sale-artworks-grid"> | ||
| <InfiniteScrollArtworksGridContainer |
Contributor
Author
There was a problem hiding this comment.
reverted to use the deprecated InfiniteScrollArtworksGridContainer because pagination is broken in the MasonryInfiniteScrollArtworkGrid when not full screen + embedded in a parent scroll view, it would be nice to have a shared component that support this
Update test expectations to use "grid" instead of false for viewAs filter paramValue. This aligns with the ViewAsValues.Grid enum value which correctly equals "grid". The previous value of false was incorrect - ViewAsValues is defined as: - Grid = "grid" - List = "list" All tests passing (39 passed). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR resolves DIAM-18
Description
Refreshes a feature we attempted to release a long time ago, that is using newer filterArtworks connection on the Sale screen which has more filters and better sorting behavior (pushes closed lots to bottom).
See here and ticket for more context: https://artsy.slack.com/archives/C02BAQ5K7/p1768903042850919
cc @artsy/diamond-devs
android-ff-disabled-low.mov
android-ff-enabled.mov
ios-ff-disabled-low.mov
ios-ff-enabled-low.mov
PR Checklist
Follow-ups
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.