Skip to content
OverviewSolutions
13
Comments
10

TechNech

@TechNech340 points

Technology enthusiast, Web developer, Knowledge is power and learning is a virtue!

Latest solutions

View more solutions

Latest comments

  • TechNech•340
    @TechNechPosted almost 3 years ago

    Hey, nice work you have done there. I would like to suggest a few changes to your CSS:

    • Remove all the margin properties from your div with class outerbox, Then just add
        min-height: 100vh;
    display: grid;
    place-items: center;

    . This will place your component right in middle.

    Also I haven't seen you read the full challenge description. You need to add JS etc and another 'Thank You' component also. It's also good practice to separate your CSS and HTML code.

    Marked as helpful
    0
  • TechNech•340
    @TechNechPosted almost 3 years ago

    Hey, hope you are doing good. I would like to suggest a few changes in your challenge:

    • In your .card class, remove height and display properties.

    • In your .rating a class, remove max-width and add width: 50px; height: 50px;, change border-radius: 50%; and padding: 1rem;

    Also I have noticed you are using rem and px values mixed up which creates lot of issues. What I have learned is to use rem values for things like padding, margins etc and em values for fonts, which also helps in responsiveness.

    Hope I helped, Take care.

    Marked as helpful
    0
  • TechNech•340
    @TechNechPosted about 3 years ago

    Hope you are doing good. I would suggest you these few quick fixes on your component site.

    • In your .container you don't need that much of margin you can do is to simplify that property to margin: 0 auto

    • Then add these properties height: 100vh; display: flex; flex-direction: column; justify-content: center; to your body css, which will center your component.

    • In .qr-text simplify padding: 1rem;

    • In your .qr-text__p add max-width: 25ch;

    Marked as helpful
    1
  • TechNech•340
    @TechNechPosted about 3 years ago

    Hey! Hope you are doing good. I checked out your work and it's very good. I would like to point out a few things:

    • The QR component is not centered, to fix that you can add height: 100vh; display: flex; flex-direction: column; justify-content: center; properties to the body in CSS and it will center it.

    • Also try to use HTML5 semantic markup as much as you can, for example: instead of your .main-box div you can use main tag instead and for the wrapper you can use section tag.

    Hope this helps, Take care! Happy coding. :)

    0
  • TechNech•340
    @TechNechPosted about 3 years ago

    Hope you are doing good. I would suggest that you:

    • Add a css property for img with a max-width: 100% that will fix your issue.
    • Also you can use object-fit property on the parent div. Be sure you do some research on these first.

    Let me know if I can be of any help.

    Cheers! Happy Coding. :)

    0
  • TechNech•340
    @TechNechPosted about 3 years ago

    Hi, good works it almost looks similiar to the design. I think you don't need to change any of it.

    • You don't have any .container property setup in CSS, I suggest you set it.
    • Also on your qr class add display: flex; then you can align items to center just justify-content and align-items.
    • I see in code you have added script tag which isn't used, so remove that.

    Always try to use semantic HTML5 tags instead of a lot div, it will keep your code cleaner. The rest is good work!

    Happy Coding! :)

    Marked as helpful
    0
View more comments