Copy over new version of Milo's geo-based locales #130

hparra · 2023-07-13T23:15:33Z

Adopt milo fix for incorrect lang attributes on html tag
Add new reg property for future use

See adobecom/milo#915 and adobecom/milo#938

Test URLs:

aem-code-sync · 2023-07-13T23:15:36Z

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

Re-run PSI Checks

codecov-commenter · 2023-07-13T23:17:17Z

Codecov Report

❗ No coverage uploaded for pull request base (main@dd33b71). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #130   +/-   ##
=======================================
  Coverage        ?   96.36%           
=======================================
  Files           ?        6           
  Lines           ?      468           
  Branches        ?        0           
=======================================
  Hits            ?      451           
  Misses          ?       17           
  Partials        ?        0

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

aem-code-sync · 2023-07-13T23:19:25Z

Page	Scores	Audits	Google
/?martech=off		Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 401)

JasonHowellSlavin

LGTM. Alphabetizing for the win!

meganthecoder · 2023-07-14T21:16:09Z

@hparra There's no ui or functionality to verify here, right? It's just adding the new lang and reg properties and alphabetizing?

hparra · 2023-07-14T21:45:55Z

@meganthecoder There is new functionality in Milo that is enabled when these values exist. You can confirm in test urls that HTML lang attribute now only uses language subtag instead of full tag, e.g <html lang="en"> vs. <html lang="en-US">. See adobecom/milo#915 and linked issue for more details. Milo features use getConfig().locale and do not read locale from this attribute.

meganthecoder · 2023-07-14T22:18:26Z

@hparra Cool, I updated the description to reflect that and assigned myself to verify. Is the reg tag being used yet? I didn't see anything in the PRs you mentioned.

hparra · 2023-07-14T22:22:43Z

Is the reg tag being used yet?

Not yet. A couple of new features use it and I will begin porting existing features to use it one by one -- those changes will be in Milo only but it will affect all Milo consumers. There will be additional comms of those.

aem-code-sync · 2023-07-14T22:49:16Z

Page	Scores	Audits	Google
/customer-success-stories?martech=off		Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 401)
/la/customer-success-stories?martech=off		Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 401)

meganthecoder

There are a few locales added that we don't support in bacom. Also, when I was testing, I saw that CaaS cards weren't loading for la

meganthecoder · 2023-07-14T22:59:44Z

scripts/scripts.js

+    cl: { ietf: 'es-CL', lang: 'es', reg: 'CL', tk: 'oln4yqj.css' },
+    cn: { ietf: 'zh-CN', lang: 'zh', reg: 'CN', tk: 'puu3xkp' },
+    co: { ietf: 'es-CO', lang: 'es', reg: 'CO', tk: 'oln4yqj.css' },
+    cr: { ietf: 'es-419', lang: 'es', reg: 'CR', tk: 'oln4yqj.css' },


BACOM doesn't support cr (costa rica), and if you notice, we didn't have it in our config before. We redirect this locale to la. Reference: https://wiki.corp.adobe.com/pages/viewpage.action?pageId=2301888547

Suggested change

cr: { ietf: 'es-419', lang: 'es', reg: 'CR', tk: 'oln4yqj.css' },

meganthecoder · 2023-07-14T23:00:28Z

scripts/scripts.js

+    cz: { ietf: 'cs-CZ', lang: 'cs', reg: 'CZ', tk: 'aaz7dvd.css' },
+    de: { ietf: 'de-DE', lang: 'de', reg: 'DE', tk: 'vin7zsi.css' },
+    dk: { ietf: 'da-DK', lang: 'da', reg: 'DK', tk: 'aaz7dvd.css' },
+    ec: { ietf: 'es-419', lang: 'es', reg: 'EC', tk: 'oln4yqj.css' },


Same for ec (ecuador)

Suggested change

ec: { ietf: 'es-419', lang: 'es', reg: 'EC', tk: 'oln4yqj.css' },

meganthecoder · 2023-07-14T23:02:19Z

scripts/scripts.js

+    fr: { ietf: 'fr-FR', lang: 'fr', reg: 'FR', tk: 'vrk5vyv.css' },
+    gr_el: { ietf: 'el', lang: 'el', reg: 'GR', tk: 'fnx0rsr.css' },
+    gr_en: { ietf: 'en-GR', lang: 'en', reg: 'GR', tk: 'pps7abe.css' },
+    gt: { ietf: 'es-419', lang: 'es', reg: 'GT', tk: 'oln4yqj.css' },


Same for gt (guatemala)

Suggested change

gt: { ietf: 'es-419', lang: 'es', reg: 'GT', tk: 'oln4yqj.css' },

meganthecoder · 2023-07-14T23:03:37Z

scripts/scripts.js

+    ph_en: { ietf: 'en', lang: 'en', reg: 'PH', tk: 'pps7abe.css' },
+    ph_fil: { ietf: 'fil-PH', lang: 'fil', reg: 'PH', tk: 'ict8rmp.css' },
+    pl: { ietf: 'pl-PL', lang: 'pl', reg: 'PL', tk: 'aaz7dvd.css' },
+    pr: { ietf: 'es-419', lang: 'es', reg: 'PR', tk: 'oln4yqj.css' },


Same for pr (puerto rico)

Suggested change

pr: { ietf: 'es-419', lang: 'es', reg: 'PR', tk: 'oln4yqj.css' },

hparra · 2023-07-14T23:13:28Z

There are a few locales added that we don't support in bacom

This should be OK. locales is just a source of truth for mappings. locales should NOT be used to enable or disable a particular locale. I can remove but it makes syncing more difficult.

Also, when I was testing, I saw that CaaS cards weren't loading for la

This is probably not OK and is the sort of thing I was afraid of. Can we sync on this so I can understand what is happening?

hparra · 2023-07-17T21:39:15Z

Waiting for adobecom/milo#945

JasonHowellSlavin · 2023-07-24T14:31:12Z

@hparra @meganthecoder looks like la cards are loading again on this one, some links:
https://hparra-patch-2--bacom--adobecom.hlx.page/la/customer-success-stories?martech=off
https://hparra-patch-2--bacom--adobecom.hlx.page/la/resources/main

meganthecoder · 2023-07-24T21:50:40Z

I believe a different solution was determined here: adobecom/milo#963

@hparra Can this PR be closed?

hparra · 2023-07-24T22:01:26Z

@meganthecoder I was waiting until handoff to close all PRs but I think we're ok.

Copy over new version of Milo's geo-based locales

0e42171

hparra requested a review from JasonHowellSlavin July 13, 2023 23:20

hparra mentioned this pull request Jul 13, 2023

Update itef for /la geo #123

Closed

JasonHowellSlavin approved these changes Jul 13, 2023

View reviewed changes

meganthecoder approved these changes Jul 14, 2023

View reviewed changes

meganthecoder self-assigned this Jul 14, 2023

meganthecoder requested changes Jul 14, 2023

View reviewed changes

hparra added the do not merge PR should not be merged yet label Jul 17, 2023

hparra mentioned this pull request Jul 17, 2023

Use locale from getConfig (caas) adobecom/milo#945

Merged

hparra closed this Jul 24, 2023

hparra deleted the hparra-patch-2 branch July 24, 2023 22:01

meganthecoder mentioned this pull request Jul 11, 2024

MWPW-154250 Update India lang code #223

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy over new version of Milo's geo-based locales #130

Copy over new version of Milo's geo-based locales #130

hparra commented Jul 13, 2023 •

edited by meganthecoder

Loading

aem-code-sync bot commented Jul 13, 2023

codecov-commenter commented Jul 13, 2023 •

edited

Loading

aem-code-sync bot commented Jul 13, 2023

JasonHowellSlavin left a comment

meganthecoder commented Jul 14, 2023

hparra commented Jul 14, 2023

meganthecoder commented Jul 14, 2023

hparra commented Jul 14, 2023

aem-code-sync bot commented Jul 14, 2023

meganthecoder left a comment

meganthecoder Jul 14, 2023

meganthecoder Jul 14, 2023

meganthecoder Jul 14, 2023

meganthecoder Jul 14, 2023

hparra commented Jul 14, 2023

hparra commented Jul 17, 2023

JasonHowellSlavin commented Jul 24, 2023

meganthecoder commented Jul 24, 2023

hparra commented Jul 24, 2023

Copy over new version of Milo's geo-based locales #130

Copy over new version of Milo's geo-based locales #130

Conversation

hparra commented Jul 13, 2023 • edited by meganthecoder Loading

aem-code-sync bot commented Jul 13, 2023

codecov-commenter commented Jul 13, 2023 • edited Loading

Codecov Report

aem-code-sync bot commented Jul 13, 2023

JasonHowellSlavin left a comment

Choose a reason for hiding this comment

meganthecoder commented Jul 14, 2023

hparra commented Jul 14, 2023

meganthecoder commented Jul 14, 2023

hparra commented Jul 14, 2023

aem-code-sync bot commented Jul 14, 2023

meganthecoder left a comment

Choose a reason for hiding this comment

meganthecoder Jul 14, 2023

Choose a reason for hiding this comment

meganthecoder Jul 14, 2023

Choose a reason for hiding this comment

meganthecoder Jul 14, 2023

Choose a reason for hiding this comment

meganthecoder Jul 14, 2023

Choose a reason for hiding this comment

hparra commented Jul 14, 2023

hparra commented Jul 17, 2023

JasonHowellSlavin commented Jul 24, 2023

meganthecoder commented Jul 24, 2023

hparra commented Jul 24, 2023

hparra commented Jul 13, 2023 •

edited by meganthecoder

Loading

codecov-commenter commented Jul 13, 2023 •

edited

Loading