@qluback
Submitted
@Kein-Internet
@qluback
Submitted
@Kein-Internet
Posted
Very good job in getting it close to the design.
@ICode88
Submitted
@Kein-Internet
Posted
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.
@trajev
Submitted
What are you most proud of, and what would you do differently next time?
@Kein-Internet
Posted
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
@shubhamr10
Submitted
@Kein-Internet
Posted
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
@MatPawluk
Submitted
@Kein-Internet
Posted
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.
@LarisaKampe
Submitted
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 😅
@Kein-Internet
Posted
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
@sherlineau
Submitted
@Kein-Internet
Posted
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!