aemsites / esri

Apache License 2.0
1 stars 1 forks source link

Add alternate language links to head, enables Global Nav lang switcher #134

Closed twhite313 closed 1 month ago

twhite313 commented 1 month ago

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):

Fix #94

Test URLs:

aem-code-sync[bot] commented 1 month ago

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed. In case there are problems, just click a checkbox below to rerun the respective action.

Commits * [b404113](https://github.com/aemsites/esri/commit/b40411391b8f700c22e073571fea5c35b01067fe) :white_check_mark: (latest) * [b068352](https://github.com/aemsites/esri/commit/b068352852289727fef9412479d07f29c6fe123f) :white_check_mark: * [64f2331](https://github.com/aemsites/esri/commit/64f233165b80c9441376c9cc269fd74801258156) :white_check_mark: * [8cb6f60](https://github.com/aemsites/esri/commit/8cb6f601dcb89c54cfeaf531ca7bc3e054422164) :white_check_mark: * [c774cee](https://github.com/aemsites/esri/commit/c774cee537318f64dabccd842a0f8dc3fe430a30) :white_check_mark: * [68c4293](https://github.com/aemsites/esri/commit/68c42935a6aa7448d8306ae57d0822ff964022e8) :white_check_mark: * [683c8f0](https://github.com/aemsites/esri/commit/683c8f008a935d4bd33abee4b7e9a224686737ce) :white_check_mark: * [588b913](https://github.com/aemsites/esri/commit/588b913b8582f45917b0712a3f2c23fd64264a48) :white_check_mark: * [68348de](https://github.com/aemsites/esri/commit/68348de613c2a22d7641f6d09f62d47f54b3d8c6) :white_check_mark: * [e5c08e1](https://github.com/aemsites/esri/commit/e5c08e11b48553443648ccecc4e3994d1bd6c997) :white_check_mark: * [a798d5a](https://github.com/aemsites/esri/commit/a798d5ab6c417ac9788841e03ec0bdf26be91b1b) :white_check_mark: * [1b4d2d6](https://github.com/aemsites/esri/commit/1b4d2d6e35186776b2718e5cf6bd684d2a3e261b) :white_check_mark:
aem-code-sync[bot] commented 1 month ago
Page Scores Audits Google
M / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
D / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
twhite313 commented 1 month ago

@alexcarol not sure if its the best place to add it, but I have alternate langauges links being added to the head, which enables the language switcher. (The icon is misaligned for some reason, but that is for another task.)

I can't get past some of the linter errors. One makes no sense: it says splitPattern isn't used, but it clearly is 4 lines later. I tried modifying some of the other issues but it just ends up breaking my code.

alexcarol commented 1 month ago

@alexcarol not sure if its the best place to add it, but I have alternate langauges links being added to the head, which enables the language switcher. (The icon is misaligned for some reason, but that is for another task.)

I can't get past some of the linter errors. One makes no sense: it says splitPattern isn't used, but it clearly is 4 lines later. I tried modifying some of the other issues but it just ends up breaking my code.

I don't see "splitPattern" used anywhere, are you looking for it in the diff or in your editor? (in case there's changes that might not be pushed). Can you share a link to the line where it's used?

alexcarol commented 1 month ago

@twhite313 here's a suggested change for your PR: https://github.com/aemsites/esri/pull/135 Feel free to use it or change as you need

twhite313 commented 1 month ago

@alexcarol not sure if its the best place to add it, but I have alternate langauges links being added to the head, which enables the language switcher. (The icon is misaligned for some reason, but that is for another task.) I can't get past some of the linter errors. One makes no sense: it says splitPattern isn't used, but it clearly is 4 lines later. I tried modifying some of the other issues but it just ends up breaking my code.

I don't see "splitPattern" used anywhere, are you looking for it in the diff or in your editor? (in case there's changes that might not be pushed). Can you share a link to the line where it's used?

Well that's really weird. You and the linter couldn't see it. Literally there on line 60 and 64. Whatever; moving on.

split

Oh! The error changed. I see it is failing AFTER line 64 now. I was getting errors that it was never declared. Anyway... moving on.

alexcarol commented 1 month ago

@alexcarol not sure if its the best place to add it, but I have alternate langauges links being added to the head, which enables the language switcher. (The icon is misaligned for some reason, but that is for another task.) I can't get past some of the linter errors. One makes no sense: it says splitPattern isn't used, but it clearly is 4 lines later. I tried modifying some of the other issues but it just ends up breaking my code.

I don't see "splitPattern" used anywhere, are you looking for it in the diff or in your editor? (in case there's changes that might not be pushed). Can you share a link to the line where it's used?

Well that's really weird. You and the linter couldn't see it. Literally there on line 60 and 64. Whatever; moving on.

split

Oh! The error changed. I see it is failing AFTER line 64 now. I was getting errors that it was never declared. Anyway... moving on.

The issue was that it was assigned, but never read, so it's essentially not "used".