Juan J• 30
@JuanJefry23
Submitted
P
kassahunAmdie
@kassahunAmdie
All comments
- PkassahunAmdie• 70
@kassahunAmdie
Posted
hi Juan, A good attempt on the challenge. You have the structure down. But few things could make a great change. I noticed you have set a dimension or restriction on the container and also on the image you have set width. I have learned these will cause a lot of issues on your end result. For your image set the display as block and set a max-width to 100%.
I also see you have used position: absolute on your container-card. I didn't see the need for this. Over all please watch Kevin Powell's responsive web design guide. I myself learned a lot. The biggest lesson was the browser will by default make your page responsive. We usually make it unresponsive by injecting unnecessary CSS. I hope this helps.
0 - PGonzalo Tejada• 130
@vgt3j4d4
Submitted
What are you most proud of, and what would you do differently next time?
I think I wouldn't do it any different.
What challenges did you encounter, and how did you overcome them?
- Showing a different banner on mobile devices
- Trying to use the correct aria roles
PkassahunAmdie• 70@kassahunAmdie
Posted
Great work Gonzalo. Your solution is clearly laid out and easy to follow. I even picked up few points from you. New way to think about.
1 - matt2282• 310
@matt2282
Submitted
What are you most proud of, and what would you do differently next time?
I enjoyed adding a shadow the background of my container. I would put my style in a style.css file instead of the html file. Using the space-between flexbox option makes the most sense for pages like this because the content at the bottom and top need to be equally spaced from the edges.
What challenges did you encounter, and how did you overcome them?
I was trying to make the yellow leaning text box only as big as the text but it was very large. It turned out to be an issue with default margins of the h1 header.
What specific areas of your project would you like help with?
Is my code deviating from any best practices?
PkassahunAmdie• 70@kassahunAmdie
Posted
hi @matt2282, good job in completing the challenge and you did good. The way you structured your code is easy to follow along. For the question you asked, "Is my code deviating from any best practices?", I noticed two major things you may want to consider.
- Try not to use "h1" more than once on your webpage.
- It is best practice to put your CSS in its own file. (Forget this one. I realized you have mentioned about it)
One last thing, as far as the solution is concerned is, you missed the hover state as outlined in the design file. May be you were like me, so excited you forgot about it. But all in all very good job. Keep working on it.
0 - Rinel• 120
@rineliniguezsosa
Submitted
PkassahunAmdie• 70@kassahunAmdie
Posted
hi @rineliniguezsosa, I am impressed with your work. The only thing I saw that I think you would re-consider is, setting fixed height on the .article selector. On the image it is ok but on most other selectors I have learned setting height has unintended outcomes. Great job
0 - frontpro• 70
@frontpro
Submitted
PkassahunAmdie• 70@kassahunAmdie
Posted
The improvement I would suggest is
- margin bottom on your .tag to push the date a bit down
- the font-size on content--head h3 needs to increase a few more pixels.
- I would add a line-height on your body to create a spacing to match the design. I had 1.5 that worked out for me.
I think these would get it close to the design size.
The structure of your code is great and easily understandable. Great work.
Marked as helpful
1 @d1ma-angel
Submitted
PkassahunAmdie• 70@kassahunAmdie
Posted
Hello d1ma-angel, Your solution is very good. The only suggestion I have is to use the Figma file to find the exact size. It may not be the perfect solution but I would try to limit the width and height of your .card in the css. The width you have there can get a bit lower. That worked for me for this project. Everything else is excellent in my opinion. I actually picked up some points to improve on my approach.
0
