Spikes/svg to external#14022
Draft
eagerterrier wants to merge 30 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This draft/spike PR removes the UK-only services (archive, cymrufyw, naidheachdan, newsround, scotland, sport) from the World Service codebase and migrates per-service brand logos from inline JSX BrandSVG objects under chameleonLogos/ to external brandLogo.svg files served from public/<service>/images/. The Brand component is reworked to render the logo via the shared Image component using the SVG URL, and integration test snapshots / helpers are updated accordingly.
Changes:
- Remove UK-only services from service configs, themes,
chameleonLogos, public asset tests, AT analytics,getStatsDestination, related test fixtures/snapshots, etc. - Replace inline
BrandSVGJSX (group/viewbox/ratio) with external SVG URLs, introducingbrand-logo-ratios.jsonand anisSvgmode on theImagecomponent; rewritepsammead-brandto consume an SVG URL. - Update integration tests, common helpers (
header.ts,footer.ts), and snapshots to assert on<img src=…brandLogo.svg>instead of inline<svg><path/></svg>; remove deleted services' test files and entries; add helper scripts to extract logos and generate ratios.
Reviewed changes
Copilot reviewed 210 out of 271 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/app/legacy/psammead/psammead-brand/src/index.jsx (+test) |
Replace styled BrandSvg with Image, accept SVG URL + aspectRatio. |
src/app/legacy/containers/Brand/index.jsx |
Build brand SVG URL from env config and read ratios from JSON. |
src/app/components/Image/{index.tsx,index.styles.tsx} |
Add isSvg prop with responsive SVG sizing and inline AMP styles. |
src/app/components/ThemeProvider/** (themes, mocks, loadable, test, getThemeConfig, chameleonLogos/) |
Drop brandSVG from each service theme + remove chameleonLogos helpers. |
src/app/models/types/{global,theming}.ts |
Drop UK-only services and BrandSVG/brandSVG from typings. |
src/app/lib/config/services/*, ws-nextjs-app/utilities/serviceConfigs/index.ts |
Remove UK-only service entries from runtime config and loadable maps. |
src/app/contexts/RequestContext/getStatsDestination/index.ts |
Drop UK-only switch arms; isUK param now unused. |
src/app/components/ATIAnalytics/**, src/app/components/LinkedData/**, MediaLoader/utils/getProducerFromServiceName*, TopicTags, RelatedTopics, Recommendations, EmbedConsentBanner tests |
Remove UK-only service test cases / mappings. |
src/app/legacy/containers/ArticleTimestamp/timeFormatTests/expectedFormats.json, src/public.test.ts, extractWorldServiceFromUrl/index.test.ts, toggles snapshots |
Remove UK-only service fixtures/entries. |
ws-nextjs-app/integration/**/__snapshots__/*.snap (many) |
Replace inline brand <path> snapshots with SVG URLs; update CSS class hashes. |
ws-nextjs-app/integration/common/{header,footer}.ts |
Assert on <img> inside #brandSvgHeader/Footer instead of raw <svg>. |
ws-nextjs-app/integration/pages/articles/{news,scotland}/{amp,canonical}.test.ts |
Delete now-removed test entry points. |
ws-nextjs-app/pages/[service]/articles/handleArticleRoute.ts |
Drop BBCScotland from passport-home overrides. |
scripts/{extract-brand-logos.js,generate-brand-logo-ratios.js,update-brand-logo-imports.js} |
One-off migration scripts to produce SVG files, ratio JSON, and rewrite logo modules. |
| const path = require("path"); | ||
|
|
||
| const publicDir = path.resolve(__dirname, "../public"); | ||
| const outputPath = path.resolve(__dirname, "brand-logo-ratios.json"); |
| {svg.group} | ||
| </BrandSvg> | ||
| height="30" | ||
| width={aspectRatio ? aspectRatio[0] * (30 / aspectRatio[1]) : 0} |
| SIMORGH_PUBLIC_STATIC_ASSETS_PATH, | ||
| } = getEnvConfig(); | ||
|
|
||
| export default \`\${SIMORGH_PUBLIC_STATIC_ASSETS_ORIGIN}\${SIMORGH_PUBLIC_STATIC_ASSETS_PATH}/${serviceName}/images/brandLogo.svg\`; |
Comment on lines
10
to
+13
| service: Services; | ||
| }; | ||
|
|
||
| const getStatsDestination = ({ isUK = true, env = 'test', service }: Props) => { | ||
| const getStatsDestination = ({ env = 'test', service }: Props) => { |
Comment on lines
+107
to
+122
| <Image | ||
| preload={linkId !== 'footer'} | ||
| fetchPriority={linkId !== 'footer' ? 'high' : 'low'} | ||
| lazyLoad={linkId === 'footer'} | ||
| src={svg} | ||
| alt={product} | ||
| focusable="false" | ||
| aria-hidden="true" | ||
| height="32" | ||
| > | ||
| {svg.group} | ||
| </BrandSvg> | ||
| height="30" | ||
| width={aspectRatio ? aspectRatio[0] * (30 / aspectRatio[1]) : 0} | ||
| style={{ marginLeft: '0.5px' }} | ||
| imageSrcSet={false} | ||
| isSvg | ||
| placeholder={false} | ||
| aspectRatio={aspectRatio} | ||
| /> |
Comment on lines
124
to
+192
| @@ -149,6 +152,15 @@ const Image = ({ | |||
| {...(srcSet && { srcSet: imgSrcSet })} | |||
| {...(imgSizes && { sizes: imgSizes })} | |||
| {...(preload && { 'data-hero': 'true' })} | |||
| {...(isSvg && { | |||
| style: { | |||
| aspectRatio: hasFixedAspectRatio | |||
| ? `${aspectRatioX} / ${aspectRatioY}` | |||
| : 'auto', | |||
| height: '24px', | |||
| marginLeft: '0', | |||
| }, | |||
| })} | |||
| /> | |||
| </> | |||
| ) : ( | |||
| @@ -177,6 +189,7 @@ const Image = ({ | |||
| hasFixedAspectRatio | |||
| ? styles.imageFixedAspectRatio | |||
| : styles.imageResponsiveRatio, | |||
| isSvg && styles.imageSvg, | |||
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
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.
draft