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

Refonte de l'archi des routes avec les "nested routes" de Vue #522

Merged
merged 7 commits into from
May 30, 2024

Conversation

ddahan
Copy link
Contributor

@ddahan ddahan commented May 25, 2024

D'où vient le besoin ?

  1. En essayant de créer une page contenant une lettre officielle type "déclaration acceptée", qui aurait vocation à être convertie en PDF, on se rend compte que ce n'est pas possible car le fichier App.vue contient déjà tout le templating compl'alim (header, footer, etc.). Or, nous on ne veut aucun templating sur cette page.

  2. Travail préparatoire de la feature qui affichera les infos "par entreprise" dans l'url front

Comment on remédie à ça ?

On modifie le fichier App.vue pour qu'il ne contienne pas de templating (à part les toasts qui ont vocation à être globaux).

Ensuite, grâce aux nested routes et l'utilisation de children, on peut définir :

  • un ensemble de pages enfants (ex : accueil, blog, ...) qui vont hériter du layout Main définie dans la classe mère, qui contient nos headers/footers compl'alim.
  • une autre page qui n'est pas liée à un layout, et qui donc ne les affichera pas.

2e utilisation

Dans la foulée, j'ai aussi essayé de grouper les pages "pro" avec un autre layout appelée Pro qui ne contient que la rôle bar et un fr-container.
Cela signifie que :

  • dès que l'utilisateur est sur une page commençant par "/pro", il aura la rolebar affichée.
  • la logique authenticationRequired: true peut n'être séttée qu'une seule fois sur la route parent ! Cela évite les oublis.
  • le tout est globalement plus propre car organisé de manière hiérarchique.

Exemple : on voit ici que la rolebar est affichée car on est dans une page fille de "l'espace pro".

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Le dossier layouts est insipiré par la logique de Nuxt: https://nuxt.com/docs/guide/directory-structure/layouts

authenticationRequired: true,
requiredRole: "InstructionRole",
},
path: "/lettre-officielle",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cette page s'affiche bien seule, sans aucun templating compl'alim autour

@@ -260,7 +273,8 @@ function chooseAuthorisedRoute(to, from, next, store) {
next({ name: "LandingPage" })
})
} else {
if (to.meta.home) next({ name: store.loggedUser ? "DashboardPage" : "LandingPage" })
// 2) vérifie les règles de redirection
if (to.path === "/") next({ name: store.loggedUser ? "DashboardPage" : "LandingPage" })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai du utilisé le path, plutôt que meta.home pour décrire cette redirection, car si on set home: True sur la page mère, toutes les pages children en héritent, et on a une boucle infinie de redirection.

@ddahan ddahan marked this pull request as ready for review May 28, 2024 07:38
@ddahan ddahan requested a review from alemangui May 28, 2024 07:38
@alemangui
Copy link
Collaborator

alemangui commented May 29, 2024

Petit comment avant la review

Une possibilité plus succincte et flexible serait d'avoir un paramètre meta "fullscreen" (exemple chez MC) :

 {
    path: "/diagnostic-tunnel/:canteenUrlComponent/:year/:measureId",
    name: "DiagnosticTunnel",
    component: DiagnosticTunnel,
    meta:
      fullscreen: true,
    },
  },

Et dans l'App.vue on a quelques v-ifassez simples (link) :

<AppHeader v-if="!fullscreen" />

Cela dit ce n'est pas mal d'avoir les routes nestés à ce niveau aussi, sauf pour un truc : on aura probablement des vues fullscreen à l'intérieur des URLs qui ne le sont pas. Exemple : un tunnel - ça ferait qu'une vue hypothétique /declarations aurait le header et footer, et /declarations/13/modifier rentrerait dans un tunner sans header ni footer. Ça enleverait l'enfant "logique" de l'URL du fichier router.

Je suis un peu mitigé par rapport à la sous-route /pro qui inclut automatiquement la bannière car sur plusieurs pages destinées aux professionnels on ne voudrait pas forcément afficher ce bandeau, surtout les longs formulaires pour éviter de prendre plus d'espace vertical. T'en penses quoi d'éventuellement laisser cette fonctionnalité pour après (ou de pouvoir la surcharger avec un meta ?)

@ddahan
Copy link
Contributor Author

ddahan commented May 29, 2024

  • Pas super fan de la solution en v-if avec les meta pour gérer ce problème là, je trouve que les nested routes sont une solution plus "naturelle" , plus scalable quand on commence à avoir plusieurs cas, et plus lisible via le routeur. Sachant qu'on devra les utiliser ailleurs très probablement.
  • Pour la bannière qu'on veut peut être pas afficher dans tous les cas, je vois. C'est peut être plus un problème de choix d'UI qu'un soucis de nested routes. Du coup je te propose de garder la nested route /pro (ou un autre nom) mais remettre la bannière uniquement sur le dashboard en dur pour le moment (et donc retirer le layout "pro" qui ne sert plus à rien). Mais je trouve ça logique d'avoir une sous-section de routes pour les utilisateurs connectés, au moins pour éviter d'avoir à re-spécifier les règles authenticationRequired des pages à chaque fois.

ça ferait qu'une vue hypothétique /declarations aurait le header et footer, et /declarations/13/modifier rentrerait dans un tunner sans header ni footer. Ça enleverait l'enfant "logique" de l'URL du fichier router.

Je pense qu'il faudrait dans l'idéal éviter ce genre de cas parce que ça peut être assez confusant d'alterner entre une UI avec le header Compl'alim, et une UI fullscreen selon les features. Eventuellement ça, ça marche bien pour les login/signup, mais après une fois connecté, j'imagine l'utilisateur faire ses démarches toujours avec une UI constante, non ?

@alemangui
Copy link
Collaborator

alemangui commented May 29, 2024

@ddahan C'est un pattern assez courant lors qu'on est sur un processus/flow, par ex :
Screencast from 29-05-24 14:43:18.webm

Du coup je te propose de garder la nested route /pro (ou un autre nom) mais remettre la bannière uniquement sur le dashboard en dur pour le moment (et donc retirer le layout "pro" qui ne sert plus à rien).

Ça m'irait 👍

@ddahan ddahan merged commit fbf3336 into staging May 30, 2024
5 checks passed
@ddahan ddahan deleted the nestedroutes branch May 30, 2024 08:27
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.

2 participants