Skip to content
OverviewSolutions
6
Comments
4
Ahmed Abu Qahf
@ahmedsomaa

All comments

  • Ahmed Abu Qahf•130
    @ahmedsomaaPosted 4 months ago

    Hey, amazing job. Congrats on completing this challenge!

    A few tips!

    HTML:

    • I like the fact that you used <figure/> as a container for the <img/>.
    • It would be better to divide your page into sections using <section/> element and give each one a unique id instead of using a <div/>, this would give it a better semantic meaning. Example of sections could be Ingredients, Instructions...etc.

    CSS:

    • You can import multiple fonts in one @import statement like this @import url("https://fonts.googleapis.com/css2?family=Outfit:wght@400;600;700&family=Young+Serif&display=swap");
    • Set the font-size: 16px for the whole document and then use rem for any other font-size like <h1/>, <h2/>. Also, your document's font size is set to 11px which way too small, make it 16px or 14px at least.
    • Change the color of the text in the document to stone-600 as this is much closer to the design that the one's you used.
    • Change the list-style of the Ingredients to square at least that's how I see it. It's too small.
    • Add a line-height: 1.5 or margin-top & margin-bottom for list items as this looks bigger in the design.
    0
  • Ahmed Abu Qahf•130
    @ahmedsomaaPosted 4 months ago

    HTML

    First, I want to commend you on the clear structure and use of essential HTML elements. The layout is straightforward, and it’s great to see you’ve included meta tags for viewport settings and a favicon. Here’s some feedback to further enhance your work:

    • Use <img> for the user's avatar instead of the background-image as this improves accessibility and SEO by allowing descriptive alt text and better semantic meaning, whereas background-image does not support these features.
    • Use alt text for images or add descriptive aria-label to interactive elements for screen readers.
    • Consider using <button> or <a> tags for interactive elements instead of div with title attributes.
    • Define or remove the container class since it's referenced in HTML but not styled in CSS.

    CSS

    Your CSS demonstrates a good understanding of layout and styling principles. The use of Google Fonts and consistent color schemes is commendable. Here’s how you can refine your work:

    • Add media queries to ensure responsiveness across different screen sizes (e.g., 320px to 1440px).
    • Include comments in the CSS to describe the purpose of different sections for better readability and maintainability.
    • Define styles for the .container class or remove it if not used.
    • Update the hover state color for .social-link to match the design specification (hsl(75, 94%, 57%)).
    • Ensure color consistency throughout the stylesheet to match the design guide, define them in the :root or using @property.
    Marked as helpful
    1
  • Ahmed Abu Qahf•130
    @ahmedsomaaPosted 4 months ago

    Great Job! I like that you did the whole challenge using the CSS selectors and neither using Flexbox nor Grid.

    A couple of things to consider here:

    • maybe next time try to look into CSS architectures like OOCSS, BEM...etc, they make maintaining your CSS much better.
    • try to make the website responsive for screens <= 320.
    • try to do it with Flexbox or Grid, see whether you'd prefer them or keep using CSS positions.
    0
  • Gakii•560
    @G-GakiiSubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    I was able to build a responsive website using media queries

    What challenges did you encounter, and how did you overcome them?

    The main challenge was making the website responsive

    Responsive QR Code Component

    2
    Ahmed Abu Qahf•130
    @ahmedsomaaPosted 4 months ago

    The HTML provided is mostly semantic, using a div for the container and including appropriate elements like h2 and p for headings and paragraphs. However, it could be improved by using more descriptive tags. For example, wrapping the entire content in a <main> tag could enhance the semantic structure of the document. You could also add a more descriptive alternate text to the image such as "QR code for Frontend Mentor website."

    Marked as helpful
    0