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] Ajouter la page de choix de locale sur pro.pix.org (PIX-6834). #469

Merged

Conversation

VincentHardouin
Copy link
Member

🎄 Problème

Actuellement, les comportements de pix.org et pro.pix.org ne sont pas iso, ce qui ajoute de la complexité. Le besoin pour le pro d'avoir une page de choix de langue est aussi présent.

🎁 Proposition

Ajouter la nouvelle page de choix de langue sur le site PRO

🌟 Remarques

🎅 Pour tester

Se rendre sur la RA pro.org

  • Constater qu'une page d'accueil permettant de choisir sa locale est présente
  • Revenir sur pro.pix.org et constater qu'on est redirigé vers la locale précédemment choisie

@yannbertrand
Copy link
Member

Top les tests supplémentaire pour pix pro 👍

@yannbertrand yannbertrand force-pushed the pix-6834-add-locale-choice-page-in-pix-pro branch from 0347e28 to 8909e47 Compare January 16, 2023 13:39
@yannbertrand
Copy link
Member

Le check deploy/sclng: pix-site-review est un fail de ma part, ne pas y faire attention

@yannbertrand
Copy link
Member

yannbertrand commented Jan 16, 2023

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

Les variables d'environnement seront accessibles ici.

@VincentHardouin VincentHardouin force-pushed the pix-6834-add-locale-choice-page-in-pix-pro branch from 8909e47 to 55983f9 Compare January 16, 2023 14:16
Comment on lines +11 to +16
<locale-link
v-for="locale in locales"
:key="locale.code"
class="locale-choice__link"
:locale="locale"
/>
Copy link
Member

Choose a reason for hiding this comment

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

Les liens en RA ramènent vers site-pr469 sans setter de cookie donc pas possible de tester en cliquant... (pareil sur la home ou dans le locale switcher).

Par contre en settant mon cookie à la main je suis bien redirigé vers la locale que j'ai stocké en pref.

Et si je stocke fr-be, je reste bien sur la page de sélection de préférence :)

Copy link
Member

Choose a reason for hiding this comment

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

Je redéploy avec les bonnes vars d'env pour faire le test de bout en bout


describe('#mounted', () => {
describe('when there is no cookie', () => {
test('do not redirect', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
test('do not redirect', async () => {
test('doesn't redirect', async () => {

Copy link
Member Author

@VincentHardouin VincentHardouin Jan 17, 2023

Choose a reason for hiding this comment

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

C'est un copier/coller de pix-site/index.test.js je propose de le faire dans une autre PR

})

describe('with a crafted cookie', () => {
test('cookie value is ignored', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
test('cookie value is ignored', () => {
test('ignores cookie value', () => {

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.

OK pour Phili, OK pour Jeff, OK pour moi

@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-6834-add-locale-choice-page-in-pix-pro branch from 55983f9 to 2815e05 Compare January 17, 2023 17:30
@pix-service-auto-merge pix-service-auto-merge deleted the pix-6834-add-locale-choice-page-in-pix-pro branch January 17, 2023 17:32
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.

4 participants