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

Info section #5

Merged
merged 9 commits into from
Oct 4, 2021
Merged

Info section #5

merged 9 commits into from
Oct 4, 2021

Conversation

demircaner
Copy link
Owner

  • Add tests for the info section and rename the Hero.js to HomePage.js
  • Modify the Footer test for profy.dev since there are two links with the same name now
  • Fix the indentation issue with the list in How it works section when resizing

@mentor-tara
Copy link
Contributor

Great job 🚀 I'm a bit busy at the moment but I'll leave a review soon.

In the meantime, consider starting with the next task already to practice the professional Git workflow. If you need a refresher, feel free to follow this section in the course again.

Copy link
Contributor

@mentor-tara mentor-tara left a comment

Choose a reason for hiding this comment

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

Before I approve this PR I'd like to share a few tips with you.

Please go through the list below and review your own code first. Imagine you're the reviewer or a person who didn't see the code before. Ask yourself if everything is clearly written and understandable. Are components and variables properly named?

Reviewing your own code with this mindset is an important practice that will help you learn a lot and write high-quality code.

To make it a bit easier for you here are the tips I promised. Some have a link 🔗 attached where you can find more information. I'd recommend reading the bold tips in any case. Feel free to check the boxes when you think your code fits.

General:

  • Use the article tag instead of section 🔗

Styles:

  • Refrain from styling children with CSS selectors 🔗
  • Extract shared CSS values 🔗
  • Use global styles when appropriate (e.g. for font-size of headings, color of links...) 🔗

Tests:

  • Use within to test multiple occurences of elements (e.g. the profy.dev link) 🔗

Once you're done with reviewing your own code feel free to check the example implementation. But remember: It's an important exercise to review your code first on your own.

When you think this PR is ready to be merged just let me know by requesting another review. I'll approve right away.

@demircaner demircaner requested a review from mentor-tara October 4, 2021 22:22
Copy link
Contributor

@mentor-tara mentor-tara left a comment

Choose a reason for hiding this comment

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

Great work! 🎉🎉🎉 You can merge the PR now.

Note: This is the last task included in the free membership.

It's up to you of course, but I'd be super happy to finish this project with you.

This is what you'll get when you upgrade to a premium membership:

  • Get access to the tasks and designs for the search page.
  • Build a dynamic form to fetch data from an API.
  • Transform the data as preparation for the UI.
  • Build the interactive heatmap.
  • Write automated tests for dynamic components.
  • Get advice on how to prepare this project for your job hunt.

I hope you're ready to take the leap. This is where it gets real.

Go Premium Now

See you soon 😄

Note: If you're already a premium member, you can continue right away, of course.

@demircaner demircaner merged commit 651e8d5 into main Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants