Create/update skills to support migration and document a plan

[andrewscfc]

Summary generated by AI, edited by @andrewscfc

Summary

Adds the AI tooling to carry out the SCSS Modules migration, plus a plan for how the work is sequenced.

Stacked on #14073 — its changes appear in this branch but belong to that PR.

Changes

.github/skills/migrate-component-to-scss-modules/SKILL.md (new)
The step-by-step process for taking a component off Emotion: an Emotion-to-SCSS value lookup, triage checks for components that can't be migrated by a styling change alone, and per-batch verification steps.

.github/skills/convert-webcore-component/SKILL.md (updated)
This skill told agents to produce index.styles.ts files and Emotion css props. Left unchanged it would work against the standards merged in #14280, so the styling half is rewritten for SCSS Modules. TypeScript guidance is untouched.

docs/SCSS-Modules-Migration.mdx (new)
Sequencing plan for engineers — scope, batching principles, blocked components, and a dated starting batch order. Published to Storybook, and deliberately outside the paths agents read so it can hold planning detail without consuming their context.

src/app/components/ThemeProviderSCSSModules/themeTokens.scss (small fix)
gridWidths.scss existed but was never added to the @forward list alongside the other five token files, so theme.$gridWidths-* didn't resolve. Fixed at the source rather than documenting a workaround in the skill. Hadn't been noticed before because its only two Emotion consumers (Billboard, PortraitVideoCarousel) aren't migrated yet.

src/app/components/ThemeProviderSCSSModules/gridWidths.scss (small fix)
The values were unitless numbers, so every consumer had to remember to convert them. Every existing usage converted via pixelsToRem anyway, so this now bakes in rem at the source, matching how spacings.scss already works.

Worth discussing: should we add clsx?

Emotion's css={[base, cond && modifier]} silently drops falsy entries. className doesn't, so the direct translation renders class="link false" — a silent bug. This shape appears in 11 components.

The skills currently require a ternary with an empty-string fallback and say not to add a dependency, following our "avoid new dependencies" rule. I (@andrewscfc) lean towards adding clsx instead — ~200 bytes, and it removes the pitfall across ~30 migrations rather than relying on everyone getting the ternary right. If we agree, both skills need updating.

Notes

The blocked-component list and batch ordering were derived from the codebase rather than assumed. That turned up two components which look like easy early wins but aren't: MostRead/Label passes a css prop into a psammead component and uses a palette value as a runtime prop, and RelatedContentSection imports a legacy Emotion styled component (OptimoPromos/PromoItem/index.styles) directly rather than owning its own item styling.

[Copilot]

🟡 Changes recommended

The updated Skill documentation contains guidance that is internally inconsistent/misleading and should be corrected to avoid agents producing incorrect migrations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds/updates Copilot Skills to support the repo’s ongoing migration from Emotion to SCSS Modules, and introduces a Storybook-published migration sequencing plan for engineers. It also fixes a missing SCSS token forward so gridWidths.scss can be consumed via @scss/themeTokens.

Changes:

• Added a new migrate-component-to-scss-modules Skill with a conversion workflow and token mapping guidance.
• Updated the convert-webcore-component Skill to align styling guidance with SCSS Modules (instead of Emotion).
• Added docs/SCSS-Modules-Migration.mdx describing migration scope, batching principles, and an initial batch sequence; forwarded gridWidths.scss from themeTokens.scss.

File summaries

File	Description
src/app/components/ThemeProviderSCSSModules/themeTokens.scss	Forwards gridWidths.scss so grid width tokens are available via @scss/themeTokens.
docs/SCSS-Modules-Migration.mdx	Adds an engineer-facing sequencing plan for the SCSS Modules migration (scope, batching, blocked components, verification).
.github/skills/migrate-component-to-scss-modules/SKILL.md	New Skill documenting a step-by-step migration process and token mapping guidance.
.github/skills/convert-webcore-component/SKILL.md	Updates Skill guidance to use SCSS Modules + TypeScript rather than Emotion-based styling.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The changes are primarily documentation/tooling guidance plus a small, low-risk SCSS token export fix, with only a minor comment-level consistency tweak suggested.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[emilysaffron]

On the surface, im pro adding clsx, seems likely relying on everyone getting the ternary right wll result on us missing it somewhere across these migrations. How is it used in reality, what would the code look like?

[Louis-Matsika]

clsx is another dependency, but given how used we are to writing Emotion patterns like css={[styles.link, alignWithMargin && styles.alignWithMargin]}, the equivalent clsx(styles.link, alignWithMargin && styles.alignWithMargin) feels like a natural migration path. It also keeps components with multiple style modifiers much more readable when compared to using increasingly complex template literal compositions.

the similarity to our existing emotion patterns should also make migrations easier to review and help reduce migration-related bugs

[pvaliani]

I might be missing something, but does RelatedTopics not hit the same blockers as MostRead/Label? It imports the psammead SectionLabel, then passes it both css={styles.sectionLabel} and backgroundColor={GREY_2}.

I think that means the migration skill would stop when it reached batch 3. Recommendations looks to have the same pattern too:
https://github.com/bbc/simorgh/blob/02d4cb633e131de44d27873c773d943f26a3be58/src/app/components/Recommendations/index.tsx#L5-L4

Should both move into the blocked list?

[andrewscfc]

Good spot revised here: 76580b2
👍

[pvaliani]

This may already be captured in the comment but just for clarity, should “done” also include having no remaining css props or Emotion styled usage? The primitive components we have like Heading and Paragraph both contain inline css={{ ... }} without a direct Emotion import or an index.styles file, because the repo uses Emotion’s JSX runtime globally. It might be that the current checks could say we’re finished while Emotion is still required.

For example, in Heading

simorgh/src/app/components/Heading/index.tsx

Line 50 in 37e9dcc

css={{

It doesn't import@emotion or has an index.styles. file, but these css props still use Emotion because it is configured globally in our ts config:

simorgh/tsconfig.json

Line 17 in 837ffd7

"jsxImportSource": "@emotion/react",

[andrewscfc]

Another great spot! Revised here: 73ce350

[pvaliani]

Should verification include something that compiles the SCSS, such as a Storybook or our Next build? I don't know if Jest actually compiles the SCSS so wondered if tests could still pass anyway from a typo or invalid style sheet

[pvaliani]

looking further into this if it helps:

https://jestjs.io/docs/webpack#mocking-css-modules

[andrewscfc]

AI edited response (let me know if this reads okay, I put a fair bit of time into revising this but it'll probably never truly read like me but the content is better imo!)

You're right, and it's a fair concern — not just about Jest compiling the SCSS, but that this migration removes a capability. Emotion's css prop is resolved by its real runtime, which injects actual <style> tags into jsdom, so toHaveStyle assertions against Emotion components validate real computed CSS. Once a component moves to SCSS Modules, .module.scss imports are mocked with identity-obj-proxy (see jest.config.js) — every class name just proxies back to its own key as a string (e.g. styles.link → "link") — so that capability goes away for that component: no real stylesheet is loaded, and a typo or invalid selector can't fail a test.

I did consider adding real SCSS-compiling test infra to close that gap and got as far as a working prototype, but I decided against keeping it: it couldn't reach full parity with toHaveStyle anyway. Compiling a single .module.scss file in isolation gives you its raw declared rules, but not what a browser would actually render. For example, a value like color: var(--brand-link-color) would show up literally as that variable reference, not the actual resolved colour, because the custom property is only defined in a separate theme file that isn't part of compiling one component's stylesheet in isolation. It would've also been inconsistent with how the rest of the codebase's components are tested. Instead we're relying on the Next.js build (fails on invalid SCSS) and Storybook/Chromatic (catches visually broken output) to cover this, and testing behaviour via class/attribute presence (toHaveClass/toHaveAttribute) rather than computed style. I've updated Step 5 of the skill to say this explicitly rather than leave it implicit.

[andrewscfc]

bc44a57

[pvaliani]

I’m +1 on adding clsx. From what I understand so far it makes conditional classes easier to read and will safely ignores values like false or undefined. Also seems it makes it simpler to preserve any existing className passed into a component.

Thanks for putting this together @andrewscfc . Have left a few comments for consideration, apologies it took so long ✅ 🫡

[andrewscfc]

Thanks for the feedback folks, I'm pushing up the clsx addition to this PR, let me know what you think

[pvaliani]

Thanks for the feedback folks, I'm pushing up the clsx addition to this PR, let me know what you think

Looks good. It's reported as unused atm so do we need to silence that for a bit in our GH actions for Simorgh CI or are we planning to do a first migrated component in here as an example alongside the dep addition?

[andrewscfc]

Thanks for the feedback folks, I'm pushing up the clsx addition to this PR, let me know what you think

Looks good. It's reported as unused atm so do we need to silence that for a bit in our GH actions for Simorgh CI or are we planning to do a first migrated component in here as an example alongside the dep addition?

Yeah, I'm doing batch 1 in this PR that I'll finish off next week: #14371

I'll probably merge that into this and merge in one most likely