If anyone wants to check out my code and let me know any issues, please do! Love to see what I can fix and improve on.
Scott Tabor
@scottttaborAll comments
- @scottttaborSubmitted over 1 year ago@scottttaborPosted over 1 year ago
Realized I forgot to change the font weights! Other than that, I feel pretty good about this one!
0 - @ashleyftwSubmitted over 1 year ago
This is my first challenge project! I wasn't sure if I need to add media query here. I started with the mobile design first. Would love any feedback! Thank you!
@scottttaborPosted over 1 year agoHey there Ashley! For this specific challenge I did not use a media query. For these types of challenges where it is just a small component like this one I set the card to be a maximum width that will fit pretty much all screen sizes. You could technically get very specific and make media queries for specific screen sizes, but in general I think the component looks just fine with one standard max width.
The main layout of my code looks like this usually.
<main> <div class="container"> <div class="card"> content </div> </div> </main>
Main tag covers the whole screen, container will contain my card and cards content then that card class will be set to a max width to where it fits all screens.
Also sidenote, I am pretty new to all this as well. So if I am wrong on anything and anyone who reads this sees that, please correct me! I don't want to give out any wrong info.
One more thing! Don't forget to set your images
<img>
tags to fill 100% of their parent containers, so it will fit in the container nicely. You don't always want the image to be its native size. This would make your image shrink and not make your card so big!0 - @Raul-maderoSubmitted over 1 year ago
I would gratefully accept every suggestions
@scottttaborPosted over 1 year agoHey Raul, looks nice, the only thing I would try to fix is some of the margin (specifically bottom margin on the image. Looks a like a little bit too much. Also, I am very new to working on actual projects and am a beginner to coding in general so I would love a second opinion on this, but your last p tag, you have set it to be displayed as inline block, which in this case I don't think you need to. Its default setting of just "block" should be suffice. Some extra code that I think you can get rid of :)
Marked as helpful0 - @scottttaborSubmitted over 1 year ago
I used Javascript to be able to change the source of the image instead of HTML srcset. I just wanted the practice of using javascript on a project! Id love any feedback on the format of my code, how I wrote my javascript code and included it in my HTML. My only question is that the product card is responsive in all the mobile options except when it goes below a width of 375px there seems to be some white space that shows below the main background. Any tips for why that is happening would be appreciated.
@scottttaborPosted over 1 year agoI actually figured out what was wrong immediately after I uploaded this. On my main tag I had the minimum width set to 375px, once I took that out It solved my issue.
0 - @yuvan05Submitted almost 2 years ago
Hi, is there any problem with my code?
@scottttaborPosted almost 2 years agoHey Yuvan! I think for the most part your code looks good! It matches closely with the initial design. I am a newbie when it comes to coding so feel free to take my advice with a grain of salt, but the only thing I saw in your stylesheet is that you didn't include the font weights for the text on the card. In the style guide it mentions that the font weights were 400 and 700. I don't know how crucial this would be in an actual production setting, but it might be important. I believe the "heading text" would be the 700 weight, while the text after would be 400. Thats just my eyeball test. But again, other than that I thought your code looked good. Clean and precise.
Marked as helpful0