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

[FEATURE] Ajouter la nouvelle barre de navigation dans Pix Admin (PIX-16550) #11404

Merged
merged 9 commits into from
Feb 14, 2025

Conversation

er-lim
Copy link
Contributor

@er-lim er-lim commented Feb 12, 2025

🥞 Problème

La nouvelle barre de navigation n'était pas encore utilisée sur Pix Admin

🥓 Proposition

L'ajouter avec un feature toggle (FT_PIX_ADMIN_NEW_SIDEBAR_ENABLED) le temps que ça soit validé fonctionnellement.

🧃 Remarques

  • On en profite pour mettre un logo pour Pix Admin 😄

😋 Pour tester

En local

  • Ajouter dans votre .env côté API la variable FT_PIX_ADMIN_NEW_SIDEBAR_ENABLED=true
  • Vérifier que la nouvelle barre s'affiche.
  • Faire des tests de non régression sur ça.

En RA

  • Se connecter à la RA de Pix Admin
  • Vérifier qu'il n'y a pas de régressions sur les pages

@er-lim er-lim added the cross-team Toutes les équipes de dev label Feb 12, 2025
@er-lim er-lim self-assigned this Feb 12, 2025
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@er-lim er-lim force-pushed the tech-add-pix-navigation-in-admin branch 4 times, most recently from a7018be to 808f67c Compare February 12, 2025 13:32
@er-lim er-lim changed the title [FEATURE] Ajouter la nouvelle barre de navigation dans Pix Admin (PIX-DUNNO) [FEATURE] Ajouter la nouvelle barre de navigation dans Pix Admin (PIX-16550) Feb 12, 2025
@AndreiaPena
Copy link
Member

Proposition mettre un aria-label avec le label en complet pour au moins le lire entièrement en lecteur d'écran Capture d’écran 2025-02-12 à 15 47 29

@AndreiaPena
Copy link
Member

AndreiaPena commented Feb 12, 2025

Proposition : sur la classe .page-body > remplacer padding par padding-top: var(--pix-spacing-3x);

Pour avoir ça :)
Capture d’écran 2025-02-12 à 15 52 57

Et sur .page-header un border-radius: 5px; :D

@AndreiaPena
Copy link
Member

AndreiaPena commented Feb 12, 2025

D'ailleurs Profil cible n'a pas de fond blanc dans sa section et je trouve le résultat pas si mal !

Capture d’écran 2025-02-12 à 15 57 06

On se rend compte que les blocs blancs prennent finalement beaucoup de place, et donc si on retire le padding, c'est déjà ça de gagné

Capture d’écran 2025-02-12 à 15 57 58

Argument pour faire passer ce menu en PROD : on peut gagner un chouilla d'espace :p

admin/tests/integration/components/layout/sidebar_test.gjs Outdated Show resolved Hide resolved
admin/translations/en.json Outdated Show resolved Hide resolved
admin/app/components/layout/sidebar.gjs Show resolved Hide resolved
admin/tests/integration/components/layout/sidebar_test.gjs Outdated Show resolved Hide resolved
@er-lim er-lim force-pushed the tech-add-pix-navigation-in-admin branch 2 times, most recently from f6d2bf3 to 9a6537d Compare February 13, 2025 10:19
@Faraopix
Copy link
Contributor

Les entrées dans la nav sont très collées-serrées par rapport à ce que je vois sur storybook ou sur les autres app :
RA
pixui

J'ai fait un petit tour sinon, fonctionnellement je ne vois rien d'anormal 👍

@Faraopix
Copy link
Contributor

Et je ne sais pas si ça a un rapport mais je vois que "Type" ne s'affiche pas en entier :
Capture d’écran 2025-02-13 à 13 59 51

@QuentinChapelain-ui
Copy link

QuentinChapelain-ui commented Feb 13, 2025

Hello sur les différents retours :

  • Proposition mettre un aria-label avec le label en complet pour au moins le lire entièrement en lecteur d'écran : + 1 pour moi

  • Pour le titre en haut de page, on peut enlever le fond blanc il n'apporte rien et rends moins ISO l'interface avec les autres plateforme orga et certif. Pour consrver un spacing et faciliter la lecture on peut comme sur app ajouter une ligne / seperateur en dessous de 1px gris pix-neutral-100.

Capture d’écran 2025-02-13 à 15 49 44
  • Je rejoins le retour de Géraldine au sujet du spacing des items du menu, il y. effectivment un espace sur orga. C'est un ajustement local à admin ?
Capture d’écran 2025-02-13 à 15 45 10 Capture d’écran 2025-02-13 à 15 47 48

@er-lim er-lim force-pushed the tech-add-pix-navigation-in-admin branch 2 times, most recently from 8127766 to c1e0787 Compare February 13, 2025 16:35
@er-lim
Copy link
Contributor Author

er-lim commented Feb 13, 2025

Je fais un message groupé par rapport à tous vos retours (merci pour les feedbacks 🙏 ). J'en ai traité une partie :

Les entrées dans la nav sont très collées-serrées par rapport à ce que je vois sur storybook ou sur les autres app :

@Faraopix C'est corrigé !

Et je ne sais pas si ça a un rapport mais je vois que "Type" ne s'affiche pas en entier :

@Faraopix Non ça n'a pas de rapport, c'est le composant PixSelect qui n'a pas calculé la bonne taille par rapport aux options. Je ne l'ai pas traité 😛

Proposition mettre un aria-label avec le label en complet pour au moins le lire entièrement en lecteur

@AndreiaPena Fait !

Proposition : sur la classe .page-body > remplacer padding par padding-top: var(--pix-spacing-3x);
Et sur .page-header un border-radius: 5px; :D

@AndreiaPena C'est corrigé 😄

Pour le titre en haut de page, on peut enlever le fond blanc il n'apporte rien et rends moins ISO l'interface avec les autres plateforme orga et certif. Pour consrver un spacing et faciliter la lecture on peut comme sur app ajouter une ligne / seperateur en dessous de 1px gris pix-neutral-100.

@QuentinChapelain-ui c'est fait, par contre il n'y a pas de FT appliqué. On verra donc cette modification dans la version avec l'ancien menu.

Ajouter un spacing entre l'input et le bouton "Charger"

@QuentinChapelain-ui done !

quand un item du menu est actif, l'icône devrait être fill. c'est le cas dans le DS : https://www.figma.com/design/8RJ3aCSfdeQ8AZZVBBYKS8/Design-System-Nebulix?node-id=7018-10270&t=8xNyMEDFvAkP5DbK-1

@QuentinChapelain-ui c'est au composant de gérer ça, la gestion devra être faite sur Pix UI

@AndreiaPena AndreiaPena force-pushed the tech-add-pix-navigation-in-admin branch from c1e0787 to ec3d343 Compare February 14, 2025 08:46
@AndreiaPena AndreiaPena self-assigned this Feb 14, 2025
@AntoineLPix
Copy link

AntoineLPix commented Feb 14, 2025

Les identifiants externes d'Orga et Centres qui sont longs débordent du tableau
image

La recherche utilisateur sur 13 pouces (mise à l'échelle 125% - NB on recommande 150% sur win11) avec affichage à 100% dans le navigateur ne tient plus sur une ligne, il faut passer à 80% dans le nav pour avoir les champs et boutons sur une ligne
image

Les adresses e-mails longues ne rentrent pas dans le tableau (ici page équipe de Pix Admin)
image

Tableau des sessions ne tient pas dans un écran 13 pouces (mise à l'échelle 125%) avec affichage à 100% dans le navigateur, il faut passer à 70%
image

Pas de souci sur 24 pouces mise à l'échelle 100% (reco win 11)

@AndreiaPena AndreiaPena force-pushed the tech-add-pix-navigation-in-admin branch 2 times, most recently from 856b8f1 to 46d9b7b Compare February 14, 2025 16:39
@AndreiaPena AndreiaPena force-pushed the tech-add-pix-navigation-in-admin branch from 46d9b7b to a7c5712 Compare February 14, 2025 18:16
@AndreiaPena AndreiaPena added 🚀 Ready to Merge and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Feb 14, 2025
@pix-service-auto-merge pix-service-auto-merge merged commit 55f2027 into dev Feb 14, 2025
10 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the tech-add-pix-navigation-in-admin branch February 14, 2025 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants