11523 sujets

JavaScript, DOM et API Web HTML5

[Résolu] Revue de code pour un menu accordéon

Bonjour,

Fut une époque où Alsacréations avait créé son propre tutoriel pour un menu accordéon... et c'est comme cela que j'avais découverts ce forum...

Il y a environ deux semaines j'ai peaufiné un menu accordéon en remplacement d'une première version écrite en jQuery il y a 4/5 ans.

Cette première version s'appuyait sur une liste html et la remappait. La nouvelle version fait de même mais s'appuie cette fois sur les éléments details/summary et est écrite en ES6. J'assume la non rétrocompatibilité avec les navigateurs anciens : ce script s'intégrera à mon prochain framework (plus seulement front mais aussi back) que je prévoie opérationnel pour... dans deux ans, alors bon...

Le CodePen : Accordion v2

Notez que l'apparence de l'accordéon ne changera pas si javascript est désactivé (vous pouvez vous amuser à supprimer le script pour voir). Seul les animations et les comportements optionnels auront disparus.

Voilà, voilà. Le code est fonctionnel mais peut sans doute être optimisé. Je n'ai rien à gagner à ce partage sinon une revue de code de votre part qui me permettrait de m'améliorer. Je suis ouvert à toutes les critiques constructives :
- sur le script
- sur l'accessibilité
- sur le design
- etc...

N'hésitez-pas. Merci.
Modifié par Olivier C (26 Jun 2020 - 06:51)
Modérateur
Hello,

Pas assez bon en JS pour t'aider, un détail qui me dérange à la lecture, c'est pourquoi detailss/details et pas details/detail ?

Un autre point, est que tu ne gères pas le redimessionement de la fenetre et donc le max-height n'a plus une bonne valeur…

Dommage qu'en stylus, les nested ne sont pas autorisé, ça allégerait énormément la lisibilité. Et pourquoi pas des utiliser des variables CSS ? Dernière chose qui peut paraitre un détail mais pour les couleurs, pourquoi utiliser une majuscule ?

Voilà pour moi Smiley smile
Yordi a écrit :
un détail qui me dérange à la lecture...

C'est le cas de le dire !

Yordi a écrit :
... c'est pourquoi detailss/details et pas details/detail ?

Moi aussi je me suis posé la question en cherchant le nom de ma variable, mais "details" est le nom de l'élément, et quand je cherche un élément dans une boucle for of j'utilise toujours cette convention, du coup je n'ai pas voulu déroger à la règle...

Yordi a écrit :
Un autre point, est que tu ne gères pas le redimensionnement de la fenêtre et donc le max-height n'a plus une bonne valeur…

Cette règle n'est pas liée à l'accordéon mais ajoutée pour la démo. Toutefois chez moi c'est quand-même responsive...

Yordi a écrit :
Dommage qu'en stylus, les nested ne sont pas autorisé, ça allégerait énormément la lisibilité.

Si si on peut, mais c'est vrai que je ne les utilise pas. Je devrais m'y mettre peut-être mais je n'ai jamais eu de problème pour lire mon code ainsi...

Yordi a écrit :
Et pourquoi pas utiliser des variables CSS ?

Ce sera le cas lorsque ce module front sera intégré définitivement dans le framework pour lequel il a été conçu. Ce ne seront peut-être pas des variables CSS mais celle du préprocesseur que j'utilise. Ceci dit j'en suis un peu revenu des variables à tout va et de l'héritage qui peut aller avec, ça peut-être vite une usine à gaz.
Yordi a écrit :
Dernière chose qui peut paraître un détail mais pour les couleurs, pourquoi utiliser une majuscule ?

Les deux conventions sont possibles.
Modifié par Olivier C (26 Jun 2020 - 12:54)
Administrateur
Bonjour,

je viens de rapidement tester ton Codepen et - du point de vue accessibilité - son fonctionnement semble plutôt propre (pas de défaut qui me saute aux yeux). On peut l'utiliser au clavier, il y a des boutons et le contenu suit immédiatement son bouton.
Comme pistes d'améliorations j'en vois 2 en priorité : indiquer l'état du bouton/contenu (ce dernier est-il caché ou visible ?) en ARIA et utiliser des titres autour des boutons. Je ne suis pas suffisamment au courant des détails de tous les Design Patterns ARIA (je sais surtout quelle masse de boulot ça représente Smiley eek ) mais quelques ressources :
- l'implémentation Van11y de N. Hoffmann vue et revue par Goetsu et quelques autres
- supers explications, mises en contexte, etc sur le projet européen Access&Use.eu
EDIT : je n'ai pas regardé ton code JS ; seulement le résultat
Modifié par Felipe (26 Jun 2020 - 15:05)
Je vous remercie tous deux pour vos interventions, j'ai commencé à lire vos suggestions et vais regarder ça de près.

Si quelqu'un veut faire des remarques sur le code JS qu'il n'hésite pas.
Salut,

Dans l'ensemble ton code est propre et ne contient pas de bug. Maintenant il faudrait l'aérer et le rendre plus lisible car tout est entassé dans une seule fonction. Par exemple on note 2 parties dans cette fonction :
- le remplacement des éléments HTML
- la gestion de l'événement click
En séparant ces 2 fonctionnalités, on y verrait plus clair. En continuant le processus de séparation des fonctionnalités, on peut découper le programme en tout un tas de petites fonctions faciles à comprendre et à modifier.

L'autre point important est le nom des variables et des fonctions. En les nommant judicieusement, on peut arriver à comprendre le code simplement en le lisant, sans devoir réfléchir à ce que l'auteur a voulu faire. Cela implique de choisir des noms qui représentent les éléments fonctionnels et pas les éléments techniques. Par exemple, tu utilises detailss pour nommer la liste des balises <details> de ta page. Ces balises représentent tes menus, donc on pourrait par exemple les appeler menuElements. Cela permet de comprendre plus facilement les données que le code manipule et ce qu'il en fait.

Je me suis essayé à un exercice de refactoring sur ton code pour que tu voies à quoi ça peut ressembler une fois le découpage et le renommage terminés. Là j'ai poussé l'exercice à l'extrême mais on n'est pas obligé d'aller aussi loin. Je n'ai pas modifié ton algorithme, simplement restructurer le code : https://codepen.io/ostara/pen/BajdPOO?editors=0011

L'étape suivante est de remplacer les fonctions par des classes, car toutes ces fonctions manquent de structure. Le code serait beaucoup plus lisible et simplifié sous forme de méthodes dans des classes.
Meilleure solution
Yordi a écrit :
Dommage qu'en stylus, les nested ne sont pas autorisé, ça allégerait énormément la lisibilité...

@Yordi je me suis essayé aux nested en révisant mon code CSS, mais je ne trouve pas ça aussi clair que cela : Accordion, styles updated

Ostara a écrit :
https://codepen.io/ostara/pen/BajdPOO

@Ostara : Beau boulot. Ce code n'est pas aussi lisible pour moi, mais il doit l'être dans un monde pro et c'est sans doute à cela que je dois tendre pour progresser... En tous les cas, chapeau bas, merci pour ton travail, je vais regarder ça de plus près.

@Felipe : j'ai commencé à lire quelques articles sur les aria et leur implémentation n'est effectivement pas simple, voir contre productive si elle est mal maîtrisée (par exemple navigation par tabulation remplacée par une navigation avec les flèches). Je vais voir ce que je peux faire de ce côté là aussi.

Dans tous les cas merci à vous pour vos interventions de différentes natures. Vous m'avez motivé pour prendre le temps de rester sur ce bout de code avant de passer à autre chose.
Modifié par Olivier C (28 Jun 2020 - 23:54)
Salut,
Ce n'est pas vraiment un code que l'on peut mettre en production. C'est seulement un exercice de factorisation pour s'entraîner à nommer correctement les variables et à découper son code en sous fonctions plus facilement lisibles.

L'objectif dépend évidemment de l'usage que tu fais de ton code. Si c'est un prototype temporaire, tu peux te permettre de manquer de lisibilité. Mais si tu comptes garder ton code et le faire évoluer, il y a des chances que dans 3 mois, tu ne te rappelles plus ce que tu as voulu faire. Sur 30 lignes ce n'est pas très grave, mais avec une application plus conséquente, cela peut vite devenir chronophage.

L'autre aspect est plus altruiste. Si ton code est amené à être lu par d'autres personnes, c'est faire preuve de respect que d'écrire un code facilement compréhensible, propre et structuré. Smiley cligne
Oui j'avais bien compris. Je te remercie encore pour ton intervention qui va me faire d'autant plus progresser qu'il s'agit d'un code concret et qui est le mien. Je séparerais le code JS en deux parties.

Je ne marque pas résolu pour me réserver la possibilité d'intervenir encore sur ce sujet si je fais évoluer le code suite aux remarques des uns et des autres.
Bon, après avoir lu pas mal d'articles sur aria je ne vais pas m'y coller, du moins pas en profondeur, et ce pour plusieurs raisons :

- déjà chacun dit la sienne, les documents ne sont pas toujours au point. Ne parlons même pas des tutoriels qui se contredisent ou fonds des erreurs d'implémentation dans leurs exemples, preuve que le sujet est vraiment mal maîtrisé (et les sites de ces derniers qui n'arrivent pas toujours à être responsives, c'est beau après de parler "d'accessibilité"...).
- la raison déjà évoquée plus haut, où les aria-* changent le mode de navigation au clavier, en passant de la tabulation aux flèches. Voici un exemple, avec pourtant ce qui se fait de mieux dans le domaine selon un article de MDN (en anglais) sur le sujet : Accordion jQuery UI. Je trouve ce point particulièrement rédhibitoire.
- enfin, le problème des tests : il faudrait connaître une personne utilisant ces fameux lecteurs d'écrans pour vraiment pouvoir s'y coller sérieusement, sinon on code vraiment au pif, pour ne pas dire "en aveugle" Smiley cligne .

Du coup je vais me contenter des rôles (tablist, etc) et de deux ou trois trucs tels que "aria-expanded".

Edit : il semble que j'en tiens un pas mal ici : w3.org, Accordion Example
Modifié par Olivier C (04 Jul 2020 - 21:29)
Finalement j'ai implémenté aria en intégralité.

En effet, le comportement par défaut du menu accordéon de jQuery UI est mauvais (même si plébiscité par une page du W3C pour ce qui est des arias) et m'avait induit en erreur : mais la mauvaise navigation sur cet accordéon est le fait d'une mauvaise sémantique html, pas des arias, contrairement à ce que j'avais pu lire par ailleurs.

L'implémentation complète est très verbeuse en aria et produit beaucoup d'IDs mais il me les faudra de toute manière lorsque j'adapterais ce script pour un menu onglet (en remplacement de celui-ci).
Modifié par Olivier C (05 Jul 2020 - 02:37)
Olivier C a écrit :
Bon, après avoir lu pas mal d'articles sur aria je ne vais pas m'y coller, du moins pas en profondeur, et ce pour plusieurs raisons :
- déjà chacun dit la sienne, les documents ne sont pas toujours au point. Ne parlons même pas des tutoriels qui se contredisent ou fonds des erreurs d'implémentation dans leurs exemples, preuve que le sujet est vraiment mal maîtrisé (et les sites de ces derniers qui n'arrivent pas toujours à être responsives, c'est beau après de parler "d'accessibilité"...).
- la raison déjà évoquée plus haut, où les aria-* changent le mode de navigation au clavier, en passant de la tabulation aux flèches. Voici un exemple, avec pourtant ce qui se fait de mieux dans le domaine selon un article de MDN (en anglais) sur le sujet : Accordion jQuery UI. Je trouve ce point particulièrement rédhibitoire.
- enfin, le problème des tests : il faudrait connaître une personne utilisant ces fameux lecteurs d'écrans pour vraiment pouvoir s'y coller sérieusement, sinon on code vraiment au pif, pour ne pas dire "en aveugle" Smiley cligne .
Du coup je vais me contenter des rôles (tablist, etc) et de deux ou trois trucs tels que "aria-expanded".
Edit : il semble que j'en tiens un pas mal ici : w3.org, Accordion Example

Hello Olivier,
Je suis arrivé à peu près à la même conclusion lorsque j'ai commencé à creuser le sujet accessibilité pour mon générateur de sites web.
Sujet somme toute relativement complexe avec une documentation et des comportements au niveau des navigateurs parfois contradictoires.
Ne voulant pas toutefois faire l'impasse sur l'accessibilité, j'ai intégré le minimum vital de façon à passer les tests de validation (actuallement note 100 sur Tanaguru), ce qui nécessite déjà un investissement non négligeable en tests et développement.
Pour info et de mémoire, il me semble avoir lu un article dont je ne trouve plus la référence mais qui concluait que la mise en place de zones pouvant être déployées n'était pas souhaitable dans le cas des personnes en situation de handicap, les zones en question complexifiant in fine la navigation et la rendant encore plus dépendante de la conformité ou non des lecteurs d'écrans / gestionnaires de clavier.
Sujet clos
Aller à