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

ETQ admin et instructeur, j'ai un pied de page #9817

Merged
merged 4 commits into from
Dec 12, 2023
Merged

ETQ admin et instructeur, j'ai un pied de page #9817

merged 4 commits into from
Dec 12, 2023

Conversation

krichtof
Copy link
Contributor

@krichtof krichtof commented Dec 7, 2023

close #9784

Avec cette PR, les administrateurs et instructeurs peuvent désormais voir un pied de page qui leur permet notamment d'accéder à la documentation et de proposer des améliorations.

2023-12-07-150348_801x94_scrot

@krichtof krichtof force-pushed the 9784-footer branch 4 times, most recently from 53fda01 to cefc7c5 Compare December 7, 2023 16:35
@krichtof krichtof marked this pull request as ready for review December 7, 2023 17:01
Copy link
Member

@colinux colinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je te cache pas que ça me gène un peu ce mic-mac de layouts (peut-être car jsuis fatigué), en tout car ça me semble fragile, et pour la nav bar qui a le même genre de logique, on a une solution différente qui me semble plus flexible.

Là l'idée qui me vient (qui n'est peut-être pas meilleure), ce serait de l'intégrer directement dans layout/application quand on a pas de content_for :footer et en réutilisant le même mécanisme que la navigation pour savoir si on est dans un contexte admin, instructeur ou expert (cf app/views/layouts/_header.haml lignes ~5 et ~80). En fait je me dis qu'il faudrait arriver l'unifier et renommer le nav_bar_profile vu que c'est le même type de logique. Mais ça demande à être creusé.

Bref je voudrais bien y réfléchir un peu plus ;)

@mfo
Copy link
Contributor

mfo commented Dec 8, 2023

Dans la même logique que Colin, je pense qu'on peut homogénéiser :

ETQ usager, sur une page quelconque, il y a la mention d'accéssibilité (et la mention sur les droits) :

Screenshot 2023-12-08 at 5 57 12 PM

Ça me semble interessant de l'avoir aussi pour les admin/instructeur.

A l'inverse, ETQ admin/instructeur, j'ai les page documentation / améliorer le site. Se serait pas mal de les ajouter aux usager non ?

EQT experts, j'ai pas de footer

Conclusion

Je me demande, pourquoi ils ne partageraient pas tous un même footer ?

Screenshot 2023-12-08 at 5 57 12 PM

Avec en plus les liens doc & améliorer le site ?

@krichtof
Copy link
Contributor Author

@mfo Sur l'accessibilité, il me semble qu'on a audité uniquement la partie usagers. L'interface admin ne faisait pas partie du périmètre.

@colinux pourquoi pas tout mettre dans layout/application, je me dis que ça vaut le coup d'y réfléchir, si ça rend le code plus lisible.

based on old 'users/dossiers/_index_footer'
@krichtof
Copy link
Contributor Author

J'ai tout réécrit en essayant d'intégrer vos suggestions.

La logique est la suivante : un seul layout application. Un seul footer que j'ai mis dans le répertoire application, qui est rendu s'il n'y a pas de content_for(:footer).
Et dans le footer, si c'est un agent, j'ajoute les liens doc et featureupvote

Copy link
Member

@colinux colinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ça me plait bien, super ! j'ai vérifié aussi de mon côté et ça marche bien avec les spécificités du footer usager

@krichtof krichtof added this pull request to the merge queue Dec 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2023
@krichtof krichtof added this pull request to the merge queue Dec 12, 2023
Merged via the queue into main with commit fbf0ccd Dec 12, 2023
15 checks passed
@krichtof krichtof deleted the 9784-footer branch December 12, 2023 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ETQ Admin, on ajoute un footer avec doc et feature upvotes
3 participants