lr222gw
commented
9 years ago Tack för en bra Peer Review! :D
Den 19 april 2015 15:03 skrev hg222dt notifications@github.com:
Här kommer min peer review på Lowes klient-app! Ska tillägga att jag och Lowe diskuterat appen en del utanför Github, för att jag hade lite problem med att få igång den.
På det stora är det en intressant app, och du hade en kul tanke (som inte jag själv ens funderat över att implementera) med att välja position genom att användaren skulle ange stadsnamn (som jag förstod det!) Fick inte just den funktionen att funka helt dock.
Nedan kommer lite punkter med en del positiv feedback samt förbättringar!
1.
Instruktioner Instruktionerna var ganska tydliga men det fanns en del viktiga detaljer som fattades. Jag var tvungen att köra bundle install för ruby-projektet, vilket inte stod i instruktionerna. (Men det hör kanske inte till denna peer reviewn) Jag var även tvungen att installera bower globalt innan jag kunde köra bower install, och då köra projektet. (tog mig en del tid att förstå.) Sen hade jag till sist problem med att kunna autentisera mig som användare när jag fått igång appen. Det visade sig bero på att jag inte inkluderat “apikey=“ i textsträngen innan själva api-nyckeln i konstanten. Jag kan tycka att det bara borde behövas att lägga in själva nyckeln, utan “apikey=“, Skulle underlätta för utvecklare och missförstånd skulle inte uppstå så ofta. 2.
Datum för att skapa event Jag vet inte riktigt exakt hur din app ska användas, eller vad “eventen” är för något, men det skulle vara kanske sjysst om datumet var förifyllt med dagens datum eller något. Bara en idé :) 3.
Lösenordet kvar när jag loggat ut När jag loggar ut står lösenordet kvar i lösenordsfältet. Kanske inte är helt optimalt om användare tror att de loggat ut på en publik dator och glömmer radera sitt lösenord. 4.
Många bra feedbacks till användaren! :) Jag gillar generellt att du ofta och på ett bra sätt notifierar användaren om vad som händer, med dina flash-meddelanden i överkant. Med fina färger och allt. Men känner samtidigt att jag som användare bara behöver bli notifierad om exempelvis uppkopplingen mot api:et ut ifall att uppkopplingen misslyckas. Nu notifieras även användaren om att uppkoppling faktiskt lyckats, vilket ur interaktionsdesign-perspektiv är lite överflödig information. PS. Upptäckte att om man väljer att logga ut efter att ha fått ett rödfärgat flash-meddelande från klienten, så hänger detta meddelande med till efter utloggningen, istället för att blir grönt med texten “Utloggningen lyckades!”. 5.
Namngivning och kommentarer I din kod har du två vyer som heter view1 och view2. Vet inte riktigt vad skillnaden är mellan dem. Sen kanske det hade varit tydligare om det fanns en del förklaring för varje controller, vad den gör eller är till för. 6.
Struktur Jag hade nog valt att lägga de olika controllerna i olika filer. Kanske är en smaksak men jag tycker det blir lite mer överskådligt då. Vill man minska antalet requests sen, kan man ju använda sig av någon slags minifiering för script-filerna som kombinerar alla filerna till en. Finns bra grunt-moduler som gör detta! :) (Grunt <3 ;) 7.
myGlobals Jag gillar att du samlat en del värden i din “myGlobals”-variabel. Tycker att det tydliggör. 8.
Måste användaren ge ifrån sig sin position? När jag testar appen så måste jag tillåta den att se min position för att kartan ska synas. Det känns kanske lite snålt, och många användare vill nog inte det. Men jag vet inte om det verkligen är tänkt att vara så i din app? 9.
Att skapa event När jag försöker skapa ett event, så har kan jag inte välja position på kartan. Det dyker inte upp någon markör (vilket jag antar är vad som ska hända?). Jag kan dock ändå skapa ett event, utan att ha valt någon position. Men det uppkommer dock problem sen för mig när jag ska redigera detta event, och då får en flash: "Eventet är sparat med ogiltig position, vänligen ändra positionsnamnet (om det är ditt event)". Dock kan jag inte ändra positionen, eftersom att jag inte kan välja någon plats på kartan.
Jag har försökt förstå varför jag inte kan välja en position på kartan, men kan inte hitta det.
EDIT. Efter att ha pratat med dig (Lowe) om detta problem, så förstår jag att jag ska skriva in ett stadsnamn som appen själv ska leta upp koordinaterna på. Dock fungerar inte detta för mig heller :(
I övrigt tycker jag att du validerar datan bra när användaren skapar ett event!
Hoppas detta var behjälpligt :)
— Reply to this email directly or view it on GitHub https://github.com/lr222gw/lr222gw_Webbramwek/issues/13.
Här kommer min peer review på Lowes klient-app! Ska tillägga att jag och Lowe diskuterat appen en del utanför Github, för att jag hade lite problem med att få igång den.
På det stora är det en intressant app, och du hade en kul tanke (som inte jag själv ens funderat över att implementera) med att välja position genom att användaren skulle ange stadsnamn (som jag förstod det!) Fick inte just den funktionen att funka helt dock.
Nedan kommer lite punkter med en del positiv feedback samt förbättringar!
Jag har försökt förstå varför jag inte kan välja en position på kartan, men kan inte hitta det.
EDIT. Efter att ha pratat med dig (Lowe) om detta problem, så förstår jag att jag ska skriva in ett stadsnamn som appen själv ska leta upp koordinaterna på. Dock fungerar inte detta för mig heller :(
I övrigt tycker jag att du validerar datan bra när användaren skapar ett event!
Hoppas detta var behjälpligt :)