-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
56 image size optimization #89
Conversation
…r smaller screens to hero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. The file size reductions may not be huge but it makes it more professional. That at standardizing the names.
My only real comments at the moment are kinda nitpicky ones about consistency in the images rendering.
The new PASS logo is pretty large and much closer to the buttons than the others, in both mobile and desktop versions. In fact, they all seem a bit different. The CODE PDX one is a bit on the big side as well.
I know this is largely because the images themselves are not consistent. But if we could make them more uniform I think that'd look better. Not worth sweating too much over, though. We could come back to it later with higher definition versions, if we wanted.
Also, perhaps this is a good time to organize the images more? There's already a folder just for volunteer images.
@andycwilliams The images you mentioned are both "new" I can easily adjust this, but would prefer to do it in a separate PR. I was going to propose a slight layout change for the project logos on the project page as well, so I could submit those together. We could certainly add more folders to |
Images are always finicky. The struggle is eternal. I'm down if you want to create a separate issue. I see in the original issue that |
In response to @andycwilliams's review, I completed the following: I reorganized assets into a few folders. The only change from my previous comment was that instead of making a For the size and layout changes, I took @andycwilliams's comment and screenshots above and opened issue #90. I'll work on getting the project section to display nicely again and get a PR up for it. I also corrected a bug (that I created 🥳 ) that was causing the Ceti logo not to display on mobile. It should display correctly now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💰 big savings indeed. nice changes
This PR:
Resolves #56 by:
.webp
except for where.svg
is more appropriate.svg
s wherever available.png
filescode-pdx-volunteer-andy.webp
ortechnology-association-of-oregon-logo-366x105.webp
Component changes:
For
Hero.jsx
mobileHeroImage
mobileHeroImage
prop assignment tohome.jsx
,volunteer.jsx
andprojects.jsx
Hero
to serve the appropriately sized background hero image using ourxs
andmd
theme breakpointsFor
SecondaryPartners.jsx
mobilePartnerLogo
assignment tosecondaryPartnerList.js
smallScreenComponent
to usemobilePartnerLogo
(which triggers for screens <md
)For
PrimaryPartner.jsx
Box
component, usingsx-content
to switch to the correctly sized TAO logo based on ourxs
andmd
theme breakpointsFor
Navbar.jsx
andFooter.jsx
.svg
Screenshots (if applicable):
💰 Big savings!
Note: there should be no real change in the actual rendering of the site other than it being a little faster!
Future Steps/PRs Needed to Finish This Work (optional):
None