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

[fix] le dropdown pour les utilisateurs connectés #9313

Merged
merged 4 commits into from
Jul 13, 2023
Merged

[fix] le dropdown pour les utilisateurs connectés #9313

merged 4 commits into from
Jul 13, 2023

Conversation

lisa-durand
Copy link
Contributor

@lisa-durand lisa-durand commented Jul 12, 2023 •
edited
Loading

  • Ajout du label "connecté en tant que" pour se repérer plus facilement
  • Fix du lien "passer en admin" qui était encore présent mm quand on était connecté en tant qu'admin

APRES
Capture d’écran 2023-07-13 à 10 37 02

AVANT
Capture d’écran 2023-07-12 à 17 32 51

@what-the-diff
Copy link

what-the-diff bot commented Jul 12, 2023

PR Summary

  • New Profile Access Method
    A new method called nav_bar_profile is added in an important administrative file. This method would streamline how we access user profiles on our platform.

  • Dropdown Menu Enhancement
    The dropdown menu visible to users has seen some modifications:

    • The super admin link has been removed which was potentially confusing for regular users.
    • A handy link to view the currently logged-in user's profile is now included in the dropdown menu.
    • The descriptive text for user types (like user, instructor, admin, etc.) has been improved for better clarity.

  • Translation File Updates
    We have also updated our website's language files for English and French. This is to ensure that the new text additions & modifications are correctly translated for our users, facilitating an easier interface interaction for them.

@lisa-durand lisa-durand force-pushed the fix-dropdown-navbar-profile branch from d647d07 to 7b1b771 Compare July 12, 2023 16:07
mfo
mfo reviewed Jul 13, 2023
View reviewed changes
app/views/layouts/_account_dropdown.haml
- if multiple_devise_profile_connect?
%li
= link_to "#", class: "fr-nav__link", "aria-current" => "true" do
Copy link
Contributor

Choose a reason for hiding this comment

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

juste ça me fait bizarre que se soit un lien plutot qu'un span

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oui je suis d'accord mais comme on utilise le composant du DSFR, si on veut garder le style, sans trop bidouiller du CSS, ça reste le plus simple

Copy link
Contributor

Choose a reason for hiding this comment

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

fait sens, bon pr moi

@tchak
Copy link
Member

tchak commented Jul 13, 2023

On ne pourrai pas en profiter pour élargir le menu et faire en sorte que le texte tienne à chaque fois sur une ligne ? Je me demande aussi si on ne devrais pas garder l'icône sur l'action "se déconnecter" afin de la démarquer des autres.

@lisa-durand @tchak
Fix typo issue
e131bdd 
Co-authored-by: Paul Chavard <paul.chavard@beta.gouv.fr>
@lisa-durand
Copy link
Contributor Author

On ne pourrai pas en profiter pour élargir le menu et faire en sorte que le texte tienne à chaque fois sur une ligne ? Je me demande aussi si on ne devrais pas garder l'icône sur l'action "se déconnecter" afin de la démarquer des autres.

Pareil ici, comme on utilise le menu du DSFR, j'ai pas trouvé de manière simple de le surcharger sans rajouter trop de CSS. Pour l'icone, ça risque de créer des questions d'alignement mais peut-être avoir une couleur "rouge" ou qqchose qui se distingue pour le "déconnecter" en effet - je crois qu'il y a une sorte de "permission" du DSFR pour ce genre de cas.

@lisa-durand
Copy link
Contributor Author

Et rien à voir mais j'ai un souci de linter que je ne comprends pas - on me dit qu'il manque la key "connected_as" pour la version française, alors qu'elle est bien présente et qu'elle fonctionne 🤔

@lisa-durand
Copy link
Contributor Author

lisa-durand commented Jul 13, 2023 •
edited
Loading

@tchak au final j'ai pu fixer la width avec juste une classe CSS et ajouter les icones sans rajouter de css pour l'alignement - ça me parait en effet nettement mieux. J'ai testé avec la couleur rouge - ça ne fonctionnait pas. Pour moi c'est tout bon - mis à part cette histoire de linter si ça te parle ?
(J'ai MAJ la capture écran dans le descriptif de la PR)

@colinux
Copy link
Member

colinux commented Jul 13, 2023

sur l'i18n: peut-être car t'as rajouté des clés hors scope layouts dans le fichier, et ça vient mettre le bazar dans le namespace global (ici on est pas dans un component, toutes les clés sont mergées). Bref je suggère d'essayer sans ces clés, ou alors pas à la racine

@lisa-durand lisa-durand force-pushed the fix-dropdown-navbar-profile branch from 7d324fb to 074e3bf Compare July 13, 2023 10:48
@lisa-durand lisa-durand added this pull request to the merge queue Jul 13, 2023
Merged via the queue into main with commit 9bd6cea Jul 13, 2023
@lisa-durand lisa-durand deleted the fix-dropdown-navbar-profile branch July 13, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfo mfo mfo approved these changes

@tchak tchak tchak left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lisa-durand @tchak @colinux @mfo