Hello this is my solution for this challenge , any suggestions ?
Edoye
@DoyeDesignsAll comments
- @Abdo-al-RSubmitted almost 2 years ago@DoyeDesignsPosted almost 2 years ago
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 helpful1 - @estebanp2022Submitted almost 2 years ago
This is my first project using Tailwind CSS!
@DoyeDesignsPosted almost 2 years agoHello 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 - @AishaakinSubmitted almost 2 years ago
Hi everyone, i tried using media query property but i can get it to meet the project challenge. Can someone please help me.
@DoyeDesignsPosted almost 2 years agoHello 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 - @angusgeeSubmitted almost 2 years ago
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
@DoyeDesignsPosted almost 2 years agoGood 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 - @mrkhd-webDevSubmitted almost 2 years ago
Feedback, tips, and best practices are most welcome.
@DoyeDesignsPosted almost 2 years agoGood 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 helpful0 - @kush-x7Submitted almost 2 years ago@DoyeDesignsPosted almost 2 years ago
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 - @JerryDcodemasterSubmitted almost 2 years ago
Cool challenge especially the Image Hover stuff is tricky but fun.
@DoyeDesignsPosted almost 2 years agoI 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 helpful0