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

[version 1.7] La bannière "cookies" disparaît quand on clique sur "en savoir plus" #2487

Closed
pierre-24 opened this issue Apr 3, 2015 · 10 comments

Comments

@pierre-24
Copy link
Member

commented Apr 3, 2015

Comme je l'avais mentionné à l'époque avec #2313, y'a un soucis avec eu-cookies-banner; Personnellement, la bannière disparaît quand je clique sur "en savoir plus". Debian/jessie et Chrome/41.0 et Firefox/31.5. J'avais finalement à l'époque réussi à reproduire le truc, mais j'avais d'énorme problème de front. Donc, my bad sur celle là, mais très clairement, ça marche pas en bêta.

@Situphen

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2015

Bon, l'erreur vient de la version 1.4 de Cookies-EU-banner. Quand j'ai commencé ma PR ils étaient à la 1.3 mais comme la version n'est pas en strict dans le package.json, tu as installé la 1.4 qui était sortie entre temps.

Bref, je fais une PR chez eux. Après qu'ils l'aient accepté et qu'ils aient fais une release, je ferai une PR de beta-fix ici !

@pierre-24

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2015

... Et j'ai probablement utilisé la 1.3 pour finalement avoir le bon comportement, ceci explique celà. Bon, une fois encore ZdS contribue aux projets libres ;)

@SpaceFox

This comment has been minimized.

Copy link
Member

commented Apr 3, 2015

Donc si je comprends bien, on a ajouté une dépendance pour "mieux gérer" le coup des cookies CNIL et à la place de ça, on se retrouve avec une dépendance dont la mise à jour pète complètement la gestion des cookies CNIL ?

Est-ce qu'on ne pourrait pas plutôt soit rester sur une version fonctionnelle, soit utiliser un système fiable ?

@Situphen

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2015

Donc si je comprends bien, on a ajouté une dépendance pour "mieux gérer" le coup des cookies CNIL et à la place de ça, on se retrouve avec une dépendance dont la mise à jour pète complètement la gestion des cookies CNIL ?

La mise à jour a pété une partie de la gestion, celle qui fait en sorte que quand tu cliques sur "En savoir plus", tu n'es pas encore tracké et la bannière s'affiche. Le reste fonctionne bien.

Est-ce qu'on ne pourrait pas plutôt soit rester sur une version fonctionnelle, soit utiliser un système fiable ?

On a deux solutions :

  • soit on prend la version 1.3 ;
  • soit on attend que ma PR Alex-D/Cookies-EU-banner#8 soit mergée et qu'une nouvelle version soit créée.

Dans tous les cas je mettrai cette dépendance en strict pour que ce cas ne se reproduise plus !

@SpaceFox

This comment has been minimized.

Copy link
Member

commented Apr 3, 2015

La mise à jour a pété une partie de la gestion, celle qui fait en sorte que quand tu cliques sur "En savoir plus", tu n'es pas encore tracké et la bannière s'affiche. Le reste fonctionne bien.

C'est dommage, c'était justement l'argument qui nous a fait passer à cette dépendance de mémoire.

On va voir la réactivité de l'upstream à intégrer la PR, mais dans tous les cas ça ne doit pas ralentir notre release (un peu moins de 2 semaines pour que ce soit réintégré ici donc).

@pierre-24

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2015

On va aider: c'est @Alex-D qui est derrière eu-cookies-banner. Ping ;)

@Alex-D

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2015

C'est corrigé par @Situphen et mergé. C'était un simple _ qui manquait.

@Alex-D

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2015

J'ai déployé la 1.2.6, vous pouvez la mettre en tant que version "fixe" dans votre package.json (sans le ^), ça évitera les surprises.

Situphen added a commit to Situphen/zds-site that referenced this issue Apr 3, 2015
@SpaceFox

This comment has been minimized.

Copy link
Member

commented Apr 3, 2015

OK, merci @Alex-D pour la réactivité !

@Eskimon

This comment has been minimized.

Copy link
Member

commented Apr 6, 2015

C'est réparé

@Eskimon Eskimon closed this Apr 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.