DerLindenhonig
commented
2 years ago Kiitos kattavasta ja hyödyllisestä palautteesta! Otin ehdotukset huomioon ja yritän toteuttaa niitä projekteissani.
Ystävällisin terveisin, Anna
From: Kalle Ilves @.***> Sent: Wednesday, January 19, 2022 9:20:24 AM To: DerLindenhonig/study-app-2021 Cc: Anna Raevskaia; Mention Subject: [DerLindenhonig/study-app-2021] Full Stack projektin katselmointi (Issue #1)
Full Stack projektin katselmointi
Terve @DerLindenhonighttps://github.com/DerLindenhonig! Tässä lyhyt katselmointi Full Stack kurssin projektistasi. Katselmoinnissa esitetyt kommentit ovat parannusehdotuksia, joita voit halutessasi ottaa huomioon tässä tai tulevissa projekteissasi. Mitään muutoksia tähän projektiin ei siis ole pakko tehdä suoritusmerkintää varten.
Käytettävyys Mitä tein?
- Rekisteröin uuden käyttäjän ja kirjauduin sisään
- Lisäsin sanalistan
- Selasin omia sanalistojani
- Selasin käyttäjiä
- Muokkasin sanalistaa ja lisäsin siihen kortteja
- Pelasin peliä
- Kirjauduin ulos
Kokemus
- Sovelluksella selkeä käyttöliittymä, hyvää työtä! 🥇
- Geneerisen "React App"-titlenhttps://developer.mozilla.org/en-US/docs/Web/HTML/Element/title ja faviconin voisi muuttaa kuvaavammaksi
- Turkoosi alapalkki peittää ikävästi käyttöliittymän elementtejä. Jos body-elementtin alaosaan laittaa palkin korkeuden verran paddingia, niin ongelma ratkeaa: padding-bottom: 80px
- Rekisteröitymislomakkeessa voisi olla virheilmoituksia virhetilanteissa (esim. jos jokin kenttä jää täyttämättä)
- Rekisteröitymisen jälkeen ei tapahdu mitään
- Salasanakentissä kannattaa käyttää -elementtiä
- Kirjautumisen jälkeen aukeaa tyhjä sivu ja konsolista löytyy virhe TypeError: Cannot read properties of null (reading 'level')
- Myös sanalistan lisäyslomakkeessa voisi olla virheilmoituksia
- "My word lists"-lista ei päivity lisäämisen jälkeen ennen kuin sivun lataa uudelleen
- Ainoastaan sovelluksen juuripolku (https://blooming-spire-37746.herokuapp.com/) aukeaa suoraan selaimessa, mutta esimerkiksi https://blooming-spire-37746.herokuapp.com/registration avaaminen palauttaa virheviestin unknown endpoint. Syynä on luultavasti, se että frontendin index.html-tiedosto tarjoillaan backendin puolella vaan /-polusta. Esim. tästähttps://gist.github.com/ryanoglesby08/1e1f49d87ae8ab2cabf45623fc36a7fe Gististä voi saada vinkkejä ongelman korjaamiseen
Koodi
-
Frontend-puolen koodi on selkeästi organisoitua, hyvä!
-
Tämähttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/choco tiedosto on varmaan turha?
-
API-kutsut on abstrahoitu hyvin uudellenkäytettäviksi palveluiksi services-hakemistossa
-
API-kutsuihin liittyvät operaatiot tuottavat jonkin verran toisteellista boilerplate-koodiahttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/App.js#L66. Esimerkiksi React Queryhttps://react-query.tanstack.com/ kirjaston avulla API-kutsut voi helposti abstrahoida yksinkertaisiksi hookeiksi, kuten useUsers
-
Ovatko nämä blogejahttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/App.js#L56? Nimeäminen on hämmentävää
-
Mikä virka on refreshBlogshttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/App.js#L62-tilalla? Ehkä blogien hakemisen voisi toteuttaa näin:
const refetchBlogs = () => {
blogService.getAll().then(blogs =>
setBlogs(blogs))
}
useEffect(() => {
refetchBlogs()
// ...
}, [])
Nyt refetchBlogs-funktion voi antaa propsina komponenteille, joilla on tarve päivittää blogien tila
-
Älä määrittele komponentteja komponentin sisällähttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/components/Blog.js#L30
-
Nämähttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/components/Blogs.js#L55 filter-kutsut kannattaa yhdistää:
.filter(blog => (
blog.title?.toLowerCase().includes(filter.toLowerCase()) &&
blog.status === 'public' &&
blog.category === category
))
-
Täällähttps://github.com/DerLindenhonig/study-app-2021/blob/master/studyapp-frontend/src/components/Blogs.js#L50 myös komponentti määritelty komponentin sisällä
-
Styled-components kirjastoa on hyödynnetty järkevästi
-
Myös backend-puolen koodi on selkeästi organisoitua, hienoa!
-
Reittien käsittelijöiden ainaisista try { ... } catch (e) { next(e) }-lohkoista pääsee helposti eroon express-async-errorshttps://www.npmjs.com/package/express-async-errors-kirjaston avulla
-
Autentikaatiotokenin ekstraktoinnin kyselystä ja sen verifioinnin voisi siirtää omaan middlewareensa. Tällä hetkellä näihin operaatioihin liittyvää koodia on duplikoitu monessa reitin käsittelijässä
Kokonaisuus
Sanalista-sovelluksella on selkeä käyttöliittymä, mutta sen toiminallisuudessa on vielä jonkin verrän käyttökokemusta heikentäviä bugeja ja käytettävyysongelmia. Kaikkea toiminallisuus kuitenkin toimi jollain tasolla katselmoinnin puitteissa. Projektin koodi on kaikin puolin hyvin organisoitua, mutta koodin laadussa on vielä hieman parantamisen varaa. Tarvittavat refaktoroinnit ovat kuitenkin kohtalaisen pieniä. Hyvää työtä!
— Reply to this email directly, view it on GitHubhttps://github.com/DerLindenhonig/study-app-2021/issues/1, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS3YG4H4RXY6UTRXSRGYXT3UWZQ3RANCNFSM5MJEOHFA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>
Full Stack projektin katselmointi
Terve @DerLindenhonig! Tässä lyhyt katselmointi Full Stack kurssin projektistasi. Katselmoinnissa esitetyt kommentit ovat parannusehdotuksia, joita voit halutessasi ottaa huomioon tässä tai tulevissa projekteissasi. Mitään muutoksia tähän projektiin ei siis ole pakko tehdä suoritusmerkintää varten.
Käytettävyys
Mitä tein?
Kokemus
body-elementtin alaosaan laittaa palkin korkeuden verran paddingia, niin ongelma ratkeaa:padding-bottom: 80px<input type="password" />-elementtiäTypeError: Cannot read properties of null (reading 'level')unknown endpoint. Syynä on luultavasti, se että frontendin index.html-tiedosto tarjoillaan backendin puolella vaan/-polusta. Esim. tästä Gististä voi saada vinkkejä ongelman korjaamiseenKoodi
Frontend-puolen koodi on selkeästi organisoitua, hyvä!
Tämä tiedosto on varmaan turha?
API-kutsut on abstrahoitu hyvin uudellenkäytettäviksi palveluiksi services-hakemistossa
API-kutsuihin liittyvät operaatiot tuottavat jonkin verran toisteellista boilerplate-koodia. Esimerkiksi React Query kirjaston avulla API-kutsut voi helposti abstrahoida yksinkertaisiksi hookeiksi, kuten
useUsersOvatko nämä blogeja? Nimeäminen on hämmentävää
Mikä virka on refreshBlogs-tilalla? Ehkä blogien hakemisen voisi toteuttaa näin:
Nyt
refetchBlogs-funktion voi antaa propsina komponenteille, joilla on tarve päivittää blogien tilaÄlä määrittele komponentteja komponentin sisällä
Nämä filter-kutsut kannattaa yhdistää:
Täällä myös komponentti määritelty komponentin sisällä
Styled-components kirjastoa on hyödynnetty järkevästi
Myös backend-puolen koodi on selkeästi organisoitua, hienoa!
Reittien käsittelijöiden ainaisista
try { ... } catch (e) { next(e) }-lohkoista pääsee helposti eroon express-async-errors-kirjaston avullaAutentikaatiotokenin ekstraktoinnin kyselystä ja sen verifioinnin voisi siirtää omaan middlewareensa. Tällä hetkellä näihin operaatioihin liittyvää koodia on duplikoitu monessa reitin käsittelijässä
Kokonaisuus
Sanalista-sovelluksella on selkeä käyttöliittymä, mutta sen toiminallisuudessa on vielä jonkin verrän käyttökokemusta heikentäviä bugeja ja käytettävyysongelmia. Kaikkea toiminallisuus kuitenkin toimi jollain tasolla katselmoinnin puitteissa. Projektin koodi on kaikin puolin hyvin organisoitua, mutta koodin laadussa on vielä hieman parantamisen varaa. Tarvittavat refaktoroinnit ovat kuitenkin kohtalaisen pieniä. Hyvää työtä!