-
Notifications
You must be signed in to change notification settings - Fork 149
refactor: consolidate artist meta tag handling #15947
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
base: main
Are you sure you want to change the base?
Conversation
This is the first step in refactoring artist page meta tag management. Moving responsibility from the header to individual tab content components to enable dynamic meta tag updates (e.g., pagination). Changes: - Remove ArtistMetaFragmentContainer from ArtistApp - Remove ArtistMeta fragment from ArtistApp GraphQL query - Add integration tests to verify meta tag functionality Next steps will move meta tag responsibility to individual tabs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Move meta tag responsibility from header to individual tabs while preserving all existing functionality. Each tab now renders: - Core artist meta tags (og:nationality, birthyear, etc.) - ArtistStructuredData (JSON-LD schema) - Tab-specific meta information Changes: - Add ArtistMetaFragmentContainer to About, Artworks, and Auction Results tabs - Use semantic field aliases: artworksMeta, auctionResultsMeta - Keep core ArtistMeta using clean "meta" field name - Update tests to use correct field names - All existing ArtistMeta tests pass This enables dynamic meta tag updates (e.g., pagination) in future commits. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… tabs Enable dynamic meta tag updates for paginated content by replacing static Meta tags with PaginatedMetaTags component. Changes: - Add PaginatedMetaTags to ArtistWorksForSaleRoute (Artworks tab) - Add PaginatedMetaTags to ArtistAuctionResults (Auction Results tab) - Remove unused Meta and Title imports - Skip imageURL in PaginatedMetaTags to avoid data duplication (images already handled by ArtistMeta component) Now when users paginate through artworks or auction results, meta tags will automatically update with "- Page X" suffix and proper canonical URLs for SEO. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ArtistMeta now accepts pagination props and handles all meta tag logic internally: - Conditionally renders MetaTags or PaginatedMetaTags based on isPaginated prop - Eliminates need for separate meta tag components on each tab - Simplifies API: only one component import per tab Updated all artist tabs to use the streamlined ArtistMeta API. Removed outdated integration tests and canonical link test. All core ArtistMeta tests passing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The ArtistMeta fragment now includes additional fields (nationality, birthday, alternateNames, etc.) that were missing from the test fixture, causing TypeScript errors. Added allowlist comments for base64-encoded test IDs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ArtistMeta now requires additional fields (alternateNames, nationality, etc.) that were missing from the test fixtures, causing runtime errors. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
#3837 Bundle Size — 9.26MiB (-0.74%).e2050a0(current) vs 12fbf30 main#3836(baseline) Warning Bundle removed 1 duplicate package, 35 duplicate packages remaining – View duplicate packages Bundle metrics
Bundle size by type
Bundle analysis report Branch iskounen/chore/aritst-page-meta-... Project dashboard Generated by RelativeCI Documentation Report issue |
- Update ARTWORK_FILTER_PARAMS and AUCTION_RESULTS_FILTER_PARAMS to use snake_case format matching URL parameters - Add support for array parameters (e.g., attribution_class[0], organizations[1]) - Normalize array parameters to comma-separated values for canonical URLs - Add comprehensive tests for array parameter handling and snake_case conversion - Ensure consistent canonical URLs regardless of parameter order 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
src/Utils/filterParams.ts
Outdated
| for (const [baseKey, values] of Object.entries(arrayParams)) { | ||
| const sortedValues = [...values].sort() | ||
| filteredParams[baseKey] = sortedValues.join(",") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use qs or querystring for this vs doing all of this manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, things like Array.isArray instead of regex, etc.
Its not that big of a deal and totally non-blocking; just a good example of bot vs existing patterns and complexity vs simplicity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right! The qs library is already widely used in the codebase. 😂
I keep missing existing patterns and best practices when I work with Claude Code because I don't know about them - so I just make sure that its output is correct and commit.
I've built up a list of things that I ask Claude Code to check when I'm working in Gravity now (ex., make sure there aren't any named scopes or calculated fields you can use instead of making new queries) but I need to build up a similar list for Force and Eigen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣
@iskounen - that list yr keeping should go right in Gravity's CLAUDE.md file ;). And while yr at it, might as well spin one up for Force (if its not already there).
A great way to start is to ask claude to review package.json scripts and the docs folder (specifically best_practices) and ask it to add the CLAUDE file for you.
- Replace regex-based array parameter detection with qs.parse() - Use Array.isArray() instead of manual string parsing - Maintain same functionality while leveraging existing qs dependency - All existing tests pass unchanged Addresses code review feedback to use standard library instead of manual URL parameter parsing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…age parameter - Remove all filter parameters from canonical URLs for paginated content - Only include page=N parameter for pages greater than 1 - Remove unused filterParams utility and constants that were only used for canonical URL generation - This avoids SEO duplicate content issues while maintaining proper pagination Pages now generate: - Page 1: /artist/example (no parameters) - Page 2+: /artist/example?page=2 (only page parameter) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Problem: Artist tabs required importing both
ArtistMetaANDMetaTags/PaginatedMetaTags, causing duplicate canonical links and redundant imports.Solution: Refactored
ArtistMetato accept pagination props and handle all meta tag logic internally.Changes:
ArtistMetanow conditionally rendersMetaTagsorPaginatedMetaTagsbased on isPaginated propArtistMetaFragmentContainerimportAPI: