Article preview component master challenge

Community feedback

• 

  Ezequiel•1,030

  @3eze3Posted 3 months ago

  Hello 👀, very good solution for this challenge, Jeancy.🍕

  I have a few recommendations that can help you in your future projects:

  General:

  Having a solid code structure is key when starting any project.

  • Use an assets folder to store images, icons, and fonts.

  HTML:

  <!-- Google font --> <link rel="preconnect" href="https://fonts.googleapis.com"> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>

  • Downloading the fonts directly would improve the page’s performance, as it avoids unnecessary domain resolutions. Additionally, in CSS you can use the @font-face rule to load fonts locally.

  • It would be better to contain the .social links within a <nav> instead of a <div>. This improves semantics.

  • Consider adding an aria-expanded to the share button to indicate whether the share menu (div.social) is visible or not, if it's interactive.

  • alt: Image descriptions should be more specific when necessary. For example, instead of alt="facebook icon", something like alt="Facebook logo" or a description more relevant to the context of use.

  • Instead of using a <div> inside the <footer>, it would be more appropriate to use a <section> or a <div> with role="contentinfo" for the section containing the author information.

  • Consider using the <figure> element to wrap the article image, as it's content related to the text and has better semantics.

  CSS:

  • Repetition of the value 14px in styles like .warning, .small, .footer. You also repeat values in the media queries, like :.photo img { position: relative; }.

  • Reduce styles for tags you don’t use in your project, as this also helps you decide how your reset will be and eventually create your own reset for specific projects.

  • I noticed the design breaks at 425px, and I believe it’s due to excessive unnecessary positioning. For example, .content-text should not be absolute. The element that should be absolutely positioned is .social, because it's this element that we don’t want to interfere with the design flow.

  • In the media queries, only change the minimal necessary styles. For example, if you set .card { display: flex; }, it’s not necessary to define .card { display: flex; } again unless you want to change the display. In general, minimize the styles.

  JS:

  • The problem in the original code is that the social variable was being obtained inside the click event, which caused the element not to be found on the first click, and the action wouldn't execute correctly. This happens because the social element wasn't available until the click event was fired, making the if condition never true.

  The solution is to move the declaration of the social variable outside the event, so it’s available from the start, before the click happens.

  Additionally, instead of modifying the style directly with social.style.display, it’s better to use CSS classes to manage the visibility of the element. Use classList.toggle('visible') to toggle the visibility of the social container using classes, which is more flexible and easier to maintain.

  ---

  I hope these recommendations were helpful. Keep it up, and if you ever have any questions or get stuck on something, feel free to reach out to me on LinkedIn.

  Happy coding! 🍧

  Marked as helpful

  1

  ---

  

  @jeancy•230

  @JeancyjrPosted 3 months ago

  @3eze3 Hello, I appreciate your feedback. Thank you for analyzing my code. Your recommendations are helpful.

  0
• 

  P

  Pierpaolo Pistorio•300

  @Zelos23Posted 3 months ago

  The solution is practically identical to the design. Congratulations and keep going!

  1

  ---

  

  @jeancy•230

  @JeancyjrPosted 3 months ago

  @Zelos23 Thank you. Appreciate.

  0