Skip to content

WS-2323 - Add Language Script Switch Button to New Navigation Dual Script Language Services [Do Not Merge]#13881

Open
pvaliani wants to merge 33 commits intolatestfrom
WS-2323-add-script-switching-new-navigation
Open

WS-2323 - Add Language Script Switch Button to New Navigation Dual Script Language Services [Do Not Merge]#13881
pvaliani wants to merge 33 commits intolatestfrom
WS-2323-add-script-switching-new-navigation

Conversation

@pvaliani
Copy link
Copy Markdown
Contributor

@pvaliani pvaliani commented Apr 7, 2026

Resolves JIRA: https://bbc.atlassian.net/browse/WS-2430

Summary

  • Adds the language script switching button support to the new Navigation bar for serbian, ukchina (articles only), zhongwen and uzbek
  • Script switch button has been refreshed according to updated UX designs
  • Adds minimal additional test coverage and updates snapshots

Code changes

  • See files changed

Testing

image image image image

Useful Links

@pvaliani pvaliani self-assigned this Apr 7, 2026
@pvaliani pvaliani marked this pull request as ready for review April 8, 2026 07:25
Copilot AI review requested due to automatic review settings April 8, 2026 07:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds/updates support for the language script switch button in the header when using the new Navigation, expanding it to additional dual-script services (including ukchina on article pages only) and refreshing the button styling to match updated UX designs.

Changes:

  • Enabled new Navigation for additional services (serbian, ukchina, uzbek, zhongwen) and updated header logic to handle ukchina “articles only”.
  • Updated ScriptLink to support new-navigation styling via a prop and added/updated snapshots/tests.
  • Adjusted legacy header brand styling to accommodate script switch layout when using the new Navigation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/app/legacy/containers/Header/index.test.jsx Adds assertions for ukchina script-link rendering on article vs home pages.
src/app/legacy/containers/Header/index.styles.ts Updates header brand styles to better lay out brand + script link for new nav.
src/app/legacy/containers/Header/index.jsx Adds ukchina page-type gating for new nav; passes new-nav flag to ScriptLink.
src/app/components/Navigation/config.ts Adds additional services to the “new nav” allowlist.
src/app/components/Header/ScriptLink/index.tsx Adds isNewNavigation prop and conditionally applies new styling.
src/app/components/Header/ScriptLink/index.test.tsx Adds a snapshot test for new-navigation styling.
src/app/components/Header/ScriptLink/index.styles.ts Introduces updated UX styles for script switch button in new nav.
src/app/components/Header/ScriptLink/snapshots/index.test.tsx.snap Updates snapshots for the new-navigation ScriptLink rendering.

Comment on lines +8 to 12
'serbian',
'tamil',
'telugu',
'ukchina',
'urdu',
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SERVICES_WITH_NEW_NAV now includes ukchina, but HeaderContainer explicitly disables the new nav for ukchina non-article pages. This means the constant no longer reliably represents “services that use the new nav”, yet it’s also used elsewhere (e.g. to set useNewNav=true when fetching navigation config). Consider refactoring to a shared predicate (e.g. shouldUseNewNav({ service, pageType, pagePath })) or splitting into separate lists so ukchina can remain “articles only” without affecting other consumers.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is only for articles on ukchina.

Comment on lines 28 to 32
const currentVariantIndex = pathPartsWithoutExtension.indexOf(
currentVariant as string,
);

pathPartsWithoutExtension[currentVariantIndex] = alternateVariant;
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentVariant can be null in RequestContext, and indexOf will return -1 if the variant segment isn’t present in the URL. In that case pathPartsWithoutExtension[currentVariantIndex] = alternateVariant writes to index -1 and the joined path won’t change, producing an incorrect href (no variant switch). Add a guard for currentVariantIndex < 0 (e.g. return null or derive the replacement segment more defensively).

Copilot uses AI. Check for mistakes.
'bengali',
'marathi',
'punjabi',
'serbian',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to remove these services before merging so that we don't release the new nav to them early.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just as a reminder to remove, i'll fix the conflicts once it's gone to preview and been swarmed)

@amoore108
Copy link
Copy Markdown
Contributor

amoore108 commented Apr 9, 2026

Thanks for the changes, it looks good 👍

The only thing I can see different to the existing behaviour is on smaller screens for the likes of Serbian. Here, the button pushes against the SVG for quite some time before it snaps on to the new line, whereas with the existing/old behaviour, the SVG gets smaller and smaller, leaving a gap between it and the button and then finally snaps onto a new line. Worth checking with UX on the expectation here?

@pvaliani
Copy link
Copy Markdown
Contributor Author

pvaliani commented Apr 9, 2026

Thanks for the changes, it looks good 👍

The only thing I can see different to the existing behaviour is on smaller screens for the likes of Serbian. Here, the button pushes against the SVG for quite some time before it snaps on to the new line, whereas with the existing/old behaviour, the SVG gets smaller and smaller, leaving a gap between it and the button and then finally snaps onto a new line. Worth checking with UX on the expectation here?

No worries, i've asked the question to see if we can get clarity on that as I did notice the shrinking too during manual testing - hopefully we'll get an answer soon. Thanks for the thorough review!

Copy link
Copy Markdown
Contributor

@amoore108 amoore108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a reminder to remove the services from the new nav array before merging. Once they are removed, worth just double checking that the old nav hasn't been affected by these changes too 👍

@pvaliani
Copy link
Copy Markdown
Contributor Author

pvaliani commented Apr 9, 2026

LGTM. Just a reminder to remove the services from the new nav array before merging. Once they are removed, worth just double checking that the old nav hasn't been affected by these changes too 👍

No worries. I believe we'll do the swarm testing on preview (with new nav services in the list so we can test) and the sub task in the ticket should remind to update the array before merge

Copy link
Copy Markdown
Contributor

@hotinglok hotinglok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spacing between the right edge of the button and the edge of the page doesn't match the designs for groups 1 and 2. Happy to approve if this is intentional.

Image Image

@pvaliani pvaliani changed the title WS-2323 - Add Language Script Switch Button to New Navigation Dual Script Language Services WS-2323 - Add Language Script Switch Button to New Navigation Dual Script Language Services [Do Not Merge] Apr 9, 2026
@pvaliani
Copy link
Copy Markdown
Contributor Author

pvaliani commented Apr 9, 2026

The spacing between the right edge of the button and the edge of the page doesn't match the designs for groups 1 and 2. Happy to approve if this is intentional.

Image Image

Good spot dude, i've fixed that now:
image

Feel free to have a look and double check its all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants