search

ButterCMS / vuejs-starter-buttercms

Drop-in proof-of-concept VueJs app, fully integrated with your ButterCMS account
https://vuejs-starter-buttercms.vercel.app
5 stars 9 forks source link

Final Code overview #37

Closed ViolanteCodes closed 2 years ago

ViolanteCodes commented 2 years ago

Hi @orlyohreally - can we get a final once over on this for code? @gerzitom is aware of the need to pull the unused assets, but everything else looked okay to me :)

Thanks!

orlyohreally commented 2 years ago

@ViolanteCodes I don't know if you've already talked about it, but why package-lock.json is not in the repo? It's not critical, but maybe we'd want it to be. The only issue I've found is unused assets that is already noted and an issue is created, but I don't see that some of the images were removed https://github.com/ButterCMS/vuejs-starter-buttercms/pull/38/files

ViolanteCodes commented 2 years ago

@gerzitom

Can we have the unused image assets pulled?

@orlyohreally I'll push up package-lock.json now

gerzitom commented 2 years ago

@ViolanteCodes @orlyohreally I have removed all unused images in #38

orlyohreally commented 2 years ago

@ViolanteCodes I checked the demo and it loaded for me slightly longer than before and it seems that this starter app does not have loader displayed when the content is being loaded. Compare with https://django-starter-buttercms.herokuapp.com/

gerzitom commented 2 years ago

@orlyohreally okey, thank you for noticing, so I will implement the loader.

gerzitom commented 2 years ago

@orlyohreally loaders added in #39

ViolanteCodes commented 2 years ago

Looks like this has been approved, going to close this issue. Nice work @gerzitom, and thank you :) we're so excited for this starter :)