@brinda-an
Martin Kamír
@Martin-K-KamirAll comments
- @Martin-K-Kamir
Hey, Brinda! Nice work on the card.
Couple things i found to improve:
-
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.
-
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.
-
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.
-
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
-
- @artimys@Martin-K-Kamir
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 - @AditNovadianto@Martin-K-Kamir
Nice work @AditNovadianto!
Few things to point out.
-
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.
-
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.
-
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 :)
-
- @margsoftbf@Martin-K-Kamir
Hey, i just check your code little bit.
Couple things to look after.
-
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.
-
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.
-
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.
-
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 -
- @naathcs@Martin-K-Kamir
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 - @radanovicnikola93@Martin-K-Kamir
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.
- @catherineisonline@Martin-K-Kamir
Nice work Catherine!
Just few things i would point out.
- The page is not very responsive on couple devices. The text is overflowing and the layout is weirdly positioned not very centered.
- The social icons on the bottom are not very rounded. I would suggest on .social-media .fa-brands delete padding add flex center it out and width height 1.5rem, also the instagram icon is smaller then other icons so maybe scale it up little bit but that is small detail
- Also i would fix the scrolling behavior on full size desktop since there is no more content below. The image makes the page scrollable.
But still the website looks amazing and i love your work!
- @catherineisonline@Martin-K-Kamir
Wow, nice work! There is no much to say. The code looks good. Just wondering why set html font-size 13px?
Maybe in next project try to use SASS/SCSS it's just the best thing to do css.
Marked as helpful - @gerbelli27
- @kmeganiz@Martin-K-Kamir
Like @grace-snow said. Doing bootstrap without not knowing css is very bad practice and won't get you far. In the end you still need to learn css. It's very bad pratice to learning any libaries before the actual language.
Also if you are struggling and copying code from other people. You will not learn anything. At start you should do some courses on udemy or atleast on youtube so you can start write your code by yourself. Which is best practice.
Marked as helpful - @catherineisonline@Martin-K-Kamir
Hi, i really like your solution. If i could suggest you few things from my experience, what i would tweak a little.
-
Since you are using rems. Set to html tag in css font-size: 62.5%. So 1 rem is 10px. As default 1 rem is 16px. So setting it this way will help you calculate how much is 1 rem. For example 32.1rem is 321px :D
-
As @ToHX said set body (or make some container class) min-height: 100vh and center it with flexbox. So you can simplify your code also like @BenjaDotMin commented.
-
For the box-sizing: border-box dont use it on body tag but like this -> html box-sizing: border-box; ... and ... *, *:before, *:after { box-sizing: inherit }. It is better explained here -> https://css-tricks.com/box-sizing/
Marked as helpful -
