responsive article-preview-component

Community feedback

• 

  Aleksei Bodeev•370

  @alekseibodeevPosted 3 months ago

  Hi 👋 Nice solution!

  Here are somethings to improve:

  Mobile tooltip

  There is layout shift on mobile when toggling tooltip. This happends because you changingdisplay property on <figure> element and removing padding-bottom on .card. The easiset way to avoid those shift is to use positioning properties:

  @media (max-width: 600px) {
    .card .mobile-active {
      /* don't touch padding here */
    }
  
    .mobile-active figure {
      /* don't touch display property here */
    }
  
    .person-info {
      position: relative;
    }
  
    .mobile-active .share {
      position: absolute;
      inset: 0;
      /* width: 100%; you don't need this anymore */
      /* you other styles */
    }
  }
  

  Code snippet above will fix layout shift but you still have to adjust it a bit to look nice.

  Code style

  I would recomment to avoid using type selectors for specific styles. Use it only for resetting/normalizing CSS. For example, you have display: none applied to ul. What if you need another ul on the page? You will need to overwrite display property for each new one.

  JavaScript Variables

  For this small project this makes no difference, but it's good prectice to avoid var keyword since it can introduce bugs later on. Use const for all unchanging variables/refferences and let for anything else.

  I hope that was helpful. Bye!

  Marked as helpful

  0

  ---

  

  Rabel•190

  @RabelahmedPosted 3 months ago

  @alekseibodeev thank you for the feedback

  0