Skip to content
OverviewSolutions
7
Comments
6
Avinash DV
@avinashdv

All comments

  • Avinash DV•150
    @avinashdv

    Hi, Nice work. I think you are a little confused with media queries, I would advise a few points that could make the website responsive, please consider these changes.

    The below media query states that all the devices that starts from 375px. we can consider 426px instead of 375 as modern mobiles width is much higher than 375px. @media only screen and (min-width: 375px)

    Change it to @media only screen and (min-width: 426px) All the code that we write in this media query will get applied to desktop as we stated the min-width 426 and above.

    Now for mobile, use @media only screen and (max-width: 426px) All the code that we write inside this will get applied to mobile. It actually states that 0 to max:426 only.

    Note: In the starter file, It is given that mobile and desktop as 375px and 1440px. Which therefore means the mobile responsiveness should start from 375px and below, above 375 it should be desktop.

    Marked as helpful
  • Avinash DV•150
    @avinashdv

    Hi, The webpage looks really cool, nice work. A minor UI bug I came across: On selecting the tags, if there are only 1 or 2 jobs, the background color is covered for only the 2 jobs instead of entire body. Changing the h-auto to height:100vh on the div which is inside <div id="__next"> will solve this issue. I hope this helps, happy coding.

    Marked as helpful
  • Avinash DV•150
    @avinashdv

    Hi, The project is very nice with respect to UI. I haven't gone through code, but one UIchange I could suggest is that: The last row contains only two countries, because justify-content:space-between is used that worked well if the row contains 4 countries, but with 2 or 3 countries there is a lot of space in between the countries and mis-alignment of columns.

    I would suggest to utilize 1 or 2:

    1

    • justify-content: flex-start;
    • /* justify-content: space-between; */
    • column-gap: 50px;

    2 :last-child pseudo element to take care of the last row with css

    Note: with (1) the column-gap might not be supported in all browsers and you can check it from https://caniuse.com/ and take the decision to provide support for cross-browser compatibility.

    I hope this helps, Happy Coding.

  • Avinash DV•150
    @avinashdv

    Hi, The table can't be kept behind parent container, then the image go below the white div.

    One way to do it is to cut an svg with viewBox by moving them around x & y axis. The first two values are x & y axis where as the next two values are width and height of svg.

    Here is a link that works for an svg within a div element: https://jsfiddle.net/avinash_dv/7cfxhuL9/1/

    Note: viewBox should be added to <svg > tag.

    Reference: https://www.geeksforgeeks.org/svg-viewbox-attribute/

    I hope this helps in solving the issue.

  • Avinash DV•150
    @avinashdv

    Hi @natalie-0073 I see that you haven't registered a click event in any of the ratings. They should be clickable and when the user click's on any rating, it should be stored in a variable. Upon clicking the submit button the value stored in the variable should be used to show the rating on "Thank you" Card.

    I've written on plain JS using event-delegation. Have a look to get some idea. Repo: https://github.com/avinashdv/interactive-rating

    Marked as helpful
  • Avinash DV•150
    @avinashdv

    Hi @JustinFowlerArt UI is neat and clean and is very responsive on all devices. I found a bug functionality wise. Once the user adds the product to cart and upon checkout from cart, the message displayed correct as 'Order received. Thanks for shopping!' and trash icon is showing the appropriate message, but If user adds product to cart again, and now if user clicks on trash icon from cart, I'm getting the msg as order received instead of 'Your cart is empty'. Apart from this, I really liked your work.

    Marked as helpful