@christopher-adolphe
Posted
Hi @francisldn 👋
You did a good job on completing this challenge with CSS grid and React. 👍 I am currently working on the same challenge and I'm only left with a few minor refinements before posting my solution. I wanted to review your solution so as to compare how we approached this challenge and if possible share some tips with you. 😉 Below are few things I have noticed and you might want to check in order to improve your solution.
- Rendering the main heading in 2 colours is surely the part that makes you scratch your head when you see the design file for the 1st time. 😆 But I can tell you that you can achieve that by using a single CSS property called
mix-blend-mode
. It can have different values depending on the result you want to get, in the case of this challenge, theexclusion
value does the job. Read more about it here. You can apply it to your.heading
class like this:
.heading {
mix-blend-mode: exclusion;
}
- Along the same vein, I think that it would be better to use an
<h1>
element for theModern Art Gallery
text instead of a<div>
and then in the main content section use<h2>
elements for the other heading. - The arrow for the
Our location
link is larger as compared to the design. Moreover, it is a link so this means that it should be an<a href="/location"></a>
element instead of a<div>
. Since you are using React and React Router, it makes sense to use the<Link to="/location">
component for this. To get the arrow to the correct size, you could refactor this part like this:
<Link className="btn" to="/location">
<span className="btn__text">Our Location</span>
<span className="btn__icon">
<img className="arrow" ref={locationImageRef} src={arrowRight} alt="" />
</span>
</Link>
You can then use the .btn__icon
class to style the part of the link with the golden background and the arrow. Also note that you can leave the alt
attribute of the <img/>
element blank since this is a decorative image.
- For better accessibility, please use a
<main>
element to wrap the main content of the page and a<footer>
element for the bottom part of the page. - On large viewports, the content is stretching horizontally thus making the images of the main content look larger as compared to the design. You could correct this by adding
<div className="container">
element to wrap the different sections of the page and applying amax-width
to it. In the design, the maximum width of the page is1110px
. - On the overall, the way you have approached the layout of the whole page with CSS grid is not bad but I think it lacks of flexibility and might be difficult to maintain in the long run. Here are 2 resources that will surely help you come up with a more efficient layout. Travis Horn - Responsive grid, CSS-TRICKS - Responsive Grid
- In the main content, you are serving the desktop image for smaller viewports. In the design file, some of these images become in portrait mode on smaller viewports. You can have responsive images by using the
<picture>
and<source>
elements in conjunction with an<img/>
element to serve the right image on corresponding viewport widths. Read more here. - Please check the styling for the heading as some of them are not picking the correct font family.
- The social icons look larger as compared to the design and the links to redirect the user to the respective social media website are missing.
I hope this helps.
Keep it up.
@francisldn
Posted
Hi @christopher-adolphe thanks so much for your detailed feedback. Really appreciate it. Love your solution, so many learning points for me. And cool animation using GSAP, I'm gonna start using it too. Great job to you!
@christopher-adolphe
Posted
Hi @francisldn,
I'm happy to help and glad to see these were helpful to you.
Keep on practicing and you'll see the result. 💪
Best regards.