@diegoweb3r
Latest comments
- @Thewatcher13
- Your perfume is not a heading, just a p tag
- the old price shoukd have the s tag , the New just a span
- use the picture element for desktop and mobile pictures
- work mobile first
- dont set a height on your container the content does that
- @diegoweb3r@Thewatcher13
- Dont set a height on the cardcontainer, the content provides the height.
- Do not use absolute values for font size but relative rem values.
- Your mobile version has no picture and the text is definitly not well aligned.
- Use the picture element in html fir both picutres (desktop and mobile)
- Dor the space between elements, on figma you can use the alt button, so click by example on the text in figma , hit alt and then move your mouse to the side that you want to know the space.. if you have figma
- work mobile first, so design first the mobile version, then the tablet and after tablet the desktop
- use rem instead of px for media queries
- U should use an ul for the stats
- @Williais@Thewatcher13
Hi
I've a few things to say:
html
- use a main landmark role
- dont skip the heading order you cant have an h2 before h1
- your alt on the img should be much more clear a qr code to where?
css
- There is no need to import a charset in your css..why did you do this?
- dont use px for font sizes , but rem, look at the post from fedmentor called "why you should never use px for font size"
- never set a height on a container the content provides the height
Marked as helpful - @Renatouhu@Thewatcher13
Hi
Good that you used landmarks, but a few things:
html
- you're image should have a meaningfull alt text
- dont skip the heading order, uou cant use an h2 before you used an h1
- your section should be removed, make the main the container of your project.
css
- be consistent in your values, font size should be in rem
- you dont need a media query here, but also, the media query is to big and should be rem instead of px
Marked as helpful - @aleknovkovski@Thewatcher13
Hi Alek Can you take a look at my solution for the landing page? What do you think of my solution for the curved border?
- @vedadnurkanowitz@Thewatcher13
Hi, I've read your code an have a few things that you can change:
html
- You should have a main landmark role in every project.
- Don't skip the order of headings in html, you can't have an H2 before you have H1
- do you know the meaning of strong? Strong isn't used for bold text, it says to screenreaders ("read this line with emphasis")
- Apply headings to: your result/ great and summary
- The 76/100 should be one p tag, if you're css breaks it seems now like:
76
/100
with one p tag: 76/100
- The right column shoud be an ul
- Use landmarks in your html
So your html is important for markup your website. Your html should be:
- Readable without any line of css, think of it like a plain structured text
- If a developer read your code without looking on your website, they should be able to know that is the main thing (h1) that is an ul,...
css
- Use a css reset, look on Andy's bell his website for a good and clear one
- Never ever set font-sizes in px (absolute value), but in relative (rem) https://fedmentor.dev/posts/font-size-px/
- give the h1 and p tag also a class, it is better to do this.
- Your breakpoint should be much smaller then 1440px, something like 1024px or 768px (but convert to rem)
