@Mnaqor66
Submitted
@vstm
@Mnaqor66
Submitted
@vstm
Posted
Congratulations on your solution, the code is really clean and easy to read. I liked how you solved the responsive image using the hidden/block method with the breakpoints - why didn't I hink of that :D. I don't have that much to add to improve it, but there are some things that caught my eye (and some of those points are a bit nit-picky):
<img>
-tags could be removed<h1>
tags, one for the product name and one for the price, which is not strictly wrong but here it could be easily avoided - for example using a <h2>
, a <strong>
tag or just a plain <span>
tag for the price (there is no semantic HTML tag for a money amount as far as I know).xl:w-2/5 lg:w-3/5 w-11/12
- I'm not sure if those width's are necessary, just setting the max-w-*
might be sufficient (I haven tested it though).rounded-bl-2xl rounded-tl-2xl
could be shortened to rounded-l-2xl
, and rounded-tr-2xl rounded-tl-2xl
could be shortened to rounded-t-2xl
. This one is a matter of taste.text-neutral-darkGrayishBlue
I would have set it up so I could write text-neutral-dark-grayish-blue
). Again this one is just a matter of taste.So again, great solution overall - I actually learned about the ratio-widths (w-11/12 etc) which I didn't know. I nevertheless hope my points might help you a bit.
Happy coding! :D
@Parasdeveloper8
Submitted
@vstm
Posted
Congratulations on finishing that challenge, nicely done!
I have looked at your code and I have found some things that I would recommend to improve your code:
margin-left
and margin-right
of 25px to almost all of your elements in your content-area. This can be done a bit easier by applying a single padding to your #menu
element, this way you can define the spacing once in the parent element and you don't have to add it to your child elements<table>
HTML element. This makes aligning the content inside the table easier (but I have to admit that styling a table can also be tricky).display: flex; justify-content: center
to the body
element to center the menu.I hope these pointers help you out a bit, don't hesitate to ask if you have any question about my feedback
@Eucalyptus2
Submitted
What are you most proud of, and what would you do differently next time?
I did a walkthrough code along and had trouble linking my second time qr code without help. The second time around I was able to mimic somethings so im most proud of that. Next time I will do it without help and just simply submit lmao
What specific areas of your project would you like help with?
All feedback is welcomed and appreciated!
@vstm
Posted
Nicely done, congratulations on finishing your first challenge :D
I think your HTML looks really clean, I also like the comments in your CSS (comments are something I should use more often).
Here are some things I think you could improve:
<link rel="stylesheet" href="Outfit.zip">
can be removed, since stylesheets should be CSS files directly, the browser doesn't know what to do with a ZIP in this context<link href='https://fonts.googleapis.com/css?family=Outfit' rel='stylesheet'>
can be moved inside the <head>
tag. The browser handles it correctly but if you check out the DOM in the inspector that is because the browser moves it automatically inside the <head>
tag.<img id="qr" ...
-> I would change that id
attribute to class
and use a class selector in your css (#qr
-> .qr
). This way your "way of styling" is consistent (all the other elements you also style with class selectors). Also depending on who you ask ID selectors are discouraged because they are not reusable. For example and you can only style one element per page with it, because only one element can have id="qr"
.I hope my comment helps you, if you have any questions regarding my feedback don't hesitate to ask me :D