LOOPSTUDIOS LANDING PAGE USING HTML AND SCSS

Community feedback

• 

  ricardoychino•190

  @ricardoychino

  Posted 9 months ago

  Hi,

  Congratulations! That's a great looking solution. Very readable and good semantics, too!

  There are two things that caught my attention that I'd like to point out:

  1. At hero section, I think it would be better if you just use background property to apply the image instead of using a <picture> with position: absolute;. It is easier to handle an image change or if the image doesn't have the dimensions that fits the section. As for the responsive source of the images, you can always use a media query.

  2. Initially I was intending to recommend the selector :nth-child() for the section where you have the products, instead of using selectors .product-{n}, but actually I don't think you need to set the grid properties from lines 374 to 391 and 425 to 440 in your main.scss. I disabled those properties on devtools and your design is still looking great! The only one that I kept is the grid-template-columns: repeat(4, 1fr); for desktop viewing. Why did I point this out? Because, let's assume you need to add another product eventually. You will then probably need to add a .product-9 rule to your CSS, or use one of the currently created .product-{n} on it. Either way, there are disadvantages: maintainability or consistency, respectively. If you remove those styling, the style will still be as you intended and there's no need to worry in case you need to add or remove new items

  About the JS interactivity:

  It seems like it's pretty much halfway done and that only needs to add a JS interaction. With your current code, I'd start adding a event listener to the button which adds (or in your current code's case, remove) a class to handle visibility of the menu, such as:

  /*
    This adds a reference to the target elements
    (This is optional, but I personally believe that this 
    makes the code a bit cleaner)
  */
  const buttonElement = document.getElementById('menu')
  const navElement = document.querySelector('.header-nav')
  
  /* Adds the event listener for clicks to the <button> */
  buttonElement.addEventListener('click', () => {
    // Check if nav is hidden
    if (navElement.classList.contains('hidden')) {
      navElement.classList.remove('hidden') // Shows nav
      buttonElement.querySelector(':scope .open').classList.add('hidden')  // Hide the hamburger icon
      buttonElement.querySelector(':scope .close').classList.remove('hidden') // Shows the close icon
    } else {
      navElement.classList.add('hidden')  // Hides nav
      buttonElement.querySelector(':scope .open').classList.remove('hidden')  // Shows the hamburger icon
      buttonElement.querySelector(':scope .close').classList.add('hidden') // Hide the close icon
    }
  })
  

  This is enough to toggle the show/hide in your current code and continue to style further or change the structure if you want to. You will see that the styling will need some adjusts after applying that toggling. I'd change a little this approach, but the way you did works fine as well.

  I hope this is what you needed and that it helps. I apologize if it didn't. Again, great job and keep it up!

  02

  Marked as helpful
• 

  1deadjoe•370

  @1deadjoe

  Posted 9 months ago

  Thank you. Sadly it is the one thing I could not do perfectly. I got too focused on the styling.

  00
• 

  andrean-sihombing•350

  @imandreans

  Posted 9 months ago

  Your solution is already similar to the design, while there are difference like layout and size. You can come back and improve it later.

  The solution is responsive, sadly there's no sidebar that appear when I click the menu button.

  Your code is well-written both SCSS and HTML. I like that you use picture tag to add the images.

  00