Skip to content
OverviewSolutions
13
Comments
7
P
I feel Struggle
@Kein-Internet

All comments

  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    Very good job in getting it close to the design.

    0
  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    Close to the design, good use of CSS variables, added padding the body so it doesn't touch the sides, use of rem instead of px. Great work! Just one thing:

    • A <h2> tag should never be above a <h1> tag as screen readers and assitive technologies for the visually impaired rely on the logical heading structure for navigating users on your site. Not having proper heading orders will confuse the users that rely on these assitive technologies. I suggest you ammend the heading order and try to style them to the design.

    • The colors for the cards and the background are also inverted.

    Keep it up.

    0
  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    It is responsove and the hover effect is good, nice. Here are some takeaways from this:

    • You must use rem instead of px when setting the margin/gap, padding, and font-size so that they accomodate the users defult broswser font-size. Doing this is necessary for making an accessible site.

    • On accessibilty, please use appropriate HTML semantics and less <div> and <p> tags, for examlpe use you could wrap your content in a <section> tag rather than a <div class="left"> tag. One exception is the use of <h2> tag, you can use <p> instead as it's just a price, not a title/subsection.

    • Use CSS variables to increase readibility and reusibility.

    • You're missing the mobile app image, please add this to your code as to ensure your code accurately matches the design.

    Marked as helpful
    0
  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    Good use of semantic tags, making your code reponsive, and use of css variables, it makes your code readible and maintainable. Here's a few suggestions:

    Make sure your paying attention to detail with spelling (e.g., "protein" instead of "protien") in your html markup, to ensure your code aligns as much as possible to the draft.

    • For your CSS, it would suffice to use just one breakpoint. Since you started with the mobile first-approach you can remove @media only screen and (max-width:375px) from your code.

    • With the exception of main{ width: 800px; background-color: var(--color-white); box-sizing: border-box; padding: 35px; border-radius: 15px; margin-top: 100px; margin-bottom: 100px; }, you can remove main from all of your selectors as all of the code you have developed is already inside the main section so it's not needed.

    • Use rem instead of px when setting the margin/gap, padding, and font-size so that they accomodate the users defult broswser font-size. Doing this is necessary for making an accessible site.

    Marked as helpful
    0
  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    There's very little to say about this one, the HTML and CSS code are good for the most part, very nice.

    A couple of suggestions would be to use seperate the flex code as it's own utilty class and just applying that class to the necessary components, so avoid you repeating the same chunk of CSS for each element. I would also use rem instead of px when setting the margin/gap, padding and font-size so that they accomodate the users defult broswser font-size. Doing that is necessary for making an accessible site.

    1
  • Larisa•130
    @LarisaKampeSubmitted 3 months ago
    What challenges did you encounter, and how did you overcome them?

    For some reason even when the margin was set to 0 in the that contained User image and User-Name it still somehow put some margin in the user name? Anyway i had to set margin-top and margin-bottom separately to solve this. I am not sure why but it worked 😅

    Blog preview card - practice 2

    1
    I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    Good job on getting your solution close to the design plus the active state!

    Here are a few suggestions for your HTML:

    • Replace your <section> tag with a <main> tag. This is because every webpage needs a <main> that wraps all of the content, except for <header> and footer>. This is vital for accessibility, as it helps screen readers identify a page's "main" section.

    • Make sure your class naming is consistent throughout your code, to make it easier to maintain and for others to follow, as I noticed inconsitencies with your classes (e.g, img-card and the div card below is card-text). Also, it would suffice to write btn instead of btn-main as replacing the <section> tag with the <main> tag will make it more obvious for other looking at your code.

    • It's vital to be descriptive in the alt part of your image <img class="user-img" src="./assets/images/image-avatar.webp" alt="user">. A good alt might be the name of the user.

    • I would use <h2>, you can get away with not using <h1> as the card would most likely be part of a larger site where there would already be a <h1> title in use but I would not use <h3> as screen readers use these heading tags to help navigate users of limited visibility. On that note, using the <h6> is not necessary and can be replaced with a regular <div> tag.

    For your CSS:

    • Use the rem unit instead of px when determining font sizes, rem will automatically adjust to the user's deafult browser settings and so it will size according to what it comfertable for them and mitigate the issue of user having to manually adjust the screen to be comfertable.

    • I would add a padding: 1rem to the <body> so the card isn't touching the sides of the screen.

    Keep up the good work.

    Marked as helpful
    0
  • I feel Struggle•210
    @Kein-InternetPosted 3 months ago

    It is good that you followed the design specs and your HTML semantics are very reasonable.

    A few suggestions for your CSS code:

    • You can remove the width: 100vw and the margin: auto you have already centered the card the flex stuff in your .container class.

    • For accessibilty reasons, it is necessary to always use rem instead of px when adjusting the font-size. A visually impaired user might want to increase their deafult font-size in their settings and rem makes sure that the font size corresponds to thier change. Pixels does not do this making it an accessibilty concern.

    • It is more safer to use max-width instead of width for your .container class, using width can cause overflow of content on smaller screens (although testing your website, I don't see this problem). By using max-width, you can choose a the width you want your container to be, while also ensuring that it will shrink on smaller screens.

    Keep it up!

    0