@damigand
@ViniciusG1m3n3s
Submitted
What are you most proud of, and what would you do differently next time?
Super proud of myself for nailing this design! I’m taking a course and just got to the Flexbox module, so I tried using it for this challenge. I’m sure there’s room for improvement, so if you have any tips, send them my way! Thanks a bunch for the community support!
@damigand
Posted
Good job doing this design! The course you're taking seems to be working! There's three things I'd change about your design:
CARD
The card element could have simply been a flex container. You can then use flex-direction: column to align all items from top to bottom, and use gap to separate them from one another instead of having to use margin-top or margin-bottom in some of the child elements.
IMAGE
I can see that you're doing the trick by using background-image on a div. However, this would be much simpler if you simply replaced the div element for the img one. The styling would be almost (if not) the exact same. However, there's no real difference between the two, it's just something that would work better for accessibility than anything else.
PROFILE
The profile section is done using a table. To me, this seems a bit overkill, specially after reading that you're just learning flexbox, which could have been perfect for this section. There's two elements: img and p. You can completely remove the table and its corresponding elements and put both the image and the text inside a div element. Then, you can style it as follows:
.profile {
margin: 0;
display: flex;
align-items: center;
}
This would place both elements in the same row, and align them to the center vertically.
Keep up the good work and happy coding!
Marked as helpful
@dylans0ng
Submitted
What are you most proud of, and what would you do differently next time?
I'm proud of being able to figure out how to center all the items on the page, including the container and the text box inside the container.
What challenges did you encounter, and how did you overcome them?
I struggled getting the text to be in the bottom of the container, but I realized I could use the text-align property.
What specific areas of your project would you like help with?
I'm wondering if my positioning of all the elements are looking good.
@damigand
Posted
Hey there! Just letting you that you probably forgot to upload the QR image file in your repository and live server, so the QR image doesn't actually appear
@nielsfechtel
Submitted
What challenges did you encounter, and how did you overcome them?
Had some trouble with Grid, maybe should have used Flexbox. Check the readme for details.
@damigand
Posted
I think you have the wrong idea about handling your elements with grid. You need to establish the amount of rows and columns in your grid element using the properties grid-template-rows and grid-template-columns. For this project, looking at the challenge screenshot, it makes sense for our grid to be two rows tall and four columns wide. The first element (Daniel) takes two columns of the first row. The second element (Jonathan) takes one column of the first row. And so on with the rest of them. To make each individual card element take its corresponding space, we can make use of the grid-row and grid-column properties in the card element itself. For example, we have the given grid parent:
.grid {
display: grid;
grid-template-rows: 350px 350px */ two rows of 350px each */
grid-template-columns: repeat(4, 1fr) /* four columns, each column taking the same space as the rest */
}
Then you'll have daniel's card like this:
#daniel {
grid-row: 1 / 2;
grid-column: 1 / 3;
}
To understand why we're using these values, we need to go through how grid works. If a grid layout has 2 rows, then our grid has 3 lines dividing it from top to bottom (you can see these lines by enabling the grid layout in your browser elements tab). So, if we want our #daniel element to take space only in our first row, we need to tell him from which line to which he can extend. The first row goes from line 1 to 2, and the second row goes from line 2 to 3, so the property is grid-row: 1 / 2;. We apply the same logic for columns. If our grid layout has 4 columns, there are 5 lines dividing them. First column goes from line 1 to 2, second column goes from line 2 to 3, and so on. Since we want #daniel to take the first two columns worth of space, our value is grid-column: 1 / 3. Keep in mind that the layout will look really odd until you're done establishing the grid-row and grid-column properties for the rest of the card elements.
To rearrange the spacing, you can simply use a media query and change all of these properties to reorganize the layout of the page to make it fit better in different screen sizes. This includes changing the grid rows and columns, as well as the grid-row and grid-column properties in the cards.
If my explanation was helpful, please consider marking it as such. If I didn't clear it up enough for you, feel free to respond and I'll get back to you as soon as possible.
These things take a bit of trial and error at first, so don't be discouraged. Keep working!
Marked as helpful
@Lucknagh
Submitted
What specific areas of your project would you like help with?
I had a little difficulty getting the grid and responsiveness right, if anyone could give me some tips on what I can do better in both the grid and responsiveness, I would be grateful.
@damigand
Posted
First of all, it looks great! Incredible job so far, love to see it! I'm going to break down a few things you can do to make your code a bit better.
Responsiveness
The reason why your design lacks a little bit of responsiveness in other screen widths is because there are a lot of explicit widths established. I recommend using the combination of width: 100% along with max-width in an element. At first, the element will take as much space to fill the max-width established, and the width: 100% ensures that the content will not overflow when the screen becomes smaller. Applying it to your code, you could change the grid elemement to the following:
.grid {
width: 100%;
max-width: 1200px;
}
You'll notice that, with this change alone, the layout seems a bit off. This is because you also have to change the sizing of each individual card element to follow the same criteria. So you'll change card1 all the way up to card6 to have the following:
width: 100%;
You will notice a change in how the widths interact with each other this way, specially when you make the screen width smaller. You can also apply this same technique to height and max-height to make sure the content doesn't overflow down the bottom either, or at least make sure it looks great if it ever does overflow, although in your project, I think squeezing down the font-sizes will do the trick for you, to make sure the heights are not overflowing your page.
CSS Grid
While your code is not exactly wrong, it does have its limitations when you make use of the "card1 card1 card2 card3" kind of approach. I personally recommend using grid-template-rows and grid-template-columns for the grid element instead. This allows you to have more control over how much space can a row or a column take. This, with the combination of grid-row and grid-column on the card elements, makes up for a very solid, responsive way of organizing your layout. While I could explain how you can apply these CSS properties, I think this little chat box is pretty limited, so I think you'll be better off doing your own research.
Media queries
The media query established in your CSS applies on all screen sizes under 900px. However, you limit every element to take only 250px at most. This results in the elements being a bit too small within the range of 400px to 900px screen sizes. To fix this, you can simply use the trick we learnt earlier. Change your media query code of the card elements to the following:
.card1, .card2, .card3, .card4, .card5, .card6 {
width: 100%;
}
Isn't that much better? Now the elements take all the width of the screen (you can use different percentages if you want a bit of margin on the sides) and they become smaller as the screen size shrinks. Keep in mind that, if you use a percentage that's under 100% for these card elements, you might need to align them to the center by using justify-content: center or align-items: center in the parent card element. You can also align one specific card to the center by using justify-self: center or align-self: center instead.
Don't be discouraged if things don't go your way while learning these little tricks. It only comes with time and practice, it takes some trial and error to learn in these cases. There are a lot of tutorials out there that explain these tricks better than I did, but if you found this response helpful, please consider marking it as such.
Have a wonderful time coding, and the best of luck!
Marked as helpful
@damigand
Posted
It's nearly pixel perfect, this is great! Personally though, I think the main element should be the .card itself. You can just use the CSS property justify-content: center in the body given that it's already a flexbox element. Also, I don't think the CSS query for smaller screens is necessary. You can combine width: 100% and max-width: 327px on the .card element to make it responsive without using the query. But these are just persnickety things though.
Love to see the work!
@oaved
Submitted
What are you most proud of, and what would you do differently next time?
I think I somewhat improved my semantic HTML. I still need to develop knowledge about, and ability to write, accessible HTML.
What challenges did you encounter, and how did you overcome them?
No real challenges really
What specific areas of your project would you like help with?
I would like some help with my semantic structure of the HTML, some tips on how to make my HTML more accessible, the structuring of my CSS and my use of git and github. I know my commits wasn't the best here but maybe some feedback on my messages or something along those lines. Any feedback is welcome!
@damigand
Posted
LANDMARKS
The only thing that could make your semantic better is the use of landmarks. Landmarks are used to allow screen readers to know where the content of your page is, and gives them the ability to jump between sections on your page. You can check more information about landmarks here.
If you want to give a specific element (that isn't considered a landmark) the ability to work as a landmark, you can use the attribute role in the element's tag.
YOUR PROJECT
To make your semantic a little bit better on this project, I think the .card div could be a main element instead. Alternatively, if you really want to keep the div element, I suggest using the attribute role="main" for blind users to benefit from that accessibility.
In the future, if you also want to add the .attribution section (that I see commented on the source code of your project) or any other content related to copyright that is outside your main or role="main" element, I suggest using the attribute role="contentinfo".
If the extra content outside your main is not copyright related, I recommend doing your own research on other kinds of role options you can choose for your elements.
Marked as helpful
@Bamo-D-Abdallah
Submitted
What are you most proud of, and what would you do differently next time?
I used fetch() and created some of the elements manually through JavaScript DOM API.
What specific areas of your project would you like help with?
I think I over complicated my code, any overall code tip would be highly appreciated.
I couldn't make perfect circle unless I set both height and width which AFAIK it is not good idea to use height but I don't know if this is the way to do it.
Also I couldn't make the linear gradient match the one in the image.
@damigand
Posted
I don't see any reason as to why height is not a good idea to use. I've never heard that using height is bad, but if you want to make a perfect circle, you can set the width property to your desired value, use border-radius for the roundness and then add a property called aspect-ratio: 1 / 1. This ensures that the aspect ratio between the height and the width remains as 1 / 1, therefore making it a square / circle. But again, I don't see why you wouldn't want to use the height property instead.
@jidanrestu
Submitted
@damigand
Posted
The dimensions of the solution are a bit smaller than the dimensions of the challenge. I suggest adding the attribution section outside of the main section because it messes up with the solution a little bit. Next time, try using the next code in your attribution element:
.attribution {
position: absolute;
bottom: 0;
left: 0;
right: 0;
}
This will help you align your attribution element at the bottom of the page. Make sure it's outside of any main element, at the very end of the body. The rest is pretty solid, it looks really similar. Keep up the good work!
@Wisdomjn
Submitted
@damigand
Posted
Desktop version
The .card-container element has a margin: 20px that could be removed. Other than that, there's no major differences to highlight.
Mobile version
This one is a little bit more complicated. The design is lost both on the image and the "Learning" button, causing a scroll-bar at the bottom of the card. To fix this weird behaviour, I suggest adding the align-items: flex-start; property to the .card-container div. This will fix the button extending all the way across the card. To fix the image overflowing, you can simply add the following code to the @media query that you have:
#img {
height: 200px;
width: 100%;
object-fit: cover;
}
After that, you can play around to make the margin and padding of the image fit better within the card.
General advice
For this challenge in particular, you're given a Figma design. I recommend using it, because overall this solution lacks most of the dimensions that were given to follow. For the "Learning" button, I suggest using the HTML span element instead. The font that the challenge required is missing, but it was indicated in both the Figma design and the style-guide.md file. I noticed the use of <b> to make the font bolder; while this is not a wrong approach, both design files also establish that four (4) font prefixes had to be used. I recommend looking into the CSS font shortcut and all of the CSS attributes that it includes, as well as recommending the use of :root variables for them. I suggest removing a few files from your github repository, like the style-guide.md, the readme-template.md and the readme.md if you don't plan on using them, as well as the design folder. Last but not least, the use of landmarks is encouraged for further accessibility.
While I did criticize a lot, I do think the design is pretty well done. Fixing these persnickety behaviours is a headache at first, but it's a skill that will develop over time. Keep it up!
What challenges did you encounter, and how did you overcome them?
I did not encounter any challenges.
What specific areas of your project would you like help with?
@damigand
Posted
In my subjective opinion, there's no need for a wrapper div around the image. Using width: 100% and max-width: 288px;on the image element should be enough to fit within the flexbox div without a parent wrapper. If you want to ensure that the image remains a square, consider using aspect-ratio: 1 / 1; as well.
Keep in mind though, I don't think there's any difference between just the image element or an image within a wrapper element, it simply takes less time and effort if you use css properties on the image directly.
Your solutions looks pretty much identical, amazing job!
@farahsamsi
Submitted
@damigand
Posted
The div container for the image is unnecesary. I'm assuming the following code is used to make sure the image is properly aligned and contained within its flexbox parent:
CSS
.img-container{
width: 256px;
height: 256px;
margin-top: 10px;
display: flex;
flex-direction: column;
}
img{
/* Image */
border-radius: 20px;
}
HTML
<div class="img-container">
<img src="images/image-qr-code.png" alt="">
</div>
However, this code can simply be replaced by the following:
CSS
img {
width: 100%;
max-width: 256px;
margin-top: 10px;
}
HTML
<img src="images/image-qr-code.png" alt="">
By using width: 100%; we ensure that the image's width will be, at most, 100% of its parent, eliminating the need of a wrapper div. In addition, we can also ensure that the image remains as a square with the property aspect-ratio: 1 / 1;, but in this case, it's not necessary.
I also suggest you fill the alt="" property on images. In this case, you could use alt="qr code for Frontend Mentor".
Overall this looks very solid. On wider screen sizes, the background color doesn't fully extend both in height and width, leaving unwanted white areas. This can be fixed by adading width: 100%; and height: 100%; to both html and body elements. The padding around the QR code image is a bit too wide, but other than that, the project looks very, very similar.
Good job!
Marked as helpful