Design comparison
Community feedback
- @christopher-adolphePosted over 2 years ago
Hi @kowall1013
Nice attempt to do this challenge with React. You did a good job to split the page into components. 👍 I have noticed a few things that you might want to check to improve your solution.
HTML:
- It would be better to wrap the website's logo with an anchor
<a href="/"><img src="./assets/shared/logo.svg" alt="Officelite"></a>
instead of<h1><img src="./assets/shared/logo.svg" alt=""></h1>
and use the<h1>
tag with the title in the hero<section>
. This is better semantic and also better for SEO as it gives a clear indication of the content of the page. - You should not use more than one
<main>
tag per page and it should not be a descendent of a<section>
tag. I would suggest you to refactor the HTML of the home and registration pages by moving the hero<section>
inside the<main>
and replace the<main>
inside your hero with a<div
> tag. It should look like this:
<header> <p>Header content here</p> </header> <main> <section> <p>Hero content here</p> </section> <section> <p>Pricing content here</p> </section> </main> <footer> <p>Footer content here</p> </footer>
- You could apply the styles for your buttons directly to the links that look like a button instead of wrapping the
<button>
element with an anchor tag.
Do this: 👍
<a href="/registration" class="btn">Get Started</a>
Not this: 👎
<a href="/registration"> <button class="btn">Get Started</button> </a>
- There seem to be quite an issue of spacing in the desktop version. The two columns layout are too far apart. The vertical spacing between the hero section and the pricing section also needs to be adjusted. For the horizontal spacing you could add a wrapping
<div>
tag inside the<section>
tags to which you set amax-width
. - It would be better to use the
bg-header-pattern.svg
as background image rather than an image because does not add any value to the content of the page. Moreover, it is overlapping on the content because it is styled with absolute positioning. You could add it as a background image in the hero<section>
and use thebackground-position
property to align as per the design. - The background color of the "Try for free" links in the pricing
<section
> are different from the design. - At the moment, the elements of the footer are vertically aligned on desktop while they should be horizontally aligned.
The class names on the elements of the page look weird. Is it because you are using styled components ? 🤔
I hope this helps. You are not far from a complete solution.
Keep it up.
0@kowall1013Posted over 2 years ago@christopher-adolphe
Thank you so much for feedback, I appreciate so much!
Yes, I use styled components for styling, this is why classNames are weird :)
0@christopher-adolphePosted over 2 years ago@kowall1013 I'm happy to help and I hope you'll be able to apply these suggestions to your solution as well as future challenges.
Best regards.
0@christopher-adolphePosted over 2 years ago@kowall1013 I'm happy to help and I hope you'll be able to apply these suggestions to your solution as well as future challenges.
Best regards.
0 - It would be better to wrap the website's logo with an anchor
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord