Feat/rewrite test #95
Conversation
.env.development
Outdated
| @@ -0,0 +1,8 @@ | |||
| module.export = { | |||
There was a problem hiding this comment.
Den her og den tilsvarende for produktionen bør vist bo på -config repoet i stedet.
There was a problem hiding this comment.
Enig. Og token bør teknisk set roteres, nu det er postet til Github.
(Selv om jeg formoder den ender på et public site under alle omstændigheder.)
There was a problem hiding this comment.
Det kunne nok være en ide at tilføje .env.* til .gitignore, så kommer de ikke med ved en fejl i fremtiden
There was a problem hiding this comment.
Får jeg gjort :) Tak for at gøre mig opmærksom på det.
There was a problem hiding this comment.
Får også kigget på at få sidens token roteret.
There was a problem hiding this comment.
Synes den overordnede struktur ser fornuftig ud.
Det er nemt at gennemskue de enkelte elementers sammenhæng, så det skulle give gode muligheder for videreudvikling i fremtiden.
Jeg ville vælge at gøre KoorMap og KoorMenu lidt mere uafhængige af hinanden, som nævnt i kommentarerne.
Jeg ville også vælge at lave løsningen med vanilla web components for at mindske antallet af dependencies, der skal holdes opdateret i fremtiden - specielt når der er tale om en relativt ukompliceret applikation som den her.
Det er dog acceptabelt at bygge løsningen med VueJS, fordi vi allerede har kompetencer inhouse, så det burde være muligt at løfte vedligehold i den nærmeste fremtid.
Store-modulet har en overskuelig størrelse (lige nu). Tænker ikke, der er særlig grund til at splitte det op i flere sub-moduler. Det kan gøres i fremtiden, hvis det vokser sig betydeligt større.
.gitignore
Outdated
| @@ -0,0 +1,3 @@ | |||
| .vscode/ | |||
| node_modules | |||
| public/ | |||
There was a problem hiding this comment.
Hvis public er i .gitignore, hvor bor billeder o.lign. så henne?
There was a problem hiding this comment.
På hkpn bruger jeg vite-static-copy til at få de billeder og ikoner over, som jeg har brug for. Havde tænkt mig at gøre noget lignende her.
package.json
Outdated
| "proj4": "^2.14.0", | ||
| "vue": "^3.5.12", | ||
| "vue-router": "^4.4.5", | ||
| "vuex": "^4.1.0" |
There was a problem hiding this comment.
Er vuex stadig anvendelig med Vue3? Jeg havde indtryk af, at den "officielle" state manager for Vue3 var Pinia (https://pinia.vuejs.org/ )
There was a problem hiding this comment.
Jeg får taget et kig på det :) Tak for info
There was a problem hiding this comment.
Har fået migreret til Pinia
| class="olmap" | ||
| > | ||
| </div> | ||
| <Menu class="koor-menu" :cover-area="coverArea"/> |
There was a problem hiding this comment.
Jeg ville nok lade KoorMenu og KoorMap stå side om side (i en ny over-komponent, så vue-router kan håndtere det) i stedet for at lave en hierarkisk afhængighed her.
coverArea kan de kan synkronisere via Store.
There was a problem hiding this comment.
God Ide :) Er gjort
| })], | ||
| resolve: { | ||
| alias: { | ||
| '@': path.resolve(__dirname, './src') |
There was a problem hiding this comment.
Hvis du vil lave unit tests down the road, er det lidt besværligt med aliased imports.
Så overvej nøje, om det der vindes i developer experience nu og her står mål med tabet af DX i test-øjemed.
There was a problem hiding this comment.
Giver god mening. Fjerner det for nu.
Beskrivelse
Koordinattransformationen har flere problemer, som gør repoet svært at vedligeholde. Derfor er et rewrite blevet igangsat i samarbejde med @kbevers.
Rewritet er bygget fra bunden af, så komponenter, imports og setup er ændret. Dette pull request markerer overgangen fra et lokalt projekt til noget, der kan merges ind i
main. Jeg har derfor bedt @iamfrank om at gennemgå branchen og give feedback på opbygningen.Opbygning af Applikationen
Projektet er en Vue-applikation, der benytter en router og en store.
Storen giver adgang til getters og setters for sit state, som komponenterne bruger via
store.gettersog opdaterer viastore.dispatch.Siden indeholder tre views:
/Denmark/Greenland/About/Denmark og /Greenland indeholder komponenten
KoorMap.vue, som inkludererKoorMenu.vue.KoorMap.vue
Ansvarlig for at opdatere
CoordinatesFromi storen ved klik på kortet, samt for at opdatere markørens placering på kortet, når der sker ændringer iCoordinatesFrom.KoorMenu.vue
En container for
InputKoorogOutputKoor.CRSFromog derfra opdateresCoordinatesFrom.CRSToog derfra opdateresCoordinatesTo.InputKoor og OutputKoor indeholder begge komponenterne:
CoordinatesFromog præsenterer dem i det ønskede format. Ved ændringer i visningskoordinaten opdateresCoordinatesFrom.CoordinatesToog præsenterer dem i det ønskede format.Afslutning
Dette PR er til for at have en diskussion omkring opbygningen af applikationen, med fokus på følgende spørgsmål:
1: Skal storen opdeles i modules for at skabe større klarhed over dens brug?
2: Er Komponenternes ansvarsområder tydelige og ses dette af koden ?