Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Enregistrer et utiliser la préférence de locale de l'utilisateur (PIX-6506). #455

Merged
merged 5 commits into from
Jan 6, 2023

Conversation

VincentHardouin
Copy link
Member

@VincentHardouin VincentHardouin commented Dec 29, 2022

🎄 Problème

Actuellement l'utilisateur doit choisir sa locale lorsqu'il arrive sur le site international. Et lorsqu'il revient à la racine du site, il lui est redemandé de choisir, ce qui rajoute une interaction et fait que l'accès à Pix n'est pas immédiat.

🎁 Proposition

  • Enregistrer la locale à l'aide d'un cookie grâce à la callback proposée par nuxt-i18n qui se déclenche lors d'un changement de locale.
  • Supprimer la rédirection faite par nuxt-i18n pour recoder la redirection dans une nouvelle page index :
    • vers /locale-choice quand il n'a pas de cookie locale
    • vers l'index de la bonne locale (localized-home), quand il a un cookie
  • La nouvelle page index est présente uniquement pour le sitre vitrine international (pix.org)

🌟 Remarques

Les pistes explorées :

Middleware

  • Au début on ne déclenchait pas les middlewares, car nous n'avions pas de liens internes => corrigé par la PR [FEATURE] Fluidifier la navigation de l'utilisateur (PIX-6705)  #456
  • Dans nos essais on a eu un problème avec le SSR en dev car on ne pouvait pas accéder au cookie du côté server, mais avec la variable process.client, nous avons pu obtenir quelque chose de fonctionnel

Nous avons exploré l'utilisation du middleware en recodant le rootRedirect fait par nuxt-i18n et en ajoutant notre logique de redirection si un cookie locale est présent le POC était fonctionnel :

export default function (context) {
  if (process.client) {
    const { route, redirect } = context
    if (route.path !== '/') {
      return
    }

    const localeCookie = _getLocaleFromCookie()
    return localeCookie
      ? redirect(`/${localeCookie}/`)
      : redirect('/locale-choice')
  }
}

function _getLocaleFromCookie() {
  const localeCookie = document.cookie
    .split('; ')
    .find((item) => item.startsWith('locale'))
  if (!localeCookie) return null
  return localeCookie.split('=')[1]
}

Avantages

Inconvénients

  • Se fait à chaque changement de page, alors que nous en avons besoin que sur la page index
  • Inclut du js pas utile pour les autres pages
  • On obtenait une 403, car le serveur n'arrivait pas à accéder à la page index à la racine, car celle-ci n'existe pas.

Plugin

  • Testé avec le hook beforeEach du vue-router navigation guards. Cependant le rootRedirect qu'on demande dans la configuration de nuxt-i18n surgissait ensuite. C'est pourtant pas en accord avec le lifecycle exposé par Nuxt, cette piste pourrait être aussi exploré de nouveau.
  • Testé avec le hook window.onNuxtReady qui est sensé se lancer qu'une seule fois, mais parfois il se déclenchait parfois plusieurs fois aussi bien en dev qu'en statique. Est-ce que c'est du à nuxt-i18n et/ou au rootRedirect ? A explorer de nouveau.

Ces pistes nous ont causées des problèmes de scintillement de pages car les redirections sont faites une fois l'application chargée du côté client et parfois plusieurs fois.

Nginx

  • L'idée était de faire la redirection vers le contenu du cookie locale si l'utilisateur en avait un. Malheureusement, avec notre config actuelle le navigateur met en cache la page d'accueil, donc on ne repassait pas par le serveur et l'utilisateur était systématiquement renvoyé sur la sélection de locale.
  • Le gros avantage de nginx était que l'utilisateur n'avait pas de scintillement de page comme la redirection était faite du côté du serveur
 location = / {
    if ($cookie_locale ~ ^[a-zA-Z-]+$) {
      return 302 /$cookie_locale/;
    }
  }

Redirection navigateur vs. History API

  mounted() {
-    const preferredLocale = this.getLocaleFromCookie()
-    document.location = preferredLocale
-      ? `/${preferredLocale}/`
-      : '/locale-choice'
+    const chosenLocale = this.getLocaleFromCookie()
+    const url = chosenLocale ? `/${chosenLocale}/` : '/locale-choice'
+    this.$router.push(url)
  }

Pas de différence notable au niveau des performances. Mais la version History API est nettement plus simple à tester.

🎅 Pour tester

Procédure à suivre :

  1. Vérifier que le fonctionnement des sites Pix Pro (.fr) et Pix Pro (.org) n'a pas été modifié
  2. Vérifier que le fonctionnement du site Pix Site (.fr) n'a pas été modifié
  3. Aller sur Pix Site (.org), peu importe la page, et supprimer le cookie locale qui pourrait être dans le navigateur
  4. Aller à la racine de Pix Site (.org) (en suivant les liens ci-dessous des applications déployées ou en entrant un URL manuellement dans la barre d'adresse du navigateur) et constater une redirection vers la page /locale-choice qui oblige de choisir sa locale
  5. Cliquer sur le « bouton » d'une des 3 locales International Français, International English ou FWB. Constater une redirection vers la page de locale correspondante, respectivement /fr, /en-gb ou /fr-be. Constater dans le navigateur l'ajout d'un cookie locale ayant pour valeur l'identifiant de la locale sélectionnée, fr, en-gb ou fr-be
  6. Aller à la racine de Pix Site (.org) (en suivant les liens ci-dessous des applications déployées ou en entrant un URL manuellement dans la barre d'adresse du navigateur) et constater la redirection vers la page de locale correspondant à la locale précédemment sélectionnée
  7. Entrer manuellement dans la barre d'adresse du navigateur différents URL de pages de contenu existantes (XXX/fr-be/les-tests, /fr/pix-et-unesco, XXX/en-gb/the-tests, etc. etc.) et constater qu'aucune redirection n'a lieu
  8. Reprendre la procédure au numéro 3. en choisissant une locale différente
  9. Reprendre la procédure au numéro 3. en activant son cache navigateur

@pix-service
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles ici.

@lego-technix lego-technix force-pushed the pix-6506-save-locale-preferences branch 6 times, most recently from f75bf87 to ab6f2f0 Compare January 2, 2023 07:40
@reibecca reibecca force-pushed the pix-6506-save-locale-preferences branch from 2ad1ffe to 8e1a65d Compare January 2, 2023 14:10
@VincentHardouin VincentHardouin changed the base branch from dev to pix-6705-create-internal-links January 2, 2023 14:11
tests.sh Outdated Show resolved Hide resolved
plugins/locale-observer.js Outdated Show resolved Hide resolved
tests/plugins/locale-observer.test.js Outdated Show resolved Hide resolved
Base automatically changed from pix-6705-create-internal-links to dev January 3, 2023 09:22
@VincentHardouin VincentHardouin force-pushed the pix-6506-save-locale-preferences branch from defd0e0 to baa5384 Compare January 3, 2023 09:56
@VincentHardouin VincentHardouin marked this pull request as ready for review January 3, 2023 10:09
@VincentHardouin VincentHardouin force-pushed the pix-6506-save-locale-preferences branch from baa5384 to 4c4a278 Compare January 3, 2023 10:22
@yannbertrand yannbertrand self-requested a review January 3, 2023 13:03
@VincentHardouin VincentHardouin force-pushed the pix-6506-save-locale-preferences branch from 121e39a to b3bbf66 Compare January 3, 2023 14:36
@yannbertrand yannbertrand force-pushed the pix-6506-save-locale-preferences branch from 362cd19 to 89c3c86 Compare January 3, 2023 16:30
@lego-technix lego-technix force-pushed the pix-6506-save-locale-preferences branch from 0cb1350 to db57c41 Compare January 3, 2023 17:22
@yannbertrand yannbertrand force-pushed the pix-6506-save-locale-preferences branch 6 times, most recently from f6ed61e to 25ac905 Compare January 4, 2023 13:20
nuxt.config.js Outdated
@@ -180,6 +180,13 @@ const nuxtConfig = {
router: {
middleware: ['current-page-path'],
linkExactActiveClass: 'current-active-link',
extendRoutes(routes) {
if (config.isPixSite && config.isFrenchDomain) {
return routes.filter(
Copy link
Member Author

Choose a reason for hiding this comment

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

Cela fonctionne vraiment ? J'ai l'impression qu'il faut plutôt muter routes en regardant la doc

Copy link
Member

Choose a reason for hiding this comment

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

En local, on a constaté que ça fonctionne oui. La doc est pas complète sur le sujet, @lego-technix est allé voir le code source de Nuxt en particuliers leurs tests, parfois ils mutent, parfois ils return si ma mémoire est bonne :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh sympa :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Tout à fait, je confirme.

Copy link
Member Author

@VincentHardouin VincentHardouin Jan 5, 2023

Choose a reason for hiding this comment

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

Je viens de voir en effet c'est fait dans un test, mais du coup ils ne respectent donc pas la signature de leur fonction :/

edit: On peut voir le code qui permet ça

Copy link
Member Author

Choose a reason for hiding this comment

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

Est-ce qu'il ne faudrait pas tester ce code ?

@VincentHardouin VincentHardouin force-pushed the pix-6506-save-locale-preferences branch from 44be081 to 52975b4 Compare January 5, 2023 10:32
@reibecca reibecca force-pushed the pix-6506-save-locale-preferences branch 3 times, most recently from a774aaa to a681450 Compare January 5, 2023 16:46
@reibecca reibecca force-pushed the pix-6506-save-locale-preferences branch from ddd6d3a to 38a4fe3 Compare January 5, 2023 16:58
@yannbertrand yannbertrand force-pushed the pix-6506-save-locale-preferences branch from 4884d22 to 38a4fe3 Compare January 5, 2023 17:06
@VincentHardouin VincentHardouin force-pushed the pix-6506-save-locale-preferences branch from 38a4fe3 to b96a3f1 Compare January 6, 2023 10:05
mounted() {
const chosenLocale = this.getLocaleFromCookie()
const url = chosenLocale ? `/${chosenLocale}/` : '/locale-choice'
this.$router.push(url)
Copy link
Member

Choose a reason for hiding this comment

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

Le push ajoute une entrée dans l'historique de l'utilisateur, ce qui fait qu'on ne peut pas utiliser le bouton précédent du navigateur qui nous redirige automatiquement sur la page courante.

Je propose de remplacer par :

Suggested change
this.$router.push(url)
this.$router.replace(url)

Pour ne pas sauvegarder l'accès au / dans l'historique. (dans une autre PR)

Copy link
Member

@yannbertrand yannbertrand left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

6 participants