From 506b54a48feb76681302b751d35790beaa32f7c6 Mon Sep 17 00:00:00 2001 From: Anthony Pena Date: Fri, 29 Aug 2025 11:05:28 +0200 Subject: [PATCH] review --- README.md | 5 ++ back/mvnw | 0 front/package.json | 6 ++- front/src/app/app.component.ts | 27 ++++++----- front/src/app/app.routes.ts | 23 +++++---- .../components/footer/footer.component.html | 3 -- .../components/footer/footer.component.scss | 0 .../components/footer/footer.component.ts | 18 ++++--- .../testConnectionBackFront.service.ts | 20 ++++---- .../test-connection-back-front.component.html | 2 +- .../test-connection-back-front.component.ts | 48 ++++++++++--------- .../app/features/login/login.component.html | 16 +++++++ .../not-found-page.component.scss | 5 ++ 13 files changed, 110 insertions(+), 63 deletions(-) mode change 100644 => 100755 back/mvnw delete mode 100644 front/src/app/common/components/footer/footer.component.html delete mode 100644 front/src/app/common/components/footer/footer.component.scss diff --git a/README.md b/README.md index e8a95729..bb61715c 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,7 @@ # speaker-space Project to allow conference organizers and speakers to share informations about their talks + + +// TODO: est-ce que tu peux ajouter un peu de doc : à minima il faudrait une partie "How to develop?" avec les commandes à lancer, les configurations pour le mode dev, comment les trouver, etc. en fait tout ce qu'il faut quand tu débarques pour savoir quoi faire pour lancer l'app le plus rapidement possible en dev sans trop se prendre la tête + +// NOTE: j'ai vu que t'as mis du TailwindCSS. Je vois comment ça marche mais je maitrise pas TailwindCSS, donc je pense pas te faire de remarque sur les styles en tailwindCSS diff --git a/back/mvnw b/back/mvnw old mode 100644 new mode 100755 diff --git a/front/package.json b/front/package.json index 1d4432c0..f1b86eaa 100644 --- a/front/package.json +++ b/front/package.json @@ -38,5 +38,9 @@ "karma-jasmine": "~5.1.0", "karma-jasmine-html-reporter": "~2.1.0", "typescript": "~5.7.2" - } + }, + "// SUGGESTION": [ + "n'hésite pas à ajouter un linter (eslint: https://github.com/angular-eslint/angular-eslint) et un formatter (prettier : https://prettier.io/docs/integrating-with-linters)", + "// ça te permettra d'avoir partout un style de code uniforme (plus facile de relire les PRs)", + "// et des indications si t'as mauvaise pratique automatiquement remontées pendant que tu codes (méthodes/class/function dépréciés, problème de sécurité, etc.)"] } diff --git a/front/src/app/app.component.ts b/front/src/app/app.component.ts index d5a0c955..3e37159c 100644 --- a/front/src/app/app.component.ts +++ b/front/src/app/app.component.ts @@ -1,23 +1,28 @@ -import {Component} from '@angular/core'; -import {NavigationEnd, Router, RouterOutlet} from '@angular/router'; -import {NavbarComponent} from './common/components/navbar/navbar.component'; -import {FooterComponent} from './common/components/footer/footer.component'; +import { Component } from "@angular/core"; +import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; +import { NavbarComponent } from "./common/components/navbar/navbar.component"; +import { FooterComponent } from "./common/components/footer/footer.component"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; @Component({ - selector: 'app-root', - imports: [RouterOutlet,NavbarComponent, FooterComponent], - templateUrl: './app.component.html', + selector: "app-root", + imports: [RouterOutlet, NavbarComponent, FooterComponent], + templateUrl: "./app.component.html", standalone: true, - styleUrl: './app.component.scss' + styleUrl: "./app.component.scss", }) export class AppComponent { - title = 'frontend'; + title = "frontend"; showNavbar = true; constructor(private router: Router) { - this.router.events.subscribe(event => { + // TODO: chaque fois que tu as un .subscribe() il FAUT que tu penses "unsubscribe", sinon y'aura des fuites mémoires, et ton app va cramer la RAM de l'utilisateur + // Y'a plusieurs options : + // - unsubscribe explicite https://rxjs.dev/guide/subscription#subscription (et du coup il te faut un onDestroy() sur ton composant) + // - unsubscribe implicite via un takeUntilDestroy() https://v19.angular.dev/api/core/rxjs-interop/takeUntilDestroyed => dès que le composant est destroy, takeUntilDestroyed va automatiquement unsubscribe + this.router.events.pipe(takeUntilDestroyed()).subscribe((event) => { if (event instanceof NavigationEnd) { - const hiddenRoutes = ['/not-found', '/login']; + const hiddenRoutes = ["/not-found", "/login"]; this.showNavbar = !hiddenRoutes.includes(this.router.url); } }); diff --git a/front/src/app/app.routes.ts b/front/src/app/app.routes.ts index c382af75..7855d27a 100644 --- a/front/src/app/app.routes.ts +++ b/front/src/app/app.routes.ts @@ -1,13 +1,16 @@ -import { Routes } from '@angular/router'; -import {TestConnectionBackFrontComponent} from './common/test-connection-back-front/test-connection-back-front.component'; -import {NotFoundPageComponent} from './features/not-found-page/not-found-page.component'; -import {LoginComponent} from './features/login/login.component'; -import {LogoutHomePageComponent} from './features/logout-home-page/logout-home-page-component'; +import { Routes } from "@angular/router"; +import { TestConnectionBackFrontComponent } from "./common/test-connection-back-front/test-connection-back-front.component"; +import { NotFoundPageComponent } from "./features/not-found-page/not-found-page.component"; +import { LoginComponent } from "./features/login/login.component"; +import { LogoutHomePageComponent } from "./features/logout-home-page/logout-home-page-component"; +// SUGGESTION: n'hésite pas à lazy loader les différentes routes (https://v19.angular.dev/guide/routing/common-router-tasks#lazy-loading) +// c'est pas un gros effort, ça permet de gagner un peu en performance +// autre avantage (comme j'ai vu que t'as une PR pour la partie admin) tu peux éviter de charger les parties du code qui sont pour la section admin pour les utilisateurs export const routes: Routes = [ - { path:'', component: LogoutHomePageComponent}, - { path: 'system-info', component: TestConnectionBackFrontComponent }, - { path: 'login', component: LoginComponent }, - { path: 'not-found', component: NotFoundPageComponent }, - { path: '**', redirectTo: '/not-found' } + { path: "", component: LogoutHomePageComponent }, + { path: "system-info", component: TestConnectionBackFrontComponent }, + { path: "login", component: LoginComponent }, + { path: "not-found", component: NotFoundPageComponent }, + { path: "**", redirectTo: "/not-found" }, ]; diff --git a/front/src/app/common/components/footer/footer.component.html b/front/src/app/common/components/footer/footer.component.html deleted file mode 100644 index 16a85cd3..00000000 --- a/front/src/app/common/components/footer/footer.component.html +++ /dev/null @@ -1,3 +0,0 @@ - - © 2025 Speaker Space - diff --git a/front/src/app/common/components/footer/footer.component.scss b/front/src/app/common/components/footer/footer.component.scss deleted file mode 100644 index e69de29b..00000000 diff --git a/front/src/app/common/components/footer/footer.component.ts b/front/src/app/common/components/footer/footer.component.ts index 2e208c31..5caae24e 100644 --- a/front/src/app/common/components/footer/footer.component.ts +++ b/front/src/app/common/components/footer/footer.component.ts @@ -1,11 +1,15 @@ -import { Component } from '@angular/core'; +import { Component } from "@angular/core"; + +// SUGGESTION: quand t'as des composants pas trop gros comme ça (ça devrait être la norme de faire des petits composants, c'est plus facile à lire/maintenir/comprendre), +// tu peux faire un Single File Component (ça tend à devenir la norme dans Angular) @Component({ - selector: 'app-footer', + selector: "app-footer", imports: [], - templateUrl: './footer.component.html', - styleUrl: './footer.component.scss' + template: ` + + © 2025 Speaker Space + +`, }) -export class FooterComponent { - -} +export class FooterComponent {} diff --git a/front/src/app/common/test-connection-back-front/services/testConnectionBackFront.service.ts b/front/src/app/common/test-connection-back-front/services/testConnectionBackFront.service.ts index acf333ff..20724547 100644 --- a/front/src/app/common/test-connection-back-front/services/testConnectionBackFront.service.ts +++ b/front/src/app/common/test-connection-back-front/services/testConnectionBackFront.service.ts @@ -1,16 +1,20 @@ -import { Injectable } from '@angular/core'; -import {environment} from '../../../../environments/environment.development'; -import {HttpClient} from '@angular/common/http'; +import { inject, Injectable } from "@angular/core"; +import { environment } from "../../../../environments/environment.development"; +import { HttpClient } from "@angular/common/http"; + +interface FirestoreConnexionInfo { + message: string; +} @Injectable({ - providedIn: 'root' + providedIn: "root", }) export class TestConnectionBackFrontService { - private apiUrl = environment.apiUrl; - - constructor(private http: HttpClient) { } + // SUGGESTION: avec une syntaxe plus moderne testFirestore() { - return this.http.get<{message: string}>(`${this.apiUrl}/firestore/connection-info`); + return inject(HttpClient).get( + `${environment.apiUrl}/firestore/connection-info`, + ); } } diff --git a/front/src/app/common/test-connection-back-front/test-connection-back-front.component.html b/front/src/app/common/test-connection-back-front/test-connection-back-front.component.html index 42723f41..57be8b96 100644 --- a/front/src/app/common/test-connection-back-front/test-connection-back-front.component.html +++ b/front/src/app/common/test-connection-back-front/test-connection-back-front.component.html @@ -1 +1 @@ -

{{ firestoreMessage }}

+

{{ firestoreMessage() }}

diff --git a/front/src/app/common/test-connection-back-front/test-connection-back-front.component.ts b/front/src/app/common/test-connection-back-front/test-connection-back-front.component.ts index 558c9564..2d292752 100644 --- a/front/src/app/common/test-connection-back-front/test-connection-back-front.component.ts +++ b/front/src/app/common/test-connection-back-front/test-connection-back-front.component.ts @@ -1,28 +1,32 @@ -import {Component, OnInit} from '@angular/core'; -import {TestConnectionBackFrontService} from './services/testConnectionBackFront.service'; -import {CommonModule} from '@angular/common'; +import { Component, inject, OnInit } from "@angular/core"; +import { TestConnectionBackFrontService } from "./services/testConnectionBackFront.service"; +import { CommonModule } from "@angular/common"; +import { toSignal } from "@angular/core/rxjs-interop"; +import { catchError, map, of } from "rxjs"; @Component({ - selector: 'app-test-connection-back-front', + selector: "app-test-connection-back-front", standalone: true, imports: [CommonModule], - templateUrl: './test-connection-back-front.component.html', - styleUrl: './test-connection-back-front.component.scss' + templateUrl: "./test-connection-back-front.component.html", + styleUrl: "./test-connection-back-front.component.scss", }) -export class TestConnectionBackFrontComponent implements OnInit { - firestoreMessage: string = ''; - - constructor(private systemInfoService: TestConnectionBackFrontService) {} - - ngOnInit() { - this.systemInfoService.testFirestore().subscribe({ - next: (response: { message: string }) => { - this.firestoreMessage = response.message; - }, - error: (error) => { - console.error('Firestore connection error :', error); - this.firestoreMessage = 'Firestore connection error'; - } - }); - } +export class TestConnectionBackFrontComponent { + // TODO: évite d'utiliser ngOnInit(), depuis environ la v16 d'Angular, t'as plus besoin, mettre dans le constructeur ou directement affecter la property c'est mieux + // TODO: passe par le inject() pour ne plus avoir besoin du tout du constructor() dans la majorité des cas + // TODO: évite au maximum de faire des .subscribe() car tu crées des fuites mémoire + // au choix : + // - quand comme ici tu fais uniquement un GET qui n'a pas vraiment de raison d'être partagé entre plusieurs composant, tu peux passer par https://v19.angular.dev/guide/signals/resource# (c'est marqué expérimentale en v19 mais c'est pas grave elle finira stable sans que ça casse) + // - simplement passer par un AsyncPipe https://v19.angular.dev/api/common/AsyncPipe#examples (ça marche depuis la v4 de mémoire cette façon de faire) + // - transforme en Signal avec toSignal() https://v19.angular.dev/api/core/rxjs-interop/toSignal (j'ai transformé le code sur cette version pour te montrer) + // + firestoreMessage = toSignal( + inject(TestConnectionBackFrontService).testFirestore().pipe( + map((response) => response.message), // si la récupération se passe bien on affecte à this.firestoreMessage le message + catchError((error) => { // si elle se passe mal on met un message d'erreur + console.error("Firestore connection error :", error); + return of("Firestore connection error"); + }), + ), + ); } diff --git a/front/src/app/features/login/login.component.html b/front/src/app/features/login/login.component.html index 69ca157e..1b9f9bc6 100644 --- a/front/src/app/features/login/login.component.html +++ b/front/src/app/features/login/login.component.html @@ -15,6 +15,22 @@

+