Closed flukeout closed 8 years ago
I'll create a temporary heroku app for this.
Thank you!
The logos look kinda funky for mobile:
We should use px
as little as possible for responsive layouts. rem
is what should be used instead.
em
is ok if you truly want something expressed based relative to a font size (eg: typography).
classroom-bg.png
and header-bg.png
should be JPEGs. Can you add opacity via CSS?
All partner logos should be SVG or PNG.
@flukeout This is looking great. Just some various fixes & tweaks! 👍
@gvn curious about rationale for...
Otherwise, all linting is complete! Thanks for the thorough review.
@gideonthomas removed procfile
@flukeout ok I'll merge it in. Thank you!
Cool thanks!
All partner logos should be SVG or PNG.
Any low-color image should be SVG or PNG because JPEG is for photos. SVG and PNG compress low color imagery way better, so file size & quality are the reasons.
classroom-bg.png and header-bg.png should be JPEGs
They are very large as PNGs (over 200k each). As JPEGs they can be a fraction of that size.
Styled the page to match the designs in the redpen and added in the missing content.
In addition to a code review, can we move this into some kind of staging environment so that we can start a QA process for content and design.
cc @gideonthomas