Bryan• 70
@BataaB
Submitted
What are you most proud of, and what would you do differently next time?
There were still some styles I couldn't get right. Like how the second of line of the list element keep starting from under the bullet point.
Marcos Valenzuela
@MarcosAvg
All comments
@MarcosAvg
Posted
Hi, good job with your solution, just a tip.
- You can assign a
max-widthto your container card so that the responsiveness starts to work at a certain width container and not at the resolution of the device. - It would also be correct to use the HTML in a more semantic way, it will help to the later reading, but it is still very well structured.
Keep learning, good day!!
0- You can assign a
- Tamás Kórizs• 30
@korizst
Submitted
What are you most proud of, and what would you do differently next time?
I have exercised Flexbox again. This time I was more familiar with this topic but I would like to do more projects to practice it.
What challenges did you encounter, and how did you overcome them?
Using Flexbox tools appropriate is yet a challenge. I have read about it on CSS-Tricks. There is a very good description of Flexbox.
What specific areas of your project would you like help with?
For now, I don't need any specific help. I'm getting more and more familiar with HTML and CSS.
@MarcosAvg
Posted
- Try to use a semantic HTML, structured in classes, in larger projects it will be beneficial.
- You can learn to structure your styles with custom propierties: CSS Custom Propierties
- When using a custom font make sure you use a safe font like san-serif.
- It would be advisable to use CSS methodologies (like BEM, SMACCS) to structure your code: Using CSS Methodologies
Marked as helpful
0 What are you most proud of, and what would you do differently next time?
This was fun and it felt pretty fast.
What challenges did you encounter, and how did you overcome them?
I'm unsure how to name the different font sizes.
I left p/16px/1rem as the body text. I used h2 for the card title on purpose, thinking the h1 would be on top of the page.
What specific areas of your project would you like help with?
Should I handle the rest (the tag/chip with Learning, the date and the name) as h3 and h4, or as different p classes? I'm sure it depends on the project, but is there best practices on how to approach it if it's a small component, like this card?
@MarcosAvg
Posted
Hi, I have reviewed your code and I have seen some things that could be improved.
First of all you don't have a
font-family:declared, this is included in the assets and can be declared as a custom property in the following way:@font-face { font-family: "Figtree"; src: url(assets/fonts/Figtree-VariableFont_wght.ttf) format("ttf"), url(assets/fonts/Figtree-Italic-VariableFont_wght.ttf) format("ttf"); } --primary-font: "Figtree", sans-serif;It is also important to use a safe font such as sans-serif as an alternative. Your container displays fixed at low resolutions, which causes horizontal scrolling, I saw that it may be because the image is not defined with a
max-width:and when having the viewport in a less resolution it does not scale, this could be done introducing the image in a container and giving it the propertymax-width:since the container if it can scale in resolution also your image:<div class="img"> <img src="./assets/images/illustration-article.svg" alt="decorative image" class="decorative-image"> </div>.decorative_image{ max-width: 100%; }In the author, if you notice the image is not proportional to the design reference, as in the previous one, it can also be inside a container:
<div class="avatar"> <div class="avatar-image"> <img src="./assets/images/image-avatar.webp" alt="Profile photo of Greg Hooper" class="avatar-photo" /> </div> <p class="tag-text">Greg Hooper</p> </div>.avatar-photo { max-width: 100%; }You can add the flex growth and reduction factor to achieve a result in which the text occupies, for example, one third of the container and the image, one fourth:
.avatar-image{ flex: 1; /*https://developer.mozilla.org/es/docs/Web/CSS/flex*/ } .avatar-photo { max-width: 100%; } .tag-text { font-size: 0.875rem; font-weight: 800; color: var(--color-gray-950); flex: 3; }Marked as helpful
0- IeoB• 60
@IeoB
Submitted
@MarcosAvg
Posted
Hi!, I have some suggestions for your code.
I have checked and you could use another method to center the component on page, currently you use:
.card{ position: relative; top: 50%; left: 50%; transform: translate(-50%, 50%); }And this can make it overlap other HTML elements like:
<div class="attribution"> Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. Coded by <a href="#">Your Name Here</a>. </div>you could simplify it by using margins, using the viewport as a measure and so all your element would be centered without overlapping any other html element.
margin: 25vh auto;Also your image and text at low resolutions will overflow the chart container since they have absolute sizes like:
.card{ max-height: 499px; } img { height: 288px; width: 288px; }You can use percentages for the responsiveness of your image within its container so that it does not overflow. As well as removing the max-heigth of the card so that it adapts to the content inside it.
.card{ max-width: 320px; } img { max-width: 100%; }I also recommend the use of relative units such as rem or em in the font-size as well as in other elements such as padding or margins.
/* 1rem = 16px */0
