bima suci• 35
@bimasuci18
Submitted
anyone can help me? All feedback is welcomed so I can do better.
Fahim Mahmud
@FahimMahmudJoy
All comments
- Fahim Mahmud• 165
@FahimMahmudJoy
Posted
Hi there, great work! A few suggestions:
- You have way too many divs in your html and without any comment, it's really hard to track which one is which. You can easily get rid of those surrounding <h1>, <p> etc. since headings and paragraphs are block level by default and don't need additional div around them to style.
- I see that you have used somewhat bootstrap type approach for the stats with a row, col type. However, declaring them as an unordered list makes more semantic sense I believe.
- For the Google fonts, use link stylesheet in the html file rather than import because it's faster.
- You can center the card with body {display: flex; min-height: 100vh; justify-content: center; align-items: center}. Keep this in mind, it has saved me a ton of headache to get things centered.
- Have a look at the typography. Your paragraph font-size is quite small.
- You can make the card a bit smaller to match the design.
- And regarding media queries, you can Google or YouTube a bit too find out more. It'll make your site look great on all devices. Never give up and as always, happy coding!
Marked as helpful
0 - Harsh Porwal• 30
@mysteriouspla
Submitted
Please provide feedback regarding my work. Also please suggest how to use media queries. I got bit confused in it. Thank you !!!!!1
Fahim Mahmud• 165@FahimMahmudJoy
Posted
Hi Harsh, First of all, great job on the project. As a fellow mate who is also learning, I have a few suggestions for you. Hope they help
- Looking at your html, you could easily get rid of the extra divs (heading and content) because h1 and p are by default block level elements. So you could style them even without using divs. It would make the html a bit cleaner and easier to track and work with.
- For the stats, you may want to declare them as unordered list since it makes more sense from semantic point of view. You could then put the value (10k+ etc.) and label (companies etc.) in separate spans and style them accordingly. Also, display them as block so that they appear on separate lines according to the design.
- To center any component in the screen, use this always. body {display:flex; min-height:100vh, justify-content:center; align-items:center} It always works and has saved me a ton of headache.
- Avoid setting fixed height on any component at all costs. Fixed height is a dangerous thing because it might result in overflow if the content doesn't fit within it.
- To get the image to the required color, use that color as background color on the card-header section. Then, use mix-blend-mode: multiply on the image. You may need to change the opacity a bit to get the exact match. However, it gets you really close to the design.
- Media queries is a huge topic regarding responsive sites and I'm also learning. However, you are most likely to use @media (max-width: ....px){ design...} or @media (min-width: ....px) { design...}. The max-width means the design inside will be applicable from 0 upto that width. On the other hand, the min-width means that the design inside will be applicable from that min-width upto anything above. So, as you can see, they are for desktop first and mobile first design respectively. Going mobile first approach can help you write fewer lines of code. There are a lot more to media queries. Just Google, YouTube, there are plenty of great resources out there. Just remember, never lose hope, never give up. All the best. Happy coding
0 - Alexis Enriquez• 260
@alexiscenriquez
Submitted
Feedback appreciated
Fahim Mahmud• 165@FahimMahmudJoy
Posted
Hi Alexis, You may want to make the card look a bit more square in the large screen. On small screen, it looks fine.
0 - Yunus Alif• 150
@frontendnus
Submitted
Why i can't use background-image:
linear-gradient( rgba(var.$soft-violet, 0.5) ), url(url image);to overlay color on the image? I have to use absolute position to display colors and images.The responsive image is very trick for me. Any feedback for me?
Fahim Mahmud• 165@FahimMahmudJoy
Posted
- I don't know about sass. However, what I did was I simply put the image inside a div, set the background-color of that div to the required color. Then, I used mix-blend-mode: multiply on that image, and it gave me the exact color.
- And to position the card exactly at the center, you could set a min-height of 100vh to the body and display it as flex. Then using justify-content and align-items to center, the card will be centered in no time. Hope that helps a bit.
1
