AbdulRhman• 270
@Abdo-al-R
Submitted
Hello this is my solution for this challenge , any suggestions ?
Edoye
@DoyeDesigns
All comments
- Edoye• 170
@DoyeDesigns
Posted
Hello AbdulRhman, you've done a superb work. Your code is readable and easy to understand. I observed you used a lot of breakpoints. you were trying to target specific devices right?
Here are a few suggestions to improve your code. You can use word-wrap so that your text will be responsive based on the screen size
Marked as helpful
1 - Esteban• 340
@estebanp2022
Submitted
This is my first project using Tailwind CSS!
Edoye• 170@DoyeDesigns
Posted
Hello Esteban, I can see the effort put into this, you’ve done a wonderful job 👏🏾. Here are some suggestions to improve your code. Your HTML markup should contain a level one heading (h1). You can use an h1 tag and then style it to the text size you want.
I also observed you comment a lot, very detailed comments. You won’t have the luxury of commenting like that on bigger projects. You should practice commenting only when you want to explain concepts that may not be too clear to people who read your code.
Happy coding Esteban
1 - DayaA• 100
@Aishaakin
Submitted
Hi everyone, i tried using media query property but i can get it to meet the project challenge. Can someone please help me.
Edoye• 170@DoyeDesigns
Posted
Hello Aishaskin,
media query is a condition that has to be met before the elements on a webpage changes it’s layout structure to adjust to the user’s screen size.
For example /Default background color / Body{ Background: white; } / On screens that are 400px or less, set background color to red/ @media only screen and (max-width: 400px){ Body{ Background: red; } } /On screens that are 800px or greater, set background color to blue/ @media only screen and (min-width: 800px){ Body{ Background: blue; } } Hope this helps
Here is a link to better understand it https://www.w3schools.com/css/css_rwd_mediaqueries.asp
Also you should wrap all the work to be done in the body in a main tag.. html semantics, it make your page more accessible. So you can remove the div with the container class and use a main tag instead
You should not put a main tag inside a div. In terms of hierarchy a main tag comes before a div tag. So a div tag should be inside a main tag not the other way around.
To center the old price and new price add these codes in the parent element holding both of them. That is the div with the “inline” class;
.inline{ Justify-content: center; Align-items: center; }
Feel free to reach out to me for more clarification. Happy coding:)
1 - Gus• 60
@angusgee
Submitted
Hey folks!
- The main div.container has been pushed up above the centre of the screen by the attribution section. How can I center it, please? I can't see why the attribution section makes it take up so much space? Neither div.container nor div.attribution have any padding or margin...
- Is the shadow correct on the main div?
- Is the use of CSS Grid to center the main div, then CSS Flexbox to centre the child items, correct? Or is there any easier way to create this 'single page, single element, everything centred' layout?
Appreciate you all! <3
Edoye• 170@DoyeDesigns
Posted
Good work Lord robins!
You do not need css grid to center the page Elements. You can add;
Body { Display: flex; Min-height: 100vh; Justify-content: center; Align-items: center; }
You can check how I did mine. I reduced the amount of nested divs by grouping some elements into one div.
2 - Markhadi• 130
@mrkhd-webDev
Submitted
Feedback, tips, and best practices are most welcome.
Edoye• 170@DoyeDesigns
Posted
Good work Markhadi! Nice attention to detail.
The mobile view is very ok.
In your Html markup the div with class= “rated ml-4” you closed the div like this </div class= “img-star”>. You did this several times to close some div tags.
Attributes are only allowed on start tags
Marked as helpful
0 - Kush🫴• 225
@kush-x7
Submitted
Edoye• 170@DoyeDesigns
Posted
Hello kush-x7 Your code is very neat and easy to read. And thank you for the resources I’m about to go check them out.
I also like the initiative of adding css animation to your design. It’s cool. I’ll start thinking outside the box too.. nice work
0 - Joseph Jeremiah• 120
@JerryDcodemaster
Submitted
Cool challenge especially the Image Hover stuff is tricky but fun.
Edoye• 170@DoyeDesigns
Posted
I like your work.. it’s neat. You have a nice eye for detail. Instead of using <section> tag you should use <main> tag. I don’t think it’s necessary to put your icon inside a link <a>tag Check your mobile view… it’s not how it’s supposed to be. Happy coding
Marked as helpful
0
