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

[FEATURE] Ajouter les boutons de navigation et le séparateur (pix-15217) #756

Merged
merged 1 commit into from
Nov 13, 2024
Merged

[FEATURE] Ajouter les boutons de navigation et le séparateur (pix-15217) #756

merged 1 commit into from
Nov 13, 2024

Conversation

lionelB
Copy link
Contributor

@lionelB lionelB commented Nov 6, 2024

🎄 Problème

La navigation necéssite des boutons avec un design particulier

🎁 Proposition

On ajoute un composant PixNavigationButton

🌟 Remarques

On profite de la PR pour rajouter aussi le PixNavigationSeparator

🎅 Pour tester

Afficher la storie de PixNavigation et voir les jolis bouton et separateur

@lionelB lionelB added the cross-team Toutes les équipes de dev label Nov 6, 2024
@pix-bot-github
Copy link

Une fois l'application déployée, elle sera accessible à cette adresse https://ui-pr756.review.pix.fr
Les variables d'environnement seront accessibles sur scalingo https://dashboard.scalingo.com/apps/osc-fr1/pix-ui-review-pr756/environment

@xav-car xav-car changed the base branch from dev to pix-13150-new-tabs-component November 7, 2024 09:45
@xav-car xav-car changed the base branch from pix-13150-new-tabs-component to dev November 7, 2024 09:46
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 78dd9df to e6f130a Compare November 7, 2024 14:13
@lionelB lionelB changed the base branch from dev to pix-14553/chrome-and-shiny-navigation November 7, 2024 14:13
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch 2 times, most recently from 849a49a to 6bb2b6d Compare November 7, 2024 14:33
@AndreiaPena AndreiaPena changed the title [FEATURE] ajoute les bouton de navigation et le séparateur (pix-15217) [FEATURE] Ajouter les boutons de navigation et le séparateur (pix-15217) Nov 7, 2024
@AndreiaPena
Copy link
Member

Capture d’écran 2024-11-07 à 17 55 23

Est-ce que ce bouton permettant d'afficher une liste est prévue dans la création du PixNavigation ?

@lionelB lionelB force-pushed the pix-14553/chrome-and-shiny-navigation branch from be19980 to 18be0c0 Compare November 7, 2024 17:03
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 6bb2b6d to 5fdfb18 Compare November 7, 2024 17:14
@lionelB
Copy link
Contributor Author

lionelB commented Nov 7, 2024

Est-ce que ce bouton permettant d'afficher une liste est prévue dans la création du PixNavigation ?

il sera fait dans la PR #757

AndreiaPena
AndreiaPena reviewed Nov 7, 2024
View reviewed changes
addon/components/pix-navigation-button.hbs
@@ -0,0 +1,35 @@
{{#if @route}}
Copy link
Member

@AndreiaPena AndreiaPena Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Info : On a un cas de figure coté Certif où le bouton de l'espace surveillant doit ouvrir un nouvel onglet.
Mais avec cette implémentation, je n'ai pas pu utiliser @route et @target ensemble.
J'ai du faire un target="_blank" pour contourner le soucis. Juste pour info :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok merci pour le retour, je vais corriger pour qu'on ait la même api et qu'on fournisse le @target="_blank"

@AndreiaPena
Copy link
Member

Intégré avec succès sur Pix Certif 🥳

La branche est là si besoin de faire une PR de test pour le design : test-pix-navigation-on-pix-certif

Capture d’écran 2024-11-07 à 18 40 36

@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch 2 times, most recently from fcdf6f7 to 8188967 Compare November 7, 2024 18:00
@pierrepougetpix
Copy link

Hello 😊

Voici les points à corriger :

  • text & icon : color: pix-neutral-0
  • pix-separator: color: pix-neutral-0
  • pix-navigation_nav : font-family: "Nunito"
  • bouton "J'ai un code" : background: pix-neutral-0
  • bouton "Changer" : border-color: pix-neutral-0; color: pix-neutral-0
  • bouton "Se déconnecter" : color: pix-neutral-0
  • pix-app-layout: appliquer --pix-spacing-6x pour gap et padding

@pierrepougetpix
Copy link

Intégré avec succès sur Pix Certif 🥳

La branche est là si besoin de faire une PR de test pour le design : test-pix-navigation-on-pix-certif

Capture d’écran 2024-11-07 à 18 40 36

Pas OK niveau design… 😕
Le gap et les marges ne sont pas bonnes

@lionelB lionelB force-pushed the pix-14553/chrome-and-shiny-navigation branch 2 times, most recently from 9621563 to dd7e1e1 Compare November 13, 2024 08:16
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 8188967 to 5d46bd4 Compare November 13, 2024 08:16
@lionelB lionelB force-pushed the pix-14553/chrome-and-shiny-navigation branch from dd7e1e1 to 0c57ada Compare November 13, 2024 08:28
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 5d46bd4 to 523fe24 Compare November 13, 2024 08:29
@pierrepougetpix
Copy link

Ajustement :

  • pix-navigation_nav : font-family: "Nunito" regular quand l'item-nav n'est pas sélectionné

@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 523fe24 to 7cddee9 Compare November 13, 2024 10:39
@lionelB lionelB force-pushed the pix-14553/chrome-and-shiny-navigation branch from 0c57ada to 5b0d2af Compare November 13, 2024 11:07
@lionelB lionelB force-pushed the pix-15217/add-navigation-button branch from 7cddee9 to 2dc9d9b Compare November 13, 2024 11:07
@pix-service-auto-merge pix-service-auto-merge merged commit 9da8676 into pix-14553/chrome-and-shiny-navigation Nov 13, 2024
7 of 8 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-15217/add-navigation-button branch November 13, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreiaPena AndreiaPena AndreiaPena approved these changes

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

@pierrepougetpix pierrepougetpix Awaiting requested review from pierrepougetpix

Assignees

@lionelB lionelB

@machestla machestla

Labels
cross-team Toutes les équipes de dev Design review OK Func Review OK 🚀 Ready to Merge Tech review OK ⚠️ PR Inheritance :warning: PR Inheritance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@lionelB @pix-bot-github @AndreiaPena @pierrepougetpix @xav-car @machestla @pix-service-auto-merge