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

[BUGFIX] Problème de lien dans les menus contribuables (site-36). #61

Merged
merged 9 commits into from
Aug 19, 2019

Conversation

VincentHardouin
Copy link
Member

@VincentHardouin VincentHardouin commented Aug 6, 2019

🦄 Problème

Lors du traitement d'un lien venant de Prismic (sous le format : http://), celui-ci est géré de façon à garder que le lien complet si c'est un lien vers un autre site, ou de garder que le path si c'est un lien pix, permettant de naviguer dans les review apps ou en local. Cependant on a perdu l'usage des link-to de Ember.

🤖 Solution

  • Ajout d'un service routes-table permettant de faire le lien entre le path d'un lien et sa route
  • Ajout d'un composant créant un link-to s'il s'agît d'un lien pix.fr ou une balise d'ancre <a></a> si c'est un lien vers un autre site.

🌈 Remarques

Dans le helper as-route, je vérifie si le path commence par / car sur IE le path ne commence pas par / et donc cela renvoyait vers index.

✨ Review App

https://pix-site-integration-pr61.scalingo.io

Copy link

@ebarbier ebarbier left a comment

Choose a reason for hiding this comment

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

Je n'arrive pas à naviguer dans les pages sous IE11 (header et footer)

app/templates/components/handle-link.hbs Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
{{#if path}}
<LinkTo @route="{{as-route path}}" class="{{linkClass}}">{{{text}}}</LinkTo>

Choose a reason for hiding this comment

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

pourquoi 3 { ?

Copy link
Member Author

Choose a reason for hiding this comment

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

3 { permets de conserver le html.

Choose a reason for hiding this comment

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

ah ok, mais c'est pas un peu dangereux ? on peut injecter du js non ? j'opterais pour htmlSafe()

Copy link
Member Author

Choose a reason for hiding this comment

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

Aucune idée, d'après stackoverflow ça serait identique et la documentation d'ember ne réponds pas à la question. Je trouve ça moins lourd de mettre directement dans le hbs un {{{ }}} que de faire un traitement dans le js

if(doc.hostname === 'pix.fr') {
return doc.pathname;
}
return url;

Choose a reason for hiding this comment

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

plus de return url ?

Copy link
Member Author

@VincentHardouin VincentHardouin Aug 13, 2019

Choose a reason for hiding this comment

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

Je supprime cet helper car il était utilisé que par les menus contribuables. Seulement maintenant les menus utilisent le component handle-link qui lui utilise le helper as-route seulement quand il s'agit d'une adresse Pix.

Choose a reason for hiding this comment

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

j'ai pas tout compris. Ce que je vois c'est que le code ici a été copié et collé dans un component, sauf le return url, du coup je me demande

Copy link
Member Author

@VincentHardouin VincentHardouin Aug 13, 2019

Choose a reason for hiding this comment

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

Alors avant on utilisait le helper handle-url afin de retourner l'url complète si c'était pas une adresse https://pix.fr et sinon juste le pathname permettant de garder la navigation dans les reviews apps par exemple. Maintenant, le component gère si l'adresse est une adresse Pix ou non, permettant si oui de faire un link-to.

@VincentHardouin VincentHardouin merged commit 0f07f98 into dev Aug 19, 2019
@VincentHardouin VincentHardouin deleted the site-36-handle-link-to branch August 19, 2019 07:27
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.

5 participants