Art gallery website challenge

Community feedback

• 

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  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, the exclusion 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 the Modern 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 a max-width to it. In the design, the maximum width of the page is 1110px.
  • 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.

  1

  ---

  

  Francisldn•250

  @francisldnPosted over 2 years ago

  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!

  0

  

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  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.

  0
• 

  Richard Aiello•340

  @richieAielloPosted over 2 years ago

  Hey friend! I have a pretty useful article here about the clipped heading that helped me out when I was building this one.

  Basically you can use something like -

  background: linear-gradient(to right, #FFF 350px, #000 350px);

  background-clip: text;

  -webkit-background-clip: text;

  -webkit-text-fill-color: transparent;

  As long as you use the same end point for color 1 as color 2's starting point, the color will abruptly change from one to the other. With background-clip you can get this really cool effect on the heading. Play around with it and pick the correct start and end points for your project, I hope this helped!

  W3Docs - How to Apply CSS Styles to Only the Half of a Word or Character?

  1

  ---

  

  Francisldn•250

  @francisldnPosted over 2 years ago

  Hi @richieAiello, thanks for the advice, will try it out.

  0

  

  Francisldn•250

  @francisldnPosted over 2 years ago

  Hi @richieAiello I learnt from Christopher's comment below that this can be done using mix-blend-mode: exclusion or mix-blend-mode: difference. It works like magic.

  0
• 

  P

  Jason Nembhard•320

  @jNembhardPosted over 2 years ago

  Hey Francisldn,

  These pages are actually pretty clean. My advice? Build your next project with Styled Components or Sass instead of css. Some css code you write may not be compatible across all browsers and using these tools helps to keep the code consistent across all major browsers. Other than that, if you're building a portfolio, I'd definitely put this on there as it can be very eye-catching to people. Hope this helps!

  1

  ---

  

  Francisldn•250

  @francisldnPosted over 2 years ago

  Hi @jNembhard thanks for your useful advice. Will do for my next project.

  0