Skip to content
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.

Sign up for GitHub

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

Jump to bottom

enhancement/issue 71 acquire svgs for all assets #86

Merged
merged 7 commits into from
Sep 20, 2024
Merged

enhancement/issue 71 acquire svgs for all assets #86

merged 7 commits into from
Sep 20, 2024

Conversation

Auhseh
Copy link
Contributor

@Auhseh Auhseh commented Sep 3, 2024 •
edited by thescientist13
Loading

Related Issue

resolves #71

Summary of Changes

  1. Added all the required svgs
  2. Swapped out images for SVGs for Run Anywhere content

TODOs

  1. Update Netlify, Vercel, and NodeJS paths in Run Anywhere section / component
  2. Remove old images
  3. Re-run Lighthouse report
  4. Update WCC usage in the Build with Friends section / component (can come in a follow up PR)

@Auhseh Auhseh linked an issue Sep 3, 2024 that may be closed by this pull request
use SVGs for all logos and icons #71
Closed
@netlify Netlify
Copy link

netlify bot commented Sep 3, 2024 •
edited
Loading

✅ Deploy Preview for super-tapioca-5987ce ready!

Name Link
🔨 Latest commit a2f9ade
🔍 Latest deploy log https://app.netlify.com/sites/super-tapioca-5987ce/deploys/66ec7d718dbb430008fe2e65
😎 Deploy Preview https://deploy-preview-86--super-tapioca-5987ce.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@thescientist13 thescientist13 added the enhancement New feature or request label Sep 5, 2024
thescientist13
thescientist13 requested changes Sep 5, 2024
View reviewed changes
Copy link
Member

@thescientist13 thescientist13 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Auhseh , thanks for this.

Have some feedback for this one if you are able to (otherwise i don't mind making these changes)

  1. Let's follow the file naming convention of the project just to keep things consistent (this should get the build passing):
    • Nodejs.svg -> nodejs.svg
    • Vercel.svg -> vercel.svg
    • (I would use git mv for these since I think git has some issues with case sensitivity when renaming)
  2. I appreciate the effort, but let's remove the WCC icon for now. I've already reached out to the original designer to get a proper SVG export so I can take care of that one (since the one here looks a little jagged / choppy). 👍
  3. Can we update the image references on the home page to swap out the PNGs with the new SVGs. (all but the WCC icon are used in the Run Anywhere section)

Thanks again!

Also, this branch seemed pretty far behind main so I did a rebase to get the latest home page code where these icons are being used just to better validate the changes overall when we make the swap, so just keep that in mind when pushing / pulling with git.

@thescientist13 thescientist13 marked this pull request as draft September 5, 2024 19:59
@thescientist13 thescientist13 added the content Non Greenwood specific content like home page copy text, blog posts, etc label Sep 9, 2024
@thescientist13 thescientist13 changed the title 71 acquire svgs for all assets enhancement/issue 71 acquire svgs for all assets Sep 9, 2024
@thescientist13 thescientist13 marked this pull request as ready for review September 9, 2024 16:30
thescientist13
thescientist13 approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a chance to get some of the feedback in and things are looking good now! Should be getting the updated WCC logo soon, but if I don't get it in a couple of days, we can totally merge this and I can submit a follow PR just for WCC.

Thanks again @Auhseh 🤝

thescientist13
thescientist13 reviewed Sep 16, 2024
View reviewed changes
Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In touch with the designer now and should have a new WCC logo SVG soon, but if it's not available by end of the week, I'll merge this as is and follow up with the WCC logo on its own.

thescientist13
thescientist13 approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@thescientist13 thescientist13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, the updated WCC logo has landed! Will give this a couple days for any final reviews and otherwise will merge by end of the week.

Thanks again @Auhseh !

@Auhseh
Copy link
Contributor Author

Auhseh commented Sep 17, 2024

Looks good 🚀

@thescientist13 thescientist13 removed the content Non Greenwood specific content like home page copy text, blog posts, etc label Sep 20, 2024
@thescientist13 thescientist13 merged commit 9158ed5 into main Sep 20, 2024
5 checks passed
@thescientist13 thescientist13 deleted the 71-acquire-svgs-for-all-assets branch September 20, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thescientist13 thescientist13 thescientist13 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

use SVGs for all logos and icons
2 participants
@Auhseh @thescientist13