Add html lang attribute, at least for SSR

SAP / spartacus

Spartacus is a lean, Angular-based JavaScript storefront for SAP Commerce Cloud that communicates exclusively through the Commerce REST API.

Apache License 2.0

737 stars 381 forks source link

Add html lang attribute, at least for SSR #1661

Closed Xymmer closed 5 years ago

Xymmer commented 5 years ago

The HTML should contain a reference to the current used language on the page:

<html lang="fr">

Platonn commented 5 years ago

QA steps for <html lang="">:

Prerequisites

configure Storefront base url in the app.module
(for convenience of testing you may configure Storefront for PWA to be disabled)

Server side rendering

disable JS in your browser

run ssr:

yarn build:core:lib
yarn build
yarn build:ssr
yarn start:ssr

open localhost:4200 and verify <html lang="en"> in the srouce code in the browser
open localhost:4200/de and verify <html lang="de"> in the srouce code in the browser

Client side rendering

re-enable javascript in your browser
run yarn install
open localhost:4200
change the language in app in runtime, using the language selector
verify the proper language <html lang=" "> in the srouce code in the browser after changing active langauge

tobi-or-not-tobi commented 5 years ago

Can also be previewed without SSR, in the browser, using chrome dev tools

Xymmer commented 5 years ago

i checked out latest code and tried to build from source, but i'm getting error:

TypeError: You provided 'undefined' where a stream was expected. You can provide an Observable, Promise, Array, or Iterable. at subscribeTo (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32456:15) at subscribeToResult (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32355:76) at CatchSubscriber.error (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:35205:94) at SwitchMapSubscriber.OuterSubscriber.notifyError (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32325:26) at InnerSubscriber._error (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32386:21) at InnerSubscriber.Subscriber.error (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:29247:18) at CatchSubscriber.OuterSubscriber.notifyError (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32325:26) at InnerSubscriber._error (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:32386:21) at InnerSubscriber.Subscriber.error (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:29247:18) at Observable._subscribe (/SAPDevelop/REPOS/ssr-test/cloud-commerce-spartacus-storefront/dist/ssr/server.js:31333:117)

Xymmer commented 5 years ago

Kris says: set the modelT as the base url in the app.module, rebuild app, rebuild srr and run ssr:

yarn build:ssr
yarn start:ssr

(dev17 has invalid cert, so we have to test SSR only on modelT)

Xymmer commented 5 years ago

tried testing again but now there's an issue with assets lib. unassigning myself, someone from Europe please test Thursday morning.

Xymmer commented 5 years ago

confirmed (1) when you change language, the html lang attribute changes but haven't figured out how to check in ssr yet

Xymmer commented 5 years ago

passing as we think my test is enough