Any sort of feedback is appreciated!
Any sort of feedback is appreciated!
Any feedback is appreciated!
Any sort of feedback is appreciated!
I think i did pretty well in this challenge, but i would still like some feedback on things i could improve on.
What challenges did you encounter, and how did you overcome them?None so far in this challenge just waiting for some feedback
What specific areas of your project would you like help with?Recommendations on my code too improve are very open of course
I don't see many issues and it's a nice solution. Here are couple of suggestion.
header is usually for content that repeats across multiple pages on a site. Usually it's positioned at the very top of a page and contains the the company/brand logo and the navbar. The content you have put inside the header is specific to this page. So i wouldn't use a header here. Also a h1 being inside the header doesn't sound right either. I would include the h1 and that p in the main and remove the header.padding-top you are using on the section called four-card-feature should actually be a margin. Generally padding is used to get the spacing between an element's edges and it's content. margin and gap is what you use to get the spacing between two sibling element. What you have done is not wrong, but i think it's a good mental model to have and follow.Looks nice, i have couple of suggestions.
width: 100vw for the body. Body is a block element, and by default block elements take full width of the screen.card. If some more text were added to the card, text will start to overflow. Because the card can't grow it's height past the fixed height you have defined. You don't need to specify a height at all. The browser will calculate the height based on the content inside the container and things like font sizes and padding etc.div and span doesn't have any meaning. They are good for layout and and styling purposes. The label and the date should be p elements. Same way the user's name should also be a p. It's fine to have text inside a span, but the span must be inside a meaningful element like a heading or a p.alt=""). But if you feel like it should have an alt text, it's fine.a element inside that heading. So in a real site, when the user clicks on it, they will be taken to the actual blog page. That is the purpose of this card component. Picture it like this, there will be multiple of these cards on a page. Clicking on each card, should direct the user to the relevant blog post. For that to happen you need to have an a element inside the card.Nice solution. I have couple of suggestions.
height for text containers. In your card__content class you are using a fixed height. If some more text were added to the children of that element, text will overflow. Because of the fixed height the container can't grow freely to fit all the text content. There's no need to specify height for text containers. The browser will calculate it for you based on the content inside the element, and stuff like font size, padding etc.height: 100vh on main. Use min-height: 100vh there, so your main can grow if needed.img tag, so that i can give it an alt text like "QR code leads to frontendmentor.io". Currently if a screen reader user comes across this component there is no way for them to know that there is a QR image present in this.But yeah, other than that, it's a nice solution.
The custom font configured in the project is not displaying correctly on the deployed site, even though it works fine in the development environment.
.css file when referencing the font file. That should fix your issue.height to your grid, which causes text to overflow when the screen gets smaller. You should never limit the height of containers that contain text.I was able to write some CSS! As someone with very strong background in backend development, cloud and devops, this is a nice change of pace.
I would definitely write CSS in more scalable way next time. I was aiming for the best replication of the design, so if I wanted to scale the design a bit, I would need to improve the solution (e.g. less specificity for texts, etc.)
What challenges did you encounter, and how did you overcome them?Reading design file was a bit of a hassle at first, reading Figma without DEV mode is clunky. I'm not gonna pay for Figma unless I actually use it for real project.
What specific areas of your project would you like help with?I think that further down the line, spacing, gaps and layout will be a tricky to do right without frustration.
Looks good. You have avoided lot of mistakes everyone does on this first challenge. Here are couple more improvements you can do.
h1. In a real site this card would be placed in a page along with some other content, so it's unlikely the card heading would be the h1. It would probably be a h2 depending on the context. But in the given scenario you can use a h1 for it.<main> element, wrapping the main content of the page. In this case your div with the class .container can be the <main>, instead of being a <div>. Some other landmark elements would be <header> and <footer>. Both are usually for content that repeats across multiple pages of the site. <header> usually contains the company logo and the navbar and is positioned at the very top of each page. <footer> is where the attribution stuff go and is positioned at the very bottom. So your attribution should actually be inside a <footer>. You can change the div with the class .attribution to be <footer>.I'm proud that I have started my challenege path and would love to learn most out of this. Next time I would use different approach to solve the same challenge. I have used plain CSS in this challenge. Next time I would prefer using Tailwind CSS.
What challenges did you encounter, and how did you overcome them?There were no challenges I faced, since this was a beginner level challenge.
What specific areas of your project would you like help with?I would like to know if my approach and design principles were good enough to be applied in real projects.
Couple of suggestions.
divs. divs are good for structuring things and help with layouts, but div itself is not a semantic element, so it doesn't really have any meaning.h1. Usually this entire card will be placed in a page along with other elements, so in that case this won't likely be the h1. The h1 would be some other element that explains the content of the entire page. But in the given scenario you can make it a h1.<p>.rem unit for font-sizes. px units won't scale with the browsers font settings, so if the user who visits your webpage have changed their browser default font sizes, they won't be reflected on your page. rem on the other hand respects the browsers font settings and change accordingly. So always use rem for font sizes.height. It will cause overflowing if the content inside the card changes. You don't have to specify the height of text containers. The browser will automatically calculate it for you based on the content inside the card, and other values like font sizes, paddings etc.width either. When a fixed width is used the card can't shrink past that value, even if the screen size is small and the card doesn't have any space to fit in. That again will cause overflow issues and break your site. Instead use a max-width on the card. So you can control how large the card is allowed to be, but at the same time the card has the freedom to shrink automatically when there is no room to fit in smaller screens.ids and classes when it comes to styling. Always use classes for styling. Mixing the two will make you run into specificity issues. You can use ids when linking two elements together. For example linking an input element with the label element, or when using aria-describedby.<main>, except the content in <header> and <footer>. So the <div> that you have given the id="card" should actually be a <main>. The attribution should ideally go inside the <footer> and should be outside of the <main>.position: absolute on the card. Also i don't see a reason for the .card-body div. You can directly use the .card div. Try to keep the HTML simple.height: 100vh to the body instead of giving it to that div. And make sure you use min-height and not height. Never limit the height of the body or any element that contains text.