Skip to content
OverviewSolutions
25
Comments
12
Gilang Aditya
@madegilangaditya

All comments

  • Gilang Aditya•425
    @madegilangaditya

    Hi, Felipe great work on finishing this challenge. The solutions just look great. Just a little correction for me regarding the responsive view, in this class ._card_qel2f_21 which is the container of your advice generator rather than use width: 320px; better use max-width:320px so when the screen is below 320px, the container will not pass through the screen.

    Also, try learning mobile-first workflow standard for website design. You can check the reference here: https://www.w3schools.com/css/css_rwd_mediaqueries.asp

    Good job, keep it up 👍

  • Gilang Aditya•425
    @madegilangaditya

    Hi, congratulation for finish your challenge, thats really good from my side. Here are some suggestion for me that I found on your solutions.

    • I see you set the width and height for the card-centered and card-container class. Try to avoid that, better use padding or margin on it.
    • Use max-width and set the width to 100%, instead to set the width using px, it will help the responsiveness of your site for mobile view.
    • For the background image already good as I see, maybe you can try put it in the body.

    I think thats it from me hope can help.

  • Gilang Aditya•425
    @madegilangaditya

    Hi, congratulation for finishing the challenge, my suggestion for the dropdown menus, maybe try mix visibility and opacity on it instead using display, for the mobile can manipulate max-height and visibility. Hope can give you insight on it

  • Gilang Aditya•425
    @madegilangaditya

    Well done for finishing the challenge, regarding your issue about backround image, I think you need to wrap your container with div then you can add background image in the wrapper div. Here some source to style background image : https://developer.mozilla.org/en-US/docs/Web/CSS/background

  • Gilang Aditya•425
    @madegilangaditya

    Well done for completing this challenge man, just a couple things I can suggest you

    • Try reduce your margin for the card to nearly match the design. Also try add some padding in your left column text.
    • I don't know why you use two image to separate mobile and desktop view as I see there are no diference between them
    • Avoid use id selector for your CSS instead just use class for selector.
  • Gilang Aditya•425
    @madegilangaditya

    Good job for finishing your first challenge. There are some issue I found regarding your border radius not show in bigger screen on container class. Instead using width 80vw. Better use max-width: ...px because vw size is relative depending on your browser screen size, so that make your container is more bigger than the content itself when use display:flex that make border-radius not showing in bigger screen.

  • Gilang Aditya•425
    @madegilangaditya

    Nice job for finishing the challenge. Maybe try use display:grid instead of using transform:translate. Also the border top of every column need to be diferent color like the design.

  • Gilang Aditya•425
    @madegilangaditya

    Good job for your first challenge, Here are a couple issue that I see on your project.

    • Install google font by ctrl click on style-guide.md file then you need copy the css link from google font and put it into your index.html file
    • you need border-radius in your card class
    • The stats h2 color is different compare by design color
  • Gilang Aditya•425
    @madegilangaditya

    Nice work man..

  • Gilang Aditya•425
    @madegilangaditya

    Hi Vinay, Congratulations for finish this project, there an issue in your background image. you need background-repeat: no-repeat; so your image will not repeated.

  • Gilang Aditya•425
    @madegilangaditya

    Hi Moh, congratulations for finished the challenge. Here are some suggestions for you:

    • Use justify-content: center; in your flexbox to make your column align center.
    • add some padding in your button.
    • add margin top in your button so there is space between text and button
  • Gilang Aditya•425
    @madegilangaditya

    Hi, well done for your first challenge, I have little suggestion for your container border radius. Instead of put it on every div, better put in main class div and add overflow:hidden so your image will not overlap your main div border radius