Skip to content
OverviewSolutions
30
Comments
8
Deepanshu Gupta
@Deepanshu-5288

All comments

  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello @Mikssxed, It's really looking good just a little suggestion, When the screen size is between desktop and tablet unwanted cropping is happening always make sure that at every size of a screen all the contents are structured correctly and vertically also on desktop screen since its a one-page website so try to avoid scrolling and align such that scrolling is not happening. For a better understanding of what I am trying to say you can check the below solution : https://www.frontendmentor.io/solutions/responsive-socialproofsectionmaster-d3oq-ef3LW

    Feel free to ping me on Slack.

    I hope it helps you.

    Thanks,

    Marked as helpful
    1
  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello @henry-dura, It's really looking great.

    You can use background-position to adjust the position of the background image like background-position: 0 -150%, 100% 150%; these values should be in the same order that u have given in the background-image means before comma values will be applied to the top background since you added the URL first for this and after the comma will be applied to the bottom image.

    parameters are like x y, x y The top left corner of the screen identifies as 0 0 and the bottom right corner as 100% 100% So you can adjust these to get the requirements.

    For more clarifications check out the below link: https://www.w3schools.com/cssref/pr_background-position.asp

    I hope it helps you, Thanks

    Marked as helpful
    0
  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello @Mikssxed, It's really looking great. Just a little suggestion In the media query make the width fixed not a percentage of the available screen like what you have done right now width:90%. When u start decreasing the width your text also started moving rather what we can do is make the main tag with fixed width so that the interior component will not be affected when the width of the screen is decreasing. My suggestion will be to add one more media query so that you will have 3 breakpoints in the website - desktop, tablet, and mobile and adjust the size respectively.

    Also, make sure that whenever you use img tag also add its alt name so that if src image not available alt name can be shown.

    I hope it helps you, Thanks

    Marked as helpful
    1
  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello David, It's looking great just a little suggestion.

    Since you are using flex you can easily center your card using flex properties rather than using margin. You can refer to the below link: https://www.w3schools.com/css/css3_flexbox_container.asp

    Make sure adding min-height : 100vh as it will make your div have height of the 100% of the view height so that when you use align-content:center it will be in center of 100vh otherwise height will be adjusted to the max height of the content inside div which will not be in the center of screen.

    I hope its help you.

    Thanks, Deepanshu

    Marked as helpful
    1
  • Toprak•60
    @toprakunalSubmitted about 2 years ago

    If you give some FeedBack you'll really make me happy. I've tried my best but for getting better i need your FeedBack.

    CSS flexbox, media queries

    2
    Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello @toprakunal It's really good but I have a little suggestion.

    1. The approach you've used to center this card vertically is not the best way. Since you have used flex in the container class as well as align-content : center then add on this min-height : 100vh it will always make ur card always center for the container so you no need to give margin-top in card class like this

    .container{ min-height: 100vh; margin: 0; text-align: center; display: flex; align-items: center; justify-content: center; )

    It will give you much more control over your card rather than using a margin-top.

    1. For changing image when screen size changes from desktop to mobile you can picture tag pls refer to below link:https://www.w3schools.com/html/html_images_picture.asp

    I hope it helps you, Thanks

    Marked as helpful
    1
  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    HI @ASWIN CHANDRAN, This looks great just a little suggestion which I got from lucas. The approach you've used to center this card vertically is not the best way, because using margins you don't have much control over the component when it scales. My suggestion is that you do this alignment with flexbox using the body as a reference for the container.The first thing you need to do is to remove the margins used to align it, then apply min-height: 100vh to make the body height size becomes 100% of the screen height, this way you make sure that whatever the situation the child element (the container) align the body with display: flex and align-items: center / justify-items: center.

    body { min-height: 100vh; margin: 0; background-color: hsl(212, 45%, 89%); text-align: center; display: flex; align-items: center; justify-content: center; }

    Hope this helps you

    Marked as helpful
    1
  • Samed•10
    @SamedLepirSubmitted about 2 years ago

    My first project here, any feedback is welcome! :)

    QR Code Challange

    4
    Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Hello Samed, Try with removing the forward slash from image URL like this - background-image: url("images/image-qr-code.png");

    I hope this works. Thanks

    0
  • Deepanshu Gupta•590
    @Deepanshu-5288Posted about 2 years ago

    Its looking good just one suggestion : Add both images to the page and display them per the screen size. Like you can use display property on mobile image and make it none and using @media when the screen size is mobile make the desktop image display to none and mobile image display: block Check the below link : https://stackoverflow.com/questions/10439382/change-img-src-in-responsive-designs

    I hope this helps you .

    0