Amélioration du plugin de template Pagelinks

patch presque ok :

• corriger les problèmes d'indentation : des caractères tabulation sont utilisés au lieu d'espaces, et supprimer les espaces de fin de lignes
• je pense qu'il est préférable de tester si les labels sont vides plutôt que de les comparer à la valeur "none".
• compléter la documentation dans le commentaire sur la fonction, en particulier ce que doit contenir le dernier paramètre précisément

echo ($prevBound >= 0 ? '<a href="'.$urlStartPage->toString(true).'">' : '') . $displayProperties['start-label'] . ($prevBound >= 0 ? '</a>' : '');

Il est préférable de faire un test if classique, afin que cela soit plus lisible. Et ne faire qu'un test :

if ($prevBound >= 0) {
   echo '<a href="',$urlStartPage->toString(true),'">', $displayProperties['start-label'], '</a>';
} else {
   echo $displayProperties['start-label'] ;
}

Faire de même pour les autres liens.

Note aussi que pour les instructions echo, il est plus performant de séparer les differentes choses à afficher avec une virgule, plutôt que de les concatener.

Enfin, vu que l'on utilise plusieurs fois jUrl::getEngine(), peut être vaut mieux mettre son résultat dans une variable et de faire appel à cette variable.

Replying to laurentj:

* je pense qu'il est préférable de tester si les labels sont vides plutôt que de les comparer à la valeur "none".

En fait, y'a trois cas :

- soit un label est renseigné, auquel cas on l'affiche, - soit un label est vide, et alors on affiche le libellé prévus par défaut, - soit un label est positionné à "none" et dans ce cas on ne l'affiche pas du tout.

donc vide et none sont deux cas de figure différents.

Et si on veut mettre le nom "none" ?

plus sérieusement, je n'aime pas trop quand il y a des valeurs fixes représentant des cas exceptionnelle. Ça peut toujours être source de problème pour l'utilisateur, qui peut ne pas comprendre pourquoi quand il met telle valeur ça ne fonctionne pas comme il espère.

Je préfère donc ceci :

• si le développeur veut mettre la valeur par défaut : il n'indique tout simplement pas de valeur dans le tableau qu'il passe en paramètre. Il faudra donc faire un isset pour tester si il faut mettre la valeur par défaut ou non.
• si le développeur ne veut pas pas afficher le lien, il mettra une valeur vide
• et sinon il met la valeur qu'il veut.

Autre chose : faut-il vraiment mettre les liens dans une liste HTML ? Ce n'est pas forcément plus sémantique, et c'est plus embêtant à styler, puisque la plupart du temps, on les préfère aligné à l'horizontal. Ce qui veut dire qu'on devra mettre la plupart du temps des display:inline, ce qui est assez délicat sous IE à utiliser.

Je mettrais donc plutôt ces liens dans une div, et les labels qui n'ont pas de liens dans un span. Les liens ayant la même classe que le <li> correspondant dans ta version. L'autre avantage, c'est que ça ne cassera pas le design de ceux qui utilisent déjà pagelinks, comme les templates par défaut de jControllerDaoCrud ;-)

Je dois t'avouer que je trouve la solution de la liste html malgré tout plus sémantique qu'un simple enchaînement de liens : car il s'agit bien avant tout d'une LISTE de liens, non ? ;-)

Après je suis d'accord, si on utilise une liste, il faut que ceux qui utilise déjà le plugin ajoutent le stylage CSS pour gérer l'affichage horizontal de la liste.

Question : mais ne vaut-il mieux pas profiter du fait que Jelix est encore en beta pour le faire et avoir ainsi un truc bien carré pour la version 1 officielle ?

Enfin, c'est toi qui décide, si tu préfères néanmoins garder la solution que tu m'as indiquée au dessus, dis moi, et je me mets au travail sur les modifs illico ;-)

Après demande d'avis sur le channel irc #jelix, la conclusion est que c'est mieux de garder le ul :-)

J'attend donc ton nouveau patch incluant les autres modifs...

Fichier diff contenant les modifcations effectuées sur le plugin pagelinks

Documentation d'utilisation

Voilà je viens de poster le patch + la doc avec les modifications demandées.

J'ai également ajouté un petit test supplémentaire qui évite l'affichage des liens dans le cas où il y a moins de deux pages de résultats.

Patch ok :-) J'ai rajouté une classe pagelinks-disabled sur les items désactivés, ça permet de les styler plus facilement.

Patch inclus dans le trunk. Merci pour le patch !

De rien, je suis enchanté de pouvoir apporter une pierre à l'édifice Jelix. Merci à toi de nous permettre de contribuer...

Juste pour faire mon chieur ;-) et revenir à la classe pagelinks-disabled : es-tu certain que cela soit vraiment nécessaire ?

En effet pour styler les items désactivés il suffit simplement de styler les balises "li". Et ensuite on style les autres items (premier, précédent, courant, suivant et dernier) à l'aide de leurs classes respectives.

J'ai donc peur que le class="pagelinks-disabled" sur chaque balise li inactive (les plus nombreuses à priori) ne fasse qu'alourdir le xhtml généré (et donc les performances), sans apporter de réel bénéfice au niveau des possibilités de stylage.

Qu'en penses-tu ?

les plus nombreuses à priori

Je ne suis pas trop d'accord avec toi. À priori, les seules li inactives sont les extremités. Donc y a pas tant de disabled que ça sur une liste de liens.

sans apporter de réel bénéfice au niveau des possibilités de stylage.

Si, c'est plus simple pour celui qui ne maitrise pas toutes les subtilités de CSS ;-)

Je ne suis pas trop d'accord avec toi. À priori, les seules li inactives sont les extremités. Donc y a pas tant de disabled que ça sur une liste de liens.

oui exact, tu as raison en fait ;-)

Question : pour la mise en ligne de la documentation du plugin, on attends que Jelix v1.0 soit sortie ?

Non, tu l'intègre dans le wiki à l'emplacement adéquate, en précisant les choses qui sont spécifiques à 1.0 http://jelix.org/articles/references/tplplugins

J'arrive un peu tard dans la discussion de ce plugin mais j'ai cependant deux remarques à faire concernant les paramètres demandés par celui-ci : nombre total de résultats, résultat courant et nombre de résultat par page.

• Les paramètres cités ci-dessus ne pourraient-ils pas être embarqués dans un objet jPageCounter pour diminuer le nombre de paramètres à passer au plugin et pour déporter une partie de sa logique (et pour pouvoir s'en servir ailleurs si besoin) ? (cf l'énoncé du ticket #261).
• C'est peut-être une préférence de ma part mais je ne suis pas fan d'utiliser directement l'offset de la requête SQL comme paramètre. A celui-ci je préfère largement un numéro de page, c'est bien plus cohérent il me semble (et pourrait être traité par jPageCounter justement).

@bballizlife :

Pour ton point 1 : rien n'empechera à fournir un autre plugin qui fonctionnerait sur le principe dont tu parles.

Pour le point 2 : d'une part, la version initiale de ce plugin fonctionnait avec des indexes sur une liste d'enregistrement, autant donc continuer comme ça pour ne pas trop casser l'ancien fonctionnement. D'autre part, fonctionner sur des vrais numéros de pages, implique dans l'action qui affiche la liste, de calculer soi même l'offset qui correspond au numéro de page (puisqu'on n'a pas encore de jPageCounter). Donc c'est moins facile pour le développeur. (et chiant pour être honnête).

Ceux qui voudront des vrais numéros de page, utiliseront le future autre plugin avec la future classe jPageCounter &cie ;-)

Dans ce cas, ok, pas de souci ! :)