From 943c1207025f1a06d8a5ba145effe21b11b33b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Stenstr=C3=B8m?= Date: Tue, 1 Jul 2025 21:03:28 +0200 Subject: [PATCH 1/2] Add initial code feedback document --- code-feedback.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 code-feedback.md diff --git a/code-feedback.md b/code-feedback.md new file mode 100644 index 0000000..f854ec1 --- /dev/null +++ b/code-feedback.md @@ -0,0 +1,19 @@ +# Code feedback + +Så fik jeg mig endelig taget sammen til at kigge på din kode. + +Beklager det tog så lang tid. + +## Typescript + +Det først der springer mig i øjnene er, at der i repo'et lader til at være et mix af JS og TS. Der er heller ikke nogen tsconfig fil. + +Man kan sagtens skrive god kode i ren JS - men hvis andre skal læse det og arbejde med det er det nemmere at bruge TS. + +Personligt synes jeg også det er nemmere at skrive TS, pga typerne. + +## App.tsx + +I [`App.tsx`](./src/App.tsx) er der mange `useState` til at holde styr på en meget simpel form. Når det er en enkelt form med begrænsede inputs er det OK. Men det ville nok være nemmere (også for dig selv) hvis du brugte et form framework a la [`react-hook-form`](https://react-hook-form.com) eller [`TanStack +Form`](https://tanstack.com/form/latest). Det smukke ved den slags løsninger er at du kan nøjes med få linjer kode, som gør det samme som 6 - 10 `useState`s - og de giver også mulighed for validering og fejlhåndtering (for eksempel med [zod](https://zod.dev)). + From cb631f78a3a1c74b87c4a22c5f6fbffa38f49e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Stenstr=C3=B8m?= Date: Tue, 1 Jul 2025 21:39:11 +0200 Subject: [PATCH 2/2] Revise code feedback to enhance clarity and structure, emphasizing strengths and areas for improvement in TypeScript usage and code organization. --- code-feedback.md | 69 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/code-feedback.md b/code-feedback.md index f854ec1..1928a2b 100644 --- a/code-feedback.md +++ b/code-feedback.md @@ -4,16 +4,71 @@ Så fik jeg mig endelig taget sammen til at kigge på din kode. Beklager det tog så lang tid. -## Typescript +## Styrker -Det først der springer mig i øjnene er, at der i repo'et lader til at være et mix af JS og TS. Der er heller ikke nogen tsconfig fil. +Lad os starte med det positive -Man kan sagtens skrive god kode i ren JS - men hvis andre skal læse det og arbejde med det er det nemmere at bruge TS. +1. God struktur og organisering. Der er klar `separation of concerns` i opdelingen af filer. -Personligt synes jeg også det er nemmere at skrive TS, pga typerne. +2. Moderne tooling med vite, eslint v9, og React v19 -## App.tsx +3. Konsistent navngivning -I [`App.tsx`](./src/App.tsx) er der mange `useState` til at holde styr på en meget simpel form. Når det er en enkelt form med begrænsede inputs er det OK. Men det ville nok være nemmere (også for dig selv) hvis du brugte et form framework a la [`react-hook-form`](https://react-hook-form.com) eller [`TanStack -Form`](https://tanstack.com/form/latest). Det smukke ved den slags løsninger er at du kan nøjes med få linjer kode, som gør det samme som 6 - 10 `useState`s - og de giver også mulighed for validering og fejlhåndtering (for eksempel med [zod](https://zod.dev)). +4. Component composition: Separation between presentation and logic +5. Konsistent brug af MUI + +6. Loading states og håndtering af fejl fra API - Nice UX + +7. Dark theme - Altid nice + + +## Svagheder / Plads til forbedring + +1. Inkonsistente typescript-typer + +```typescript +// FieldCustomFuelPrice.tsx - ingen typescript props +export default function FieldCustomFuelPrice({ + formData, + onInputChange, + validationErrors, +}) { +``` + +miks af typer. Skaber forvirring. +```typescript +TPrice = string | number +``` + +2. Code duplication. + - TextField er det samme pattern over flere components + - Validering: Formvalidering kunne laves til en custom hook (eller brug et dedikeret form tool til det - f.eks. [TanStack Form](https://tanstack.com/form/latest) eller [React Hook Form](https://react-hook-form.com)) + - Input field: Numerisk input konfiguration bliver gentaget + +3. `useEffect` mangler en dependency i [`App.tsx`](./src/App.tsx), hvilket potentielt kan føre til infinite re-renders +```typescript +useEffect(() => { + // ...logic... +}, [selectedFuelType, fuelData]); // mangler formData dependency +``` + +4. Configuration + - Eslint er kun konfigureret til `.js` og `.jsx` filer - mangler `.ts` og `.tsx` + + +## Yderligere forbedringer +1. Overvej accessibility. + - tilføj ARIA labels (hvis ikke MUI allerede gør) + - Keyboard navigation + - Flyt Focus til fejlramte inputs + +2. Brug [error boundaries](https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary) + +3. Skriv bedre fejlbeskeder + +4. Overvej at lave et monorepo hvor både frontend og backend koden lever. + +## Afsluttende bemærkninger + +Din kode er god og letlæselig. Der er taget højde for UX. Der hvor der er brug for forbedring er konsistens i brugen af TypeScript. \ No newline at end of file