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

Fix 4008 : Ajoute un bouton au menu d'envoi des MP #4080

Merged

Conversation

Anto59290
Copy link
Contributor

Q R
Type de modification évolution
Ticket(s) (issue(s)) concerné(s) #4008

QA

  • Il faut reconstruire le front (modification du css et du sprite). Attention à vider le cache de votre navigateur / aller en navigation privée.
  • Se connecter à un compte, vérifier que l'icône d'envoi de Mp dans le menu déroulant est bien présent et bien placé.
  • Au clic le lien fonctionne.

A tester, si possible sur desktop, tablette et smartphone pour l'aspect graphique.
@FanJiyong : Je te propose l'intégration suivante. Ce n'est pas pixel perfect ;) . J'ai retravaillé légèrement ton icône sous AI pour rendre un peu plus visible le +, dis moi si ça te convient.

mp icon

pm-tooltip

@coveralls
Copy link

Coverage Status

Coverage decreased (-32.7%) to 54.42% when pulling e0b1fbe on Anto59290:4008_ajoute_un_bouton_pour_envoyer_un_mp into 7d5e49c on zestedesavoir:dev.

@coveralls
Copy link

coveralls commented Dec 17, 2016

Coverage Status

Coverage remained the same at 87.09% when pulling e0b1fbe on Anto59290:4008_ajoute_un_bouton_pour_envoyer_un_mp into 7d5e49c on zestedesavoir:dev.

@Emeric54 Emeric54 added C-Front Concerne l'interface du site QA svp labels Dec 18, 2016
@FanJiyong
Copy link

FanJiyong commented Dec 19, 2016

Salut,

Je ne suis pas totalement satisfait de l'apparence de ce bouton une fois intégré.
j'ai l'impression que l'icone est un peu petite, je te joint des instructions d'intégration, j'espère que cela va pouvoir t'aider.

--

Au niveau de la modification tu l'as écrasé sur la haute ?

Je pensais que le site utilisé une font icône, si tu as des besoins spécifique sur l'icône dit le moi.

En version tactile il faut que l'icône reste cliclable. n'hésitez pas à augmenter la hitbox.

@Anto59290
Copy link
Contributor Author

@FanJiyong

propal

Merci de ton retour ;)

@coveralls
Copy link

coveralls commented Dec 20, 2016

Coverage Status

Coverage remained the same at 87.09% when pulling f2071ea on Anto59290:4008_ajoute_un_bouton_pour_envoyer_un_mp into 7d5e49c on zestedesavoir:dev.

@FanJiyong
Copy link

FanJiyong commented Dec 20, 2016 via email

@Anto59290
Copy link
Contributor Author

"Gros relou" j'irais pas jusque là ;)

Sur la première version l'icône était (de mémoire) à une taille de 16px de largeur, sur l'image que tu envoie à l'instant elle en fait 17px. La différence est mince. L'image que tu envoie à l'instant n'est pas utilisable tel que puisse chaque icône est muni d'une version classique et d'une version@2x (deux fois plus grande). Tu ne pouvait pas le savoir ;)

Je vais donc repartir du SVG (sauf si entre temps tu poste une version @2x, tu peux en trouver dans le repo si tu veux jeter un coup d'oeil). Et essayer de faire un nouvelle version.

@FanJiyong
Copy link

FanJiyong commented Dec 20, 2016

Sur la première version l'icône était (de mémoire) à une taille de 16px de largeur, sur l'image que tu envoie à l'instant elle en fait 17px.

Sur la première c'est du SVG elle doit faire 500x500, je pensais vraiment à une font icon .

Là où je me sent vraiment gêner c'est par rapport a l'image du dessus où la j'ai 15 vs 17px.

Bon du coup j'ai refait une images avec les bonnes valeurs, et depuis la source
161212_nouveau_mp_img

Icone x2 :
--> icone_new_mpx2 <--

@Anto59290
Copy link
Contributor Author

@FanJiyong Je crois que c'est au pixel près la position demandée ;)
Note pour l'avenir : j'ai du intervenir un peu sur ton icône en version @2x. Quand tu fournit des icônes pour ce genre de sprites il faut que la version @2x ait des dimensions doubles (au pixel près) de la version normale. Tout cela est vérifié par une tâche de build automatique, donc pas d’exception ;)

tuvasaimer

@coveralls
Copy link

coveralls commented Dec 21, 2016

Coverage Status

Coverage remained the same at 87.09% when pulling d520c7d on Anto59290:4008_ajoute_un_bouton_pour_envoyer_un_mp into 7d5e49c on zestedesavoir:dev.

@FanJiyong
Copy link

bah là rien à redire ! 👍 Validé !

Note pour l'avenir : j'ai du intervenir un peu sur ton icône en version @2x. Quand tu fournit des icônes pour ce genre de sprites il faut que la version @2x ait des dimensions doubles (au pixel près) de la version normale. Tout cela est vérifié par une tâche de build automatique, donc pas d’exception ;)

Mon image fait 34*28 c'est pas bon ?

@Anto59290
Copy link
Contributor Author

Yeeeeeeeeeeees !
Qa can start alors ;)

La première fait 1716 la deuxième fait 3428, elle aurait du faire 3432 ;)
La zone utile est de 17
14 mais tu avait laissé 2px de transparence au dessus, du coup j'ai fait de même sur la @2x ;)

@FanJiyong
Copy link

haaa à cause de le transparence !

en tout cas merci à toi @Anto59290 (et gg pour la patience ;))

@gllmc
Copy link
Member

gllmc commented Dec 23, 2016

QA : OK !

@vhf vhf removed the QA svp label Dec 25, 2016
@vhf
Copy link
Contributor

vhf commented Dec 25, 2016

Merci !

@vhf vhf merged commit b8225d4 into zestedesavoir:dev Dec 25, 2016
@vhf vhf added this to the Version de développement milestone Dec 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Front Concerne l'interface du site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants