Skip to content
OverviewSolutions
13
Comments
11

Martin Kamír

@Martin-K-KamirCzech Republick490 points

Hello, my name is Martin Kamír i am frontend developer at rondo.cz

Latest solutions

View more solutions

Latest comments

  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Hey, Brinda! Nice work on the card.

    Couple things i found to improve:

    1. I think it's not good using of having section tag for the img header. Also i wouldn't set border top left-right radius. I would just put overflow hidden to the container of the card so nothing is overflowing.

    2. You set to card-details justify-content and align-items these properties won't work since you didn't display flex or grid. I wouldn't use these properties in this situation. What i would do is display flex, flex-direction column and gap "some value". So each item in the container have same gap. In the design demo the title have bigger gap so i would set to the title margin-bottom to give it bigger space then other elements in that container.

    3. On each text element i found you declare a font-family. Just declare font-family on the body in css. Also you set everywhere padding-bottom to make spacing between elements i would use margin-bottom instead.

    4. On the button you have box-shadow in the design demo there is blurriness on that shadow. So you may forget declare third value for the box-shadow.

    Anyway you did nice work on the card i wish you very best and have happy coding

    0
  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Hey @artimys,

    cool idea about the function. To make it work, you need to use interpolation.

    Add this to your code and it will work.

    @function brightBlue($opacity: null) { @if ($opacity) { @return hsl(245, 75%, 52%, $opacity); } @else { @return hsl(245, 75%, 52%); } }

    :root { --bright-blue: #{brightBlue(0.5)}; }

    Marked as helpful
    1
  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Nice work @AditNovadianto!

    Few things to point out.

    1. The FAQ content disappears at viewport 820px and then comes back at. I could not find why in the devtools. I would need more time to look into it.

    2. When you get to 423px viewport the whole content goes off the screen. Not very friendly for mobile users. On that container you set width to 900px. Would be better if you changed it to max-width and added media query at max-width 500px changed the width value for something smaller like 300px.

    3. When you get to phone screen sizes the FAQ content is not very centered since you set margin-left: 100px. It is not good practice to center content with margins. Use flex instead.

    Anyway i think the solution is good just needs bit work. Wish you best luck in next projects :)

    0
  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Hey, i just check your code little bit.

    Couple things to look after.

    1. On the box class you do not need to use grid. What i would do is just set it to flex column and center it.

    2. The logo do not appear. You added bad pathway for the img. Also all the properties are not necessarily mostly the absolute position. Next time just add width to 100% and it will fill up the whole space in that box.

    3. The mobile__bg and desktop__bg is not correct way how to add that background image. For example add to body the background color and then background image with url to that image.

    4. Remember the buttons do not inherit from the body font-family.

    Anyway if this is your first project i think it is pretty good job.

    Marked as helpful
    0
  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Hey Nathalia, nice work! Everythings looks amazing and nicely done!

    Only what i would fix is the card that is not very centered for smaller screens.

    Also maybe for your next project try to use rem units not px.

    Marked as helpful
    1
  • Martin Kamír•490
    @Martin-K-KamirPosted about 3 years ago

    Hey nice work!

    The website looks good but not very finished. The spacing on texts are big on larger screens since you are using flex space-between or space-evenly. Use gap instead or margins. There is missing content. On smaller screen sizes the padding on main div's is big. Use media query and make the padding smaller. Also texts are not centerd on mobile screens. Also i would lower the header's hight mainly on phone screens.

    The website looks good just give it some work and it will be perfect.

    0
View more comments