Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

chore: Homogeneous codebase #386

Merged
merged 5 commits into from
Mar 14, 2022
Merged

chore: Homogeneous codebase #386

merged 5 commits into from
Mar 14, 2022

Conversation

tlgimenes
Copy link
Contributor

@tlgimenes tlgimenes commented Mar 14, 2022 •
edited
Loading

What's the purpose of this pull request?

Makes codebase homogeneous by:

  1. Moving IconSVG to ui folder.
  2. Moving footer svg icons from custom files to icons.svg file
  3. Using IconSVG in footer component
  4. Moving EmptyState component to ui folder.

How does it work?

This PR revolves around having one example on the codebase for both "where placing UI components" and "how to add icons to the store". We should have only one solution for both tasks. Currently, we have two ways of doing these tasks. This PR solves this problem.

As a side effect, this PR removed some code from the repo:
image

How to test it?

Everything should be the same

Checklist

  • CHANGELOG entry added

@netlify
Copy link

netlify bot commented Mar 14, 2022 •
edited
Loading

✔️ Deploy Preview for basestore ready!

🔨 Explore the source changes: beae99f

🔍 Inspect the deploy log: https://app.netlify.com/sites/basestore/deploys/622fb103d7bdac0008362480

😎 Browse the preview: https://deploy-preview-386--basestore.netlify.app

@vtex-sites
Copy link

vtex-sites bot commented Mar 14, 2022 •
edited
Loading

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-386--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit d5e68bf

@gatsby-cloud
Copy link

gatsby-cloud bot commented Mar 14, 2022 •
edited
Loading

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 4m

Performance

Lighthouse report

Metric Score
Performance 💚 94
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@tlgimenes tlgimenes marked this pull request as ready for review March 14, 2022 19:57
filipewl
filipewl approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@filipewl filipewl 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.

Loved the shorter name and that more icons were added. Found no visual regression.

I ask only to also take a look at Managing SVG Icons section on the README to update some references (e.g. component name, static file location).

src/components/common/Footer/footer.scss Outdated Show resolved Hide resolved
@igorbrasileiro
Copy link
Contributor

@tlgimenes i know this isn't related to this PR, but, can you add 18px width to SignInLink icon? This is causing a Layout Shift

tlgimenes and others added 2 commits March 14, 2022 18:15
@tlgimenes @filipewl
Update src/components/common/Footer/footer.scss
35967dc 
Co-authored-by: Filipe W. Lima <fwl.ufpe@gmail.com>
@tlgimenes
fix README
beae99f
@tlgimenes
Copy link
Contributor Author

@tlgimenes i know this isn't related to this PR, but, can you add 18px width to SignInLink icon? This is causing a Layout Shift

AFAIK, it's already with 18px

@tlgimenes tlgimenes merged commit d5e68bf into master Mar 14, 2022
@tlgimenes tlgimenes deleted the chore/unify-svg branch March 14, 2022 21:30
@saranicoly saranicoly mentioned this pull request Mar 17, 2022
Merged
1 task
@hellofanny hellofanny mentioned this pull request Mar 28, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@filipewl filipewl filipewl approved these changes

@igorbrasileiro igorbrasileiro igorbrasileiro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tlgimenes @igorbrasileiro @filipewl