[MWPW-182991] Project Lingo: Language Banner #5269
Conversation
… locale config for bacom
sonawanesnehal3
requested review from
a team,
Ruchika4 and
drashti1712
as code owners
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
Commits |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
sukamat
requested review from
JasonHowellSlavin,
skumar09,
sukamat and
vhargrave
vhargrave
left a comment
vhargrave
left a comment
There was a problem hiding this comment.
In general , I think the approach is decent, however I think there's a lot of code that's been added to utils that wasn't necessary to solve the CLS problem.
Here's a proposal of how I'm thinking it could be refactored (completely untested, so be warned, and I haven't fixed other feedbacks like the cookie === 'us').
You'll probably also need to merge stage into your branch so you have the new getCountry method.
const getCookie = (name) => document.cookie
.split('; ')
.find((row) => row.startsWith(`${name}=`))
?.split('=')[1];
}
async function decorateLanguageBanner() {
const { languageBanner, locale, locales, marketsSource, env } = getConfig();
const enabled = PAGE_URL.searchParams.get('languageBanner') ?? getMetadata('language-banner') ?? languageBanner;
if (enabled !== 'on') return;
const pagePrefix = locale.prefix?.replace('/', '') || 'us';
if (getCookie('international') === pagePrefix) return;
const pageLang = locale.ietf.split('-')[0];
const cookie = getCookie('international');
const prefLang = (cookie && cookie !== 'us')
? (locales[cookie]?.ietf?.split('-')[0] || cookie.split('_')[0])
: navigator.language?.split('-')[0] || null;
let source = marketsSource;
if (env.name !== 'prod') {
const urlSource = PAGE_URL.searchParams.get('marketsSource');
if (urlSource && /^[a-zA-Z0-9-]+$/.test(urlSource)) source = urlSource;
}
const [marketsConfig, geoIpCode] = await Promise.all([
fetch(`${getFederatedContentRoot()}/federal/supported-markets/supported-markets${source ? `-${source}` : ''}.json`)
.then((r) => (r.ok ? r.json() : null))
.catch(() => null),
getCountry() || import('./geo.js').then((m) => m.default(true)),
]);
if (!geoIpCode || !marketsConfig) return;
const marketsForGeo = marketsConfig.data.filter((market) =>
market.supportedRegions.split(',').some((r) => r.trim().toLowerCase() === geoIpCode)
);
if (!marketsForGeo.length) return;
const pageMarket = marketsForGeo.find((m) => m.prefix === pagePrefix);
if (pageMarket) {
if (!prefLang || pageLang === prefLang) return;
const prefMarket = marketsForGeo.find((m) => m.lang === prefLang);
if (!prefMarket) return;
}
document.body.prepend(createTag('div', { class: 'language-banner' }));
const existingWrapper = document.querySelector('.feds-promo-aside-wrapper');
if (existingWrapper) {
existingWrapper.remove();
document.querySelector('.global-navigation')?.classList.remove('has-promo');
}
}
| .then((res) => (res.ok ? res.json() : null)) | ||
| .catch(() => null); | ||
|
|
||
| const { default: getAkamaiCode } = await import('./geo.js'); |
There was a problem hiding this comment.
probably makes sense to call getCountry first before trying to load this in.
| ]); | ||
|
|
||
| if (!geoIpCode || !marketsConfig) return; | ||
| const geoIp = geoIpCode.toLowerCase(); |
There was a problem hiding this comment.
the geoIpCode is already made lower case in the geo2 file no?
| if (marketsWithPriority.length) { | ||
| marketsWithPriority.sort((a, b) => a.priority - b.priority); | ||
| showBanner = true; | ||
| targetMarkets.push(...marketsWithPriority.map((item) => item.market)); |
There was a problem hiding this comment.
things like targetMarkets.push don't need to be happening already in utils imo. We should be doing the absolute mininmum to figure out if we're showing the banner or not. And then let the language-banner.js code handle the rest later.
|
|
||
| function getPreferredLanguage(locales) { | ||
| const cookie = getCookie('international'); | ||
| if (cookie && cookie !== 'us') { |
There was a problem hiding this comment.
what happens if the cookie is === 'us'?
| market.supportedRegions = market.supportedRegions.split(',').map((r) => r.trim().toLowerCase()); | ||
| }); | ||
|
|
||
| const pageMarket = marketsConfig.data.find((m) => m.prefix === (config.locale.prefix?.replace('/', '') || '')); |
There was a problem hiding this comment.
the market config for the United States will be 'us' . Don't think this is being handled properly here.
| } | ||
| } | ||
|
|
||
| const marketsWithPriority = []; |
There was a problem hiding this comment.
don't think the priority matters for showing the banner.
|
|
||
| function preloadMarketsConfig() { | ||
| const config = getConfig(); | ||
| const languageBannerEnabled = new URLSearchParams(window.location.search).get('languageBanner') ?? (getMetadata('language-banner') || config.languageBanner); |
There was a problem hiding this comment.
You can use the Page_URL.searchParams instead of creating a new URLSearchParams object.
|
Unit tests also seem to be failing - and merge conflicts need to be resolved |
|
This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label. |
6 participants
This PR adds the language banner feature with Project Lingo's language-matching UX. It provides language-mismatch banner to help users discover content in their preferred language.
Resolves: MWPW-182991
Related Documentations:
BACOM Supported Markets for Lingo Lang Matching
Figma for Flow diagram
Supported Markets
[Lingo] Language Banner - BACOM
GitHub Discussion
BACOM needs to add 3 configuration to enable this feature: Wiki
Test URLs:
QA:
DA-BACOM: https://business.stage.adobe.com/fr/?milolibs=language-banner&langFirst=on&languageBanner=on&marketsSource=bacom
GNav Test URLs
Gnav + Footer + Region Picker modal:
Thin Gnav + ThinFooter + Region Picker dropup:
Localnav + Promo:
Sticky Branch Banner:
Inline Branch Banner:
Blog
RTL Locale