Skip to content
OverviewSolutions
7
Comments
7
Edoye
@DoyeDesigns

All comments

  • Edoye•170
    @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 helpful
    1
  • Edoye•170
    @DoyeDesignsPosted almost 2 years ago

    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
  • Edoye•170
    @DoyeDesignsPosted almost 2 years ago

    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
    @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

    QR component using CSS and HTML

    3
    Edoye•170
    @DoyeDesignsPosted almost 2 years ago

    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
  • Edoye•170
    @DoyeDesignsPosted almost 2 years ago

    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
  • Edoye•170
    @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
  • Edoye•170
    @DoyeDesignsPosted about 2 years ago

    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