-
Notifications
You must be signed in to change notification settings - Fork 149
feat: consolidate artist app meta tags #16616
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: consolidate artist app meta tags #16616
Conversation
#6017 Bundle Size — 9.53MiB (+2.07%).6fcbe65(current) vs c74b2b5 main#6013(baseline) Warning Bundle contains 34 duplicate packages – View duplicate packages Warning Bundle introduced 2 new packages: @swc/helpers, regenerator-runtime – View changed packages Bundle metrics
Bundle size by type
Bundle analysis report Branch iskounen/feat/consolidate-artist... Project dashboard Generated by RelativeCI Documentation Report issue |
| const blurb = artist.biographyBlurbPlain?.text?.substring(0, 70) || "" | ||
| return blurb ? `${baseDescription} ${blurb}` : baseDescription | ||
| } | ||
| } |
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.
I'm not opposed to this living in Force — it's hard for me to remember the logic of why it was split out — but we should tear down the fields in Metaphysics if that's the case. Provided we ensure other projects aren't using them: https://github.com/artsy/metaphysics/blob/main/src/schema/v2/artist/meta.ts — I can't imagine Eigen has a use?
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.
I searched around our other repos and didn't find any clients using this, so it's on the chopping block.
anandaroop
left a comment
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.
Looks good to me, with one minor change below.
This PR modifies ArtistMeta so that it can provide the correct set of
<title>,<meta>and<link>tags for each of the artist page sub-routes: /articles, /cv, /series and /shows.match.location.pathnameto determine the page that it is being rendered on, and renders the correct title, description and canonical URL for each one.metaGraphQL field and starts managing the titles and descriptions for each sub-route for itself.