Skip to content
OverviewSolutions
8
Comments
42
Rohit Purkait
@codeswithroh

All comments

  • Rohit Purkait•460
    @codeswithroh

    Everything looks fine bruv. Great work

    Marked as helpful
  • Rohit Purkait•460
    @codeswithroh

    In your github repo change the name of the index1.html file to index.html to be able to deploy the site properly.

  • Rohit Purkait•460
    @codeswithroh

    You did a great job regarding this solutions. Well, there are a few things that needs to be addressed.

    1. As you told there is that problem with your navbar which pops suddenly during the change in screen size

    2. There is a horizontal scroll which is a bit annoying

    3. I would suggest you not to set style inside the tags itself because it causes problems

    4. Your site looks good for screen sizes beyond 1440px but most of the user will view this site in 1440px and for 1440px the icons are a bit bigger.

    Except these you did an awesome job. So, keep coding 😊

  • Rohit Purkait•460
    @codeswithroh

    For some reason your site and repo is not visible. Maybe you have given the wrong links. Kindly look into that matter.

  • Rohit Purkait•460
    @codeswithroh

    You did an awesome job on this one. I tried to find mistake in your solution but I couldn't find any 😂. The responsivness is on point, the site scales up and down properly. So. a really goo job.

  • Rohit Purkait•460
    @codeswithroh

    You did an amazing job with this solution. Well, there are two things that need to be addressed.

    1. The password section keeps throwing an error even if the number of characters is more than 8

    2. You used way too many flex-box. You have used flexbox in places that doesn't need it. You see by default all the elements are places in a column. So, you don't have to use flex for each element and set it to flex-direction: column

    Except this two things you site looks pretty amazing. So, keep up the good work and keep coding 😀

  • Rohit Purkait•460
    @codeswithroh

    Congrats on completing your second project 😊. You have done a really good job. But there are some little things which need to be improved.

    1. There is no border-radius on the container for the curved sides

    2. The site is not fully responsive except the two targeted screen i.e. 1440px and 375px.

    Btw Keep coding and hope to see your next solution soon 😊

  • Rohit Purkait•460
    @codeswithroh

    I didn't checked the accessiblity issues. I should have checked them before submission

  • Rohit Purkait•460
    @codeswithroh

    You have done a great work. But there are some things that could be improved:

    1. Adding margins at the top and bottom of the whole layout for small screens. Because at small screens your layout sticks to the top and bottom

    2. Reducing the padding of the links for small screens is also required. Because it is overflowing the container.

  • Rohit Purkait•460
    @codeswithroh

    You did an amazing job on this one. I really liked it. The only thing that's bugging me is the little horizontal scroll in full-screen. To solve that just use

    body {
    overflow:hidden;
}

    Except that everything looks great.

  • Rohit Purkait•460
    @codeswithroh

    I really liked your work. Everything looks neat and clean. Only one thing is left and that is to add a hover effect on the footer social-icons. Except that everything looks great.

  • Rohit Purkait•460
    @codeswithroh

    To place your whole container at the center of the screen just add

    display: grid;

place-items: center;

    in your body tag. It will save you from using position:absolute.

    And for the responsiveness try setting a flex-basis with minmax which will help you a lot in responsiveness.

    You can check out my solution as well (https://github.com/codeswithroh/stats-preview)

  • Rohit Purkait•460
    @codeswithroh

    Instead of using border-radius just use border-top-left-radius or border-top-right-radius. The same follows for the bottom radius as well. This will help you to get the rounded corners in the specific regions as required. For more clarity you can check out my solution as well (https://github.com/codeswithroh/Simple-Card-Layout)

  • Rohit Purkait•460
    @codeswithroh

    You did an awesome job. In fact, I learned a new thing from your code ( the mix-blend-mode property). I created the same effect by putting a background color and setting the opacity of the img to 0.5 but your process looks better to me.

  • Rohit Purkait•460
    @codeswithroh

    Congratulations on completing your first challenge. For your queries,

    1. For making the site responsive make sure to use flexbox or grid to make the whole layout instead of positioning each element separately using "position: relative"

    2. The img source given in your site shows that your image lies within a folder called "3-card-preview" but in your github you have not put your images inside a folder so, that is why it is not showing. Try to remove the folder name from your <img> src or try adding a folder with the same name as above and put the images in it.

    Happy Coding 😃

  • Rohit Purkait•460
    @codeswithroh

    You did an awesome work. But there are some areas of improvements like

    1. Adding a max-width or margin to the h2 and p would have been nice

    2. Adding hover effects to the buttons and links would look nice as well

  • Rohit Purkait•460
    @codeswithroh

    You did an awesome work. But there are still some room for improvement regarding this solution like

    1. From the beginning of 970px the card-component sticks to the side of the screen

    The problem is caused by the margin in card-component class. You have assigned margin in three sides only. Just change it to "margin: 140px 100px" and that will solve your problem.

  • Rohit Purkait•460
    @codeswithroh

    First of all appearance-wise, bigger fonts and line-height would be a little better. Next about the coding part, I think that instead of setting a width in % for every component just write the html and give some padding and margin to it and instead of writing media query for each element separately try to write it in one place but its just my opinion, there's no harm in your style. For positioning I would tell you to follow the process I mentioned above because I personally don't like absolute or relative positioning because that makes the responsiveness a bit hard. But at the end you did a great work