Skip to content
OverviewSolutions
3
Comments
11
Kalpesh Patil
@kalpesh172000

All comments

  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Really nice and clean code. You also added nice comments to denote part of the html file.
    • Solution looks really close to design.
    • Design is responsive.

    what can you improve

    • Use specified font family and colors to make the solution look more close to the design.
    • As you have used flexbox for body tag the content clips at top and bottom when height shrinks too much. To avoid this give min-height to the body so when height shrinks too much it makes the body scrollable and card won't clip.
    • After doing that add 24px padding to body for nice touch.
    0
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Really nice and clean code.
    • Solution looks really close to design.
    • Design is responsive.
    • This is first time ever that I have seen nesting in css. I'll research more about it.

    what can you improve

    • As you have used flexbox for body tag the content clips at top and bottom when height shrinks too much. To avoid this give min-height to the body so when height shrinks too much it makes the body scrollable and card won't clip.
    • After doing that add 24px padding to body for nice touch.
    Marked as helpful
    1
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Really nice and clean code.
    • Solution looks really close to design.
    • Design is responsive.

    What can you improve.

    • As you have used flexbox for body tag the content clips at top and bottom when height shrinks too much. To avoid this give min-height to the body so when height shrinks too much it makes the body scrollable and card won't clip.
    Marked as helpful
    0
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Solution looks really close to the design.
    • Code is clean and readable.

    Improvements

    • You provided the github profile link and not repository link.
    • You didin't give height to the body. you can can give it by following code.
    body {
    height: 100vh;
}
    0
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Solution is almost close to the design.
    • Code is easy to read and understand.
    • You used semantic HTML.

    Improvements

    • Update README.md
    • Make border 1px instead of 2px
    • You used media querri to change the dimentsion of the card but after breakpoint it touches the screen edges. to fix it add padding to the parent container.
    • There is better way to make responsive card <div> without using the media query. It is done with use of max-width and width(with % unit) as follows
    main {
    max-width: 384px;
    width: 100%;
}

    what above code does is that it allows code to grow with width 100% but only until 384px. By using this you wont need to use media querry.

    Marked as helpful
    0
  • Benjamin Siret•70
    @BenjaminSiretSubmitted 2 months ago
    What are you most proud of, and what would you do differently next time?

    RAS

    What challenges did you encounter, and how did you overcome them?

    RAS

    What specific areas of your project would you like help with?

    RAS

    Blog card - html css

    1
    Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Solution is very close to design.
    • Page is responsive.
    • Code is clean and easy to read.
    • Code is properly aligned vertically and horizontally.

    What can you improve

    • Once you set the max-width of the middle card, you can just add padding to card and set the width of inner element like <img> and <div>(which contains the paragraphs) to 'auto'. which would automatically scale the inner elements to the <div> so if size of div changes so does size of inner elements and you wont need to set media querries for them. In below example card can shring but can't grow beyond max-width. It is done with combination of max-width and width(with % unit).
    main {
    background-color: var(--white);
    max-width: 384px;
    width: 100%;
    padding: 24px;
}

.illustration {
    width: auto;
}

.card {
    width: auto;
}

    • correct font sizes before breakpoint are 14, 14, 24, 16, 14 and after are 12, 12, 20, 14, 14.

    • you should use sematic tags <main>,<header> etc the way you use <div>, so you could avoid layers of nesting by just appling the same properties to semantic HTML.

    Marked as helpful
    0
  • eze•10
    @quiellllSubmitted 2 months ago
    What are you most proud of, and what would you do differently next time?

    i think i got most of the idea right, and the final result shows something that works, right?

    What challenges did you encounter, and how did you overcome them?

    i tried to not look up anything, and being a newbie... kinda hard! i struggled a bit at the end adjusting the paddings and margins to be honest. the inspector wasnt helping much either, so i decided to submit to just move on to the next thing and avoid getting stuck on the same.

    What specific areas of your project would you like help with?

    well as i mentioned i struggled a bit with paddings and margins, and i know i didnt get them right on the submission, because when i changed one thing, it modified the rest(? not sure how or why tho.

    QR code component

    1
    Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good Things

    • Solution is really close to the design.
    • You've created css variables for reusability.

    What can you improve

    • Its the best practice to write ALL of your css in external css file and link it with link tag.
    • Use semantic HTML means use elements like <main>, <header>, <footer> instead of using <div> everywhere. This containers works exactly like div so you can apply same properties to them. Semantic HTML helps screen-reader users.
    • Make the main div responsive. you can adjust the size of main div by using the combinaiton of max-width and width properties. As follows.
    .container {
    max-width: 340px;
    width: 100%;
}

    what this does is that it allows div to grow but only until max-width size but when parent container or screen size shrinks it also shrinks as the width is set to 100%(of the parent width).

    0
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • solution is close to the design.

    Improvements

    • you can add the mentioned font from the googlefontapis by adding follwing line in <head> tag.
       <link
       href="https://fonts.googleapis.com/css2?family=Outfit:wght@100..900&family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&family=Rajdhani:wght@300;400;500;600;700&display=swap"
       rel="stylesheet">

    and using it as follows in css.

    body {
    font-family: 'Outfit', sans-serif;
}

    all the child and subchild elements will inherit the font-family.

    • you can align the main container vertically by making the parent container i.e body into flexbox and using follwoing css.
    .body{
    height: 100vh;
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
}
    • you can add border radius to img.
    0
  • Macdonald Nkhata•90
    @MacNkhataSubmitted 2 months ago
    What are you most proud of, and what would you do differently next time?

    I am most proud with the fact i was able to complete the example in one sitting and i was able to measure my level of comprehension on some of the concepts i learnt prior to taking on this project.

    What challenges did you encounter, and how did you overcome them?

    I had a problem making the borders of the image round to look like the one in the design so I went on Google to find the answers and Stack Overflow was handy.

    What specific areas of your project would you like help with?

    For now i dont need help

    QR Code Component

    1
    Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • You followed standard practice of creating the css variables, resetting the margins and paddings and so on.
    • Your solutions looks very close to the design. And additions like shadow really enhance the look of the card. Your solution is THE best one that I have seen.
    • The UI is responsive and works with all screen sizes.

    Improvements

    • Excesive use of nested <div> is making the HTML code less readable. You can achieve the result with less number of <div>s. For example <main> & <footer> themselves are similar to <div> you can apply same properties to them. This excesive use of div also resulted in large css file.
    • you assigned height and width for img in html attribute which is not a good practice. IT is a good practice to write 'ALL' of your css in external css.
    • Not everything needs to be in <div>, what i mean by the is, yes div are really good block elements can be adjusted with width and height properties. But you can do same things with many other elements too. for example you dont need to put img inside its own seperate div. as you can play with img's dimentions and position without need of putting it another div.
    0
  • Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    Good things

    • Design very closly matches to the solution.
    • Design is responsive for all sorts of screens.
    • You also considered the verticle allignment of main content.
    • Code is readable and it's easy understand your intentions.

    Improvements

    • It is good practice to put all the visible content inside the <body> tag. So try putting the <footer> tag inside body.
    • You can assign text properties like font-family for <body> tag, it will get inherited by all the child and subchild tags.
    • (This is bit subjective) You can use less number nested tags. what I mean by that is you used 2 div tags for main content. instead you could've used one less by using body tag as container. same with footer. footer itself is a container acts just like <div> so no need for making another div.
    Marked as helpful
    1
  • Khai•80
    @khlv2219Submitted 2 months ago
    What are you most proud of, and what would you do differently next time?

    I am most proud of recreating this design with less lines of code as well as be able to centering a div without problems :D

    What challenges did you encounter, and how did you overcome them?

    Firstly, I had a problem with the border-radius of the qr-code image. But then I saw that I changed something on the padding. This was the reason why the two upper corners didn't round up by the border-radius modification

    QR code component challenge

    1
    Kalpesh Patil•150
    @kalpesh172000Posted 2 months ago

    good things

    • Solution is very close to the design.
    • You created css variables for the reusability.
    • Solution is responsive.

    improvemnts

    • Use semantic html (i.e instead of creating <div> for container use <header>, <main>, <footer>, <nav> these act just like <div> but are better for screen-reader users.
    Marked as helpful
    0