@Mohammad-Moneer
@codesByJose
Submitted
What are you most proud of, and what would you do differently next time?
made the required adjustments!.. this was one difficult task for me, but im glad it pulled through.. pls do leave your thoughts concerning this
@Mohammad-Moneer
Posted
Hello there, You made a good solution 👍 Your code is well-structured and easy to read.
Suggestions for Improvement:
The use of HTML5 semantic elements (<section>, <h1>, <h2>, <h3>, <ul>, <ol>, <table>) is good. However, wrapping the content inside multiple <div> elements instead of using more semantic tags (like <main>, <article>) can be improved for better readability and SEO. For example: You could replace the element <div class="container"> with <main class="container"> and also the element <div class="innerCont"> could be <article class="innerCont">
The width of the <div class="container"> should be larger to match the design.
The font size of the headings and body text should be adjusted to match the design.
The image dimensions are explicitly set with width="320px" and height="130px", which might distort the image. It's better to use CSS to set a width of 100% for responsive designs.
Overall, your project is well-structured and follows a clear layout. I hope you find these suggestions helpful.
Marked as helpful
@MsadafK
Submitted
@Mohammad-Moneer
Posted
Hello there, You made a good solution 👍 Your code is well-structured and easy to read.
Suggestions for Improvement:
CSS Variables:
Consider using CSS variables for colors and other repetitive values. This will make your code more maintainable and easier to update in the future. For example:
css
:root {
--green: hsl(75, 94%, 57%);
--white: hsl(0, 0%, 100%);
--grey: hsl(0, 0%, 20%);
--dark-grey: hsl(0, 0%, 12%);
--off-black: hsl(0, 0%, 8%);
}
body {
background-color: var(--off-black);
color: var(--white);
}
.card-container {
background-color: var(--dark-grey);
}
.location {
color: var(--green);
}
a {
color: var(--white);
background-color: var(--grey);
}
a:hover {
background-color: var(--green);
color: var(--grey);
}
Again, great job on your project! I hope you find this suggestion helpful.
Marked as helpful
@kawsaruddin450
Submitted
What are you most proud of, and what would you do differently next time?
I think I have sorted out the challenge and divided it part by part, that makes solving the problem much easier. I'll try the responsiveness differently next time.
What challenges did you encounter, and how did you overcome them?
I had some challenge with readme file in github repo. searched youtube, watched a video tutorial and the problem was solved, as simple as that!
What specific areas of your project would you like help with?
N/A
@Mohammad-Moneer
Posted
Hello there, You made a good solution 👍
Your HTML structure is clear and well-organized. However, I noticed that the img elements do not have alternative text. Including descriptive alt text is important for accessibility, as it helps users who rely on screen readers understand the content and purpose of the images.
in my solution I have provided an alternative text as follows:
HTML:
<img decoding="async" src="images/illustration-article.svg" alt="Illustration Article">
Your CSS is well-structured and easy to read. I noticed that you applied the border-radius directly to the image element rather than its wrapper. While this works fine, applying the border-radius to the wrapper can sometimes offer more control, especially if you need to manage overflow or apply additional styling to the container. Here's an example of how you could adjust your CSS:
CSS: .blog-img { overflow: hidden; border-radius: 10px; }
.blog-img img { width: 100%; }
This approach ensures that any overflow from the image is hidden and that the border-radius is consistently applied, which can be particularly useful for maintaining design integrity across different browsers.
Again, great job on your project! I hope you find this suggestion helpful.
@Fraustiz
Submitted
What are you most proud of, and what would you do differently next time?
I wrote the code mobile first so it was easier to build the computer version after. I did a perfect job and would not change anything.
What challenges did you encounter, and how did you overcome them?
Bottom text was overlapping with the card so I lowered it.
What specific areas of your project would you like help with?
Commenting my code.
@Mohammad-Moneer
Posted
Hello there, You made a good solution 👍 I noticed one thing that could be a small enhancement to your code. Currently, you have the <img> element directly inside the .card container with a border-radius applied directly to the image. This approach works well, but in my solution I wrapped the image in a separate container (div) to ensure consistent application of the border radius and provide greater flexibility for future styling adjustments. This approach also improves the readability and maintainability of the code. Here's a small example of how you could modify your code:
HTML:
<main> <div class="card"> <div class="card-image"> <img src="./images/image-qr-code.png" alt="QR Code"> </div> <div class="card-text"> <h1>Improve your front-end skills by building projects</h1> <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div> </div> </main>CSS: .card-image { overflow: hidden; border-radius: 10px; }
.card img { width: 100%; }
Again, great job on your project! I hope you find this suggestion helpful.
Marked as helpful
