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.

Sign up for GitHub

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

Jump to bottom

[BUGFIX] Mettre à jour et rajouter les attributs nécessaire sur la navbar de Pix app (PIX-16773) #11630

Merged
merged 2 commits into from
Apr 4, 2025
Merged

[BUGFIX] Mettre à jour et rajouter les attributs nécessaire sur la navbar de Pix app (PIX-16773) #11630

merged 2 commits into from
Apr 4, 2025

Conversation

theotime2005
Copy link
Contributor

@theotime2005 theotime2005 commented Mar 11, 2025 •
edited
Loading

🥞 Problème

On a mis à jour pix-ui pour pouvoir nommer le bouton pour ouvrir la navbar sur la page d'accueil de Pix app. On doit maintenant mettre à jour le composant et l'adapter sur Pix app.

🥓 Proposition

  • Ajouter à l'appel du composant les attributs openLabel et closeLabel en leur donnant des labels traduits,
  • Traduire ses nouveaux labels.

🧃 Remarques

Le aria-label de la navbar était écrit en dur dans le code, on l'a placé dans les traductions et traduit au passage.

😋 Pour tester

  • Se rendre sur Pix app,
  • Se connecter avec un compte comme alex-terieur@example.net,
  • Mettre la page en petit format (mobile),
  • En inspectant la page, constater que sur le bouton pour ouvrir la navbar est écrit "Ouvrir la navigation",
  • Cliquer dessus,
  • Constater que sur le bouton pour fermer la navbar est écrit "Fermer la navigation".

@theotime2005 theotime2005 added 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally team-evaluation PR relatives à l'expérience d'évaluation 👀 Design Review Needed cross-team Toutes les équipes de dev dependencies Pull requests that update a dependency file labels Mar 11, 2025
@theotime2005 theotime2005 self-assigned this Mar 11, 2025
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 298b365 to e4555cf Compare March 11, 2025 14:48
@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 :

@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch 2 times, most recently from 918da80 to 440beee Compare March 12, 2025 08:32
@Faraopix Faraopix added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed Need PO validation for this functionally labels Mar 12, 2025
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch 2 times, most recently from c0788a0 to 9c152af Compare March 17, 2025 10:41
@HEYGUL HEYGUL force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 9c152af to ee9315f Compare March 17, 2025 10:56
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from ee9315f to 1b81404 Compare March 17, 2025 15:54
@theotime2005 theotime2005 removed Func Review OK PO validated functionally the PR dependencies Pull requests that update a dependency file labels Mar 17, 2025
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 1b81404 to f2af781 Compare March 17, 2025 20:47
@theotime2005 theotime2005 added 👀 Func Review Needed Need PO validation for this functionally and removed 🆘 Help needed labels Mar 17, 2025
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from f2af781 to b3dff94 Compare March 18, 2025 08:47
@Jeyffrey
Copy link
Contributor

Jeyffrey commented Mar 18, 2025 •
edited
Loading

image

Ça me va bien côté code sur PixApp.

Mais je me questionne côté composant PixUI.
À la base, on ne souhaite pas avoir de label visible sur la page (sur l'image, on peut se rendre compte que le hover dépasse sur la droite aussi).
Devrait-on avoir l'équivalent d'un .screen-reader-only / .sr-only qui cacherait l'élément visuellement ?

cc @xav-car @AndreiaPena

@theotime2005
Copy link
Contributor Author

image Ça me va bien côté code sur PixApp.

Mais je me questionne côté composant PixUI. À la base, on ne souhaite pas avoir de label visible sur la page (sur l'image, on peut se rendre compte que le hover dépasse sur la droite aussi). Devrait-on avoir l'équivalent d'un .screen-reader-only / .sr-only qui cacherait l'élément visuellement ?

cc @xav-car @AndreiaPena

Je croyais que ce qui était en aria-label n'était visible que pour les lecteurs d'écran?

@Jeyffrey
Copy link
Contributor

Jeyffrey commented Mar 18, 2025 •
edited
Loading

Je croyais que ce qui était en aria-label n'était visible que pour les lecteurs d'écran?

Oui mais, dans le code de PixNavigation, il se trouve que ce n'est pas un aria-label qui a été utilisé mais la string passée directement en dur dans le code.

https://github.com/1024pix/pix-ui/blob/dev/addon/components/pix-navigation.hbs#L13

@theotime2005
Copy link
Contributor Author

theotime2005 commented Apr 1, 2025 •
edited
Loading

@Jeyffrey @xav-car @AndreiaPena que fait-on pour ceci alors????
Il y aura aussi Pix admin à modifier

@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from b3dff94 to 632902a Compare April 3, 2025 09:52
@theotime2005 theotime2005 requested review from AndreiaPena and a team April 3, 2025 09:55
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 632902a to 7daf0a4 Compare April 4, 2025 08:12
xav-car
xav-car approved these changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@xav-car xav-car left a comment

Choose a reason for hiding this comment

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

tech ok

@xav-car xav-car force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 7daf0a4 to 4a2dcbf Compare April 4, 2025 09:42
@theotime2005 theotime2005 force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from 4a2dcbf to b93ebf6 Compare April 4, 2025 09:48
@theotime2005 theotime2005 added Tech Review OK 🚀 Ready to Merge Func Review OK PO validated functionally the PR and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Apr 4, 2025
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-16773-update-and-fix-navbar-for-pix-app branch from b93ebf6 to b6a945e Compare April 4, 2025 10:02
@pix-service-auto-merge pix-service-auto-merge merged commit 86fbf3b into dev Apr 4, 2025
9 of 11 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-16773-update-and-fix-navbar-for-pix-app branch April 4, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xav-car xav-car xav-car approved these changes

@alicegoarnisson alicegoarnisson alicegoarnisson approved these changes

@Jeyffrey Jeyffrey Awaiting requested review from Jeyffrey

@Faraopix Faraopix Awaiting requested review from Faraopix

@AndreiaPena AndreiaPena Awaiting requested review from AndreiaPena

Assignees

@theotime2005 theotime2005

Labels
cross-team Toutes les équipes de dev Design Review OK Func Review OK PO validated functionally the PR 🚀 Ready to Merge team-evaluation PR relatives à l'expérience d'évaluation team-prescription Tech Review OK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@theotime2005 @pix-bot-github @Jeyffrey @xav-car @alicegoarnisson @pix-service-auto-merge @Faraopix