bonnes pratiques jquery wtf ?

[desgnl]

Bonjour, 

 

Dans les fichiers javascript du thème par défaut, on retrouve ça "$(document).on(…" partout. C'est mal ! C'est à dire que lorsqu'on attache un évènement à la racine du document, à chaque clic dans la page, jquery parcourt tout l'arbre du DOM pour trouver les éléments auxquels ils se rapportent. c'est très lourd, pour rien. 

 

Sur la doc de Jquery (ici), il est écrit: "Attaching many delegated event handlers near the top of the document tree can degrade performance. (…) For best performance, attach delegated events at a document location as close as possible to the target elements."

 

Une meilleure façon de faire: 

• si il n'y a qu'un seul objet: "$('mon-objet').on('click', function() { … "

• si il y a une collection d'objets (genre un menu): $('ul.menu').on('click', 'li', function() { … " (comme ça on attache un seul gestionnaire au niveau du menu qui est utilisé pour tous les li. 

 

c'est pas la peine non plus d'ajouter des selecteurs css plus que nécessaire (genre: $('#legrandparent .leparent .l-objet")) Il vaut mieux pointer direct sur l'objet ($('.l-objet')).

 

Une autre bonne pratique jquery, consiste à mettre un objet jquery dans une variable (par convention, avec un"$" devant p.e.: $mon_objet = $('#id_de_mon_objet')), pour éviter à jquery d'avoir à le chercher dans la page à chaque fois qu'il est appelé dans le code… 

 

encore un truc horrible: "$(this).parent().parent()"… mais… vraiment ???

 

bref… le code javascript du thème par défaut est assez moche.

 

Notez bien que cette remarque (et les autres wtf ? que j'ai posté) le sont dans l'espoir d'aider à améliorer les choses… 

 

Et aussi, j'ai des questions irrésolues dans le forum et votre aide serait vraiment la bienvenue. obrigado

 

 

 

[desgnl]

aussi, 

 

il est déconseillé d'utiliser les méthodes show() et hide() de jquery cf https://twitter.com/paul_irish/status/564443848613847040 (hide(), toggle(), :visible, :hidden. You don't need 'em and they will only hurt you.)

 

enfin, fadeIn / fadeOut, et d'une façon générale toutes les fonctions d'animation de jquery gagnent à être remplacées par leur équivalent avec velocity.js

Edited April 29, 2015 by desgnl (see edit history)

[Marvin Lamart]

Bonjour,

Bonjour, 

 

Dans les fichiers javascript du thème par défaut, on retrouve ça "$(document).on(…" partout. C'est mal ! C'est à dire que lorsqu'on attache un évènement à la racine du document, à chaque clic dans la page, jquery parcourt tout l'arbre du DOM pour trouver les éléments auxquels ils se rapportent. c'est très lourd, pour rien. 

 

Sur la doc de Jquery (ici), il est écrit: "Attaching many delegated event handlers near the top of the document tree can degrade performance. (…) For best performance, attach delegated events at a document location as close as possible to the target elements."

 

Une meilleure façon de faire: 

• si il n'y a qu'un seul objet: "$('mon-objet').on('click', function() { … "

• si il y a une collection d'objets (genre un menu): $('ul.menu').on('click', 'li', function() { … " (comme ça on attache un seul gestionnaire au niveau du menu qui est utilisé pour tous les li.

 

Si ils utilisent la forme

$(document).on("event","selector", function(){})

C'est pour que ces évènements fonctionne sur les éléments rajouté au DOM dynamiquement.

 

 

Pour ce qui est des propriétés CSS, Prestashop possède énormément de classe "standart" pouvant être appliqué à plusieurs éléments différents pour les styliser, il faut donc ensuite faire attention lorsque l'on rédige le CSS à ne pas trop raccourcir le sélecteur de peur de sélectionner trop d'élément

 

Pour:

$(this).parent().parent()

Il y a des fois, où cette pratique est tout simplement forcée.

Edited May 6, 2015 by Marvin Lamart (see edit history)

[desgnl]

@Marvin Lamart

 

Si on veut, proprement, attacher des évènements à des éléments ajoutés dynamiquement sur le dom, 

on peut faire une ou plusieurs fonctions qui sont appelées quand le dom est ready et lorsqu'il est modifié. Ces fonctions contiennent les gestionnaires d'évènements sans faire référence à "document". C'est tout à fait possible: je l'ai fait.

Edited May 6, 2015 by desgnl (see edit history)

[Marvin Lamart]

Enfait ce que j'essayais surtout de dire ce que même si ce n'est pas la manière la plus propre de faire, c'est une manière "standard" de faire.

 

Prestashop ne peut pas se maintenir toujours au top du top, il y a toujours des améliorations à apporter et de nouvelles techniques et solutions voient le jour régulièrement.

Pour moi, le travail de Prestashop est plus de proposer un CMS facile d'accès aux intégrateur et aux utilisateurs finaux avec donc une bonne base en back-end que de se focaliser sur le front-end qui relève du travail des intégrateurs/développeurs  et qui comprend le fait de mettre en place ces améliorations plus qu'à l'équipe de Prestashop (imo).

 

Edit: Ce qui n'empêche qu'effectivement, si ils mettaient cela en place, ce serait quand même bien sympa

Edited May 6, 2015 by Marvin Lamart (see edit history)

[desgnl]

Je suis d'accord avec toi sur la philosophie, mais je trouve justement que le thème par défaut n'en est pas un. Il est trop compliqué, mal développé, et ne permet pas aux intégrateurs / développeurs de le prendre en main facilement. Il n'y a pas de séparation entre ce qui relève de la présentation et du fonctionnement, le javascript, le html et la css ne sont pas cohérents d'une page à l'autre. C'est pour cela que la plupart des thèmes se contentent de modifier trois variables scss et que beaucoup de sites prestashop se ressemblent. 

 

Sans compter les lourdeurs qui rendent le thème peu performant… 

Edited May 6, 2015 by desgnl (see edit history)