Sass modules & Vanilla js Blogr Landing page

Community feedback

• 

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  Hi Deniel!

  The markup, the styles and the JavaScript are clean and well organised. 👌

  I noticed some minor things you could improve:

  • There is a small horizontal movement on the links of the main navigation as you hover over them. It is being caused by your _font.sass partial which is applying a generic rule of font-weight: 700; on all anchors. Maybe you should override this rule in your _header.sass partial.
  • It would be nice to dismiss the main navigation dropdown whenever the user is clicking elsewhere.
  • Same for the mobile main navigation, if any of the dropdown was activated, it would be nice to dismiss it whenever the user toggles the navigation to close.

  I am also curious to know what is the use case of having: 🤔

  <img src="./img/illustration-laptop-mobile.svg" alt="Laptop powerful tooling">
  

  vs

  <picture>
    <source media="(min-width: 55em)" srcset="./img/illustration-laptop-desktop.svg">
    <img src="./img/illustration-laptop-mobile.svg" alt="Laptop powerful tooling">
  </picture>
  

  You did a great job for this challenge. Keep it up! 👍

  Marked as helpful

  1

  ---

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @christopher-adolphe Thank you so much for the feedback and great comments!

  I totally agree on ignoring the drop-down menus as you say and I wanted to do it but I'm running out of ideas on the logic to write to do it, do you have any advice? I was thinking of putting a listen event on the entire DOM but I find it quite over the top and punchy for performance as well.

  I use img when putting an image without special needs, while I use picture if I need more features such as in this case changing the image when a certain resolution is reached. By doing so I write a clean and essential code.

  Happy coding!

  0

  

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  Hi @denielden.

  This picture technique looks very interesting, I will give it a try.

  Adding an event listener to the entire body would definitely be costly. I've checkout your solution's git repo and tried the following which seem to do the job of dismissing the dropdown.

  • Added an id to the ul element:

  <nav  id="navbar" data-visible="false">
    <ul id="menu"></ul>
  </nav>
  

  • Refactored the JavaScript logic for the navbar:

  const navToggle = document.querySelector('.mobile-nav-toggle'),
  nav = document.querySelector('#navbar'),
  menu = document.querySelector('#menu');
  
  const openSubmenu = (event) => {
    const { target } = event;
    const parentMenu = target.parentNode;
    
    if (parentMenu.classList.contains('open')) {
      parentMenu.classList.remove('open');
      return;
    }
  
    const activeParentMenu = document.querySelector('li.has-submenu.open');
  
    if (activeParentMenu) {
      activeParentMenu.classList.remove('open');
    }
  
    parentMenu.classList.add('open');
  };
  
  const dismissSubMenu = () => {
    const viewportWidth = window.innerWidth;
    const activeParentMenu = document.querySelector('li.has-submenu.open');
  
    if (activeParentMenu && viewportWidth > 879) {
      activeParentMenu.classList.remove('open');
    }
  };
  
  navToggle.addEventListener('click', () => {
    const visible = nav.getAttribute('data-visible');
  
    if (visible === 'false') {
      nav.setAttribute('data-visible', true);
      navToggle.setAttribute('aria-expanded', true);
    } else {
      nav.setAttribute('data-visible', false);
      navToggle.setAttribute('aria-expanded', false);
  
      const activeParentMenu = document.querySelector('li.has-submenu.open');
  
      if (activeParentMenu) {
        activeParentMenu.classList.remove('open');
      }
    }
  });
  
  menu.addEventListener('click', openSubmenu);
  menu.addEventListener('focusout', dismissSubMenu);
  

  Instead of adding a click event listener to each li, you can leverage on the event delegation technique and extract the logic to the #menu element.

  While I was checking when the mobile navigation kicks in, I noticed that on a viewport width of 880px, the navigation is getting a kind of in-between style of mobile and desktop.

  Marked as helpful

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @christopher-adolphe Wow! Great solution and refactoring, I have already made the change :)
  This feature had bothered me for some time and I couldn't find a solution.

  I used an 880px viewport because I prefer to create as few breakpoints as possible and only when the design breaks.

  Thanks again! And happy coding :)

  0

  

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  Hi @denielden,

  I'm happy to help and glad that those feedback were helpful 🤜🤛

  I use the same approach regarding breakpoints; the fewer, the better 😎 Nowadays, there are so many devices, we just can't add breakpoints for each of them.

  Best regards

  1
• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  Hey Deniel,

  It looks awesome! Love the extra modals you added and the effect how the navbar fades in as you scroll down. Its overall pretty impressive job you did here!

  I'm unsure how to fix your issue but.. I'd start looking at the fadeIn animation you added to your modals. It has a transform-origin: top which might be the source of the problems. I'm not a 100% sure but it's worth looking at it. Maybe it's a good idea to define separate keyframes for the modals and play around with it a bit!

  But apart from this small issue, I'm bookmarking your solution now >.<

  Marked as helpful

  1

  ---

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @FluffyKas Thank you so much, I'm very glad you like it :)

  I just did as you feedback me and you were right! I haven't thought about it at all and now as you can see it doesn't do it anymore, thanks again :)

  Happy coding!

  0
• 

  MOHAMAD NAZIR MD NAZRI•180

  @NAZIRwill29Posted over 2 years ago

  Hi deniel,

  your website really awesome.

  I really love the profile picture and attribution.

  But, for mobile-nav-toggle, it is better to make it in div rather than in the image.

  HTML:

  <div class="container" onclick="myFunction(this)">
    <div class="bar1"></div>
    <div class="bar2"></div>
    <div class="bar3"></div>
  </div>
  

  CSS:

  .container {
    display: inline-block;
    cursor: pointer;
  }
  
  .bar1, .bar2, .bar3 {
    width: 35px;
    height: 5px;
    background-color: #333;
    margin: 6px 0;
    transition: 0.4s;
  }
  
  /* Rotate first bar */
  .change .bar1 {
    -webkit-transform: rotate(-45deg) translate(-9px, 6px) ;
    transform: rotate(-45deg) translate(-9px, 6px) ;
  }
  
  /* Fade out the second bar */
  .change .bar2 {
    opacity: 0;
  }
  
  /* Rotate last bar */
  .change .bar3 {
    -webkit-transform: rotate(45deg) translate(-8px, -8px) ;
    transform: rotate(45deg) translate(-8px, -8px) ;
  }
  

  javascript:

  function myFunction(x) {
    x.classList.toggle("change");
  }
  

  Marked as helpful

  0

  ---

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @NAZIRwill29 Thank you! I'm very glad you mulch :)

  In the way you recommend the animation would be very nice!

  But why do you think it is better to write all that code instead of using the image?

  0

  

  MOHAMAD NAZIR MD NAZRI•180

  @NAZIRwill29Posted over 2 years ago

  @denielden well, I just like the smooth animation change shape rather than drastically change the shape. Well, it just my opinion 😊

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @NAZIRwill29 Of course, I am not saying that your approach is wrong, on the contrary I agree that the animation is beautiful 😁

  But I find that it is a lot of code to write compared to how it can be solved with far fewer lines.

  Thanks for the feedback! And happy coding 😊

  1

  

  MOHAMAD NAZIR MD NAZRI•180

  @NAZIRwill29Posted over 2 years ago

  @denielden yeah, I agree with you, the code is really long.

  I don't know if there is another way to make it. it would be nice if we can make smooth animation with images only.

  1
• 

  Prabhash Ranjan•2,540

  @besttlookkPosted over 2 years ago

  hi Deniel, This looks great. I have not done it yet. Will do this as my next challenge. I like know few things tho.

  1. How you style scrollbar??
  2. How do you toggle the visiblitity to "go to top button". I loved both things and like to add in my next project.

  Thanks, #happyCoding

  2

  ---

  

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  @besttlookk Thank you very much, I'm glad it inspires you!

  The scroll bar styles it in the CSS using these properties:

  html
      scrollbar-width: thin
      scrollbar-color: $light-red $dark
      scroll-behavior: smooth
  

  and for webkit support:

  ::-webkit-scrollbar
       width: .7rem
  ::-webkit-scrollbar-track
       background: $dark
  ::-webkit-scrollbar-thumb
       background: $light-red
       border-radius: 1rem
  

  To activate the visibility on "go to the top button" just write a little javascript logic: if window scrooldown by 200px then change the button style to display: block

  I hope it helps you ;)

  0

  

  Prabhash Ranjan•2,540

  @besttlookkPosted over 2 years ago

  @denielden Thank you Daniel. I am going to apply this my presest challenge im doing.

  Keep Inspiring.

  #happyCoding

  1
• 

  Steven Portas•440

  @satropPosted almost 2 years ago

  Hey hey!

  Outcome looks great! Not sure if anyone else mentioned this but your using Node-Sass, nothing wrong with that… for now, Node-Sass got up dated to Dart-Sass. The msg or difference is the deprecation of @import. This is being replaced with @use and @forwaed. So you might want to start looking at how to use them.

  1

  ---

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  @satrop Hi Steven, sure I'm aware of the deprecation. In my development environment at the time of the project it was not yet supported so I used @import.

  I haven't updated sass because my attention falls on other more important projects.

  Thanks for looking at my code :)

  All the best and happy coding!

  1