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.

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

Home page styling #31

Merged
merged 8 commits into from Feb 2, 2022
Merged

Home page styling #31

merged 8 commits into from Feb 2, 2022

Conversation

JohnFTitor
Copy link
Collaborator

Hi there πŸ‘‹πŸ»

Thanks for taking the time to review the project. Here are some things important to note about it:

Sass

The project was built using Sass. The only difference was that we needed to add an additional loader to webpack, but the result is overall the same (A CSS output). Additionally, we used MiniCssExtractPlugin to generate an external CSS file in the dist folder instead of the styles in the head of the document. It has the same dynamic response, it's just that the output looks more organized.

Now into the project...

Changes implemented in "home-page-styling" branch

  • βœ… Add Styles to the layout.
  • βœ… Add media query for small devices.
  • βœ… Add Preconnect link to HTML to speed up page loading.

Final notes

Let me know if anything needs to be changed per good practices, readability, performance or if a feature implemented may break your current work. Again, thanks for the review!

Copy link
Owner

@carloshs1994 carloshs1994 left a comment

Choose a reason for hiding this comment

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

Hi @JohnFTitor

Your project is complete! There is nothing else to say other than... it's time to merge it :shipit:
Congratulations! πŸŽ‰ πŸ‘ πŸ‘ πŸ’―

To highlight 🍾

  • All linters are passing βœ”οΈ
  • You are following GitHub flow and gitflow βœ”οΈ
  • Good commit history βœ”οΈ
  • Your design is looking good and well structured πŸ‘ βœ”οΈ

Cheers and Happy coding!πŸ‘πŸ‘πŸ‘

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.


_As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

@JohnFTitor JohnFTitor merged commit 7ffcf1d into development Feb 2, 2022
@JohnFTitor JohnFTitor deleted the home-page-styling branch February 2, 2022 22:38
@JohnFTitor JohnFTitor self-assigned this Feb 17, 2022
@JohnFTitor JohnFTitor added the enhancement New feature or request label Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants