cucumeris• 160
@cucumerisGIT
Submitted
P
biwwabong
@biwwabong
All comments
To be honest this looks great, a much better job than I did. The only thig I would suggest looking into is BEM
Using a naming convention like BEM (Block, Element, Modifier) is beneficial because it makes your CSS more organized, readable, and easier to maintain. BEM helps you clearly understand the purpose of each class, avoid naming conflicts, and create reusable components, leading to a more scalable codebase. For more details BEM
This is a comment left to me by a more senior coder and is worth looking into
Marked as helpful
0- Kurakula Vinay Kumar• 130
@vinaykumar91
Submitted
Great project! It looks really nice and closely matches the original design.
You've done an excellent job with the layout and structure, and the project has come together well overall. However, there are a few areas for improvement that can help make your code more maintainable and efficient:
- Move Inline Styles to the External CSS File It’s always a good practice to keep your styles in the external CSS file for easier maintenance and consistency. For example, instead of using inline styles like this:
You can move these styles to your CSS file:
.description { margin-top: 1em; font-weight: 200; }
<p class="description">Our Artificial Intelligence powered tools use millions of project data points to ensure that your project is successful</p>- Use Semantic Elements Like <article> Instead of <div> You've used <div> for your cards, but since each card is a distinct, self-contained piece of content, using <article> would be more semantically correct. For example, instead of:
You can structure it like this:
<article class="card-supervisor"> <h3>Supervisor</h3> <p>Monitors activity to identify project roadblocks</p> <img src="./images/icon-supervisor.svg" alt="Supervisor icon"/> </article>This not only improves readability but also enhances accessibility and semantic structure.
-
Improve Class Naming for Better Readability Using class names like item1, item2, etc., can make it harder to understand the purpose of each element. More descriptive names like card-supervisor or card-team-builder make your code easier to read and maintain.
-
Optimize Google Font Loading You’re currently importing all the font weights from Google Fonts, which can slow down the page load. It’s better to only import the font weights that are actually needed. For example, instead of this:
You can simplify it by only loading the necessary weights:
<link href="https://fonts.googleapis.com/css2?family=Poppins:wght@200;400;600&display=swap" rel="stylesheet">This will improve performance and benefit SEO by reducing the page load time.
Overall, you've done a fantastic job! Just a few small adjustments will take your project to the next level. Keep up the great work!
0 The design is spot on! However, one suggestion is to use semantic tags in your HTML for better structure and accessibility.
Also, there’s some redundancy in the button markup:
html Copy code
<div class="product-buy"> <button> <img src="images/icon-cart.svg" alt="icon cart white"> <p>Add to Cart</p> </button> </div> In this case, providing an alt text for the cart icon is unnecessary and can confuse screen readers. Since the button already says "Add to Cart," that’s all a screen reader user needs to hear. As it stands, a screen reader might announce: “icon cart white, Add to Cart,” which can be confusing.To resolve this, either leave the alt attribute empty (alt=""), or better yet, use aria-hidden="true" to completely hide the icon from screen readers. This way, the screen reader will focus on the "Add to Cart" text only.
Here’s an example of how you can improve this:
html Copy code <button class="product-text-add-to-cart"> <img src="./images/icon-cart.svg" alt="" aria-hidden="true" /> <span>Add to Cart</span> </button> This approach ensures a better and more coherent experience for screen reader users.
Great job all in all! I hope these pointers help :)
0- vinirangel• 40
@vinirangel
Submitted
What are you most proud of, and what would you do differently next time?
I'm proud of designing a working mobile and desktop version. The mobile design looks good, but the desktop version could look better.
What specific areas of your project would you like help with?
I would like feedback on my html structure.
I could mention a few little things in the layout, but it really would be nitpicking. The only change I can see from a brief look is that I would of used a table for the Nutrition section at the bottom instead of divs
<section class="nutritional-value"> <h4>Nutrition</h4> <p> The table below shows nutritional values per serving without the additional fillings. </p><table> <tr> <td data-cell="nutrients">Calories</td> <td data-cell="nutrient-value">277kcal</td> </tr> <tr> <td data-cell="nutrients">Carbs</td> <td data-cell="nutrient-value">0g</td> </tr> <tr> <td data-cell="nutrients">Protein</td> <td data-cell="nutrient-value">20g</td> </tr> <tr> <td data-cell="nutrients">Fat</td> <td data-cell="nutrient-value">22g</td> </tr> </table> </section>all in all great job :)
1 - feyzanursener• 30
@feyzanrsnr
Submitted
Nice work on completing the Social Links Profile challenge! Here are a few things I think you did well, along with some suggestions for improvement:
What You Did Well:
Clear HTML Structure: Your HTML is organized and easy to follow, which makes the code readable and maintainable. Grouping elements like the profile image and name under a header and using ul for the social links is a good approach.
Effective Centering with Flexbox: You've effectively used Flexbox to center the main content, ensuring the profile card is always centered on the page. This makes the layout look great!
Suggestions for Improvement:
Improve Responsiveness on Smaller Screens: Consider using media queries to adjust the main container's width for smaller screens. This would make the profile card more adaptable and improve the user experience on mobile devices.
Add Links to Social Media Items: To enhance interactivity, consider wrapping each <li> element in an <a> tag with a corresponding href attribute. This would make the social media items clickable, aligning with their intended functionality.
Semantic HTML Elements: To further improve the semantic structure of your HTML, consider using a nav tag instead of article around the social links. Since these are navigation links to different profiles, a nav element would better convey their purpose to assistive technologies.
Using External CSS: While inline styles are fine for small projects, consider moving your CSS to an external stylesheet (e.g., style.css). This would help keep your HTML cleaner and make your project more maintainable and scalable for future changes.
Overall, great job! Just a few small tweaks would make your project even better. Keep up the good work!
Marked as helpful
0 - EdwardCrnelius• 10
@EdwardCrnelius
Submitted
I noticed two main areas for improvement in your HTML structure. First, you're using <div> tags instead of semantic HTML tags. It's generally better to use semantic tags, like <header>, <footer>, <main>, <article>, and <section>. These tags not only enhance SEO by making your content more meaningful to search engines but also improve accessibility for screen readers, allowing users with disabilities to navigate your content more effectively.
For example:
<main> <article class="container"> <figure> <img class="blog-image" src="assets/images/illustration-article.svg" alt="Image description" width="336" height="200" /> </figure>
</article> </main><a href="#" class="card-btn">Learning</a> <time datetime="2024-09-01">Published: 01 September 2024</time> <div> <h1>HTML & CSS foundations</h1> <p> These languages are the backbone of every website, defining structure, content, and presentation. </p> </div> <footer> <figure> <img src="assets/images/image-avatar.webp" alt="Greg Hooper" /> <figcaption>Greg Hooper</figcaption> </figure> </footer>Regarding the <article> tag, its use here is somewhat debatable since this is a single webpage. While it's not strictly necessary for a single item, it might be useful if you envision this card as part of a series of multiple blog post previews on a page. In a real-world scenario, using <article> would make more sense if there were multiple cards representing different blog posts.
The second point is your use of IDs for styling in your CSS. It’s uncommon to use IDs for styling in modern web development because IDs are unique and can’t be reused, which limits flexibility. Instead, classes are typically used because they are reusable and allow for more scalable and maintainable code.
If you were to add another card to the page, using IDs would require creating a whole new set of unique IDs for each element in the new card and duplicating your CSS. This quickly becomes unmanageable, especially if you have multiple cards. For example, having six cards would mean creating six different sets of IDs, resulting in a cluttered stylesheet with repetitive CSS rules.
By using classes instead, you can define a single set of styles in your CSS and apply them to any number of cards. This approach is much more efficient because you only need to write your CSS once and then reuse the same classes across all similar elements, significantly reducing repetition and improving maintainability.
On a positive note, you've done a fantastic job with the overall structure of your HTML and CSS! It's clear that you have a solid understanding of web development fundamentals, and your approach to organizing content shows good logical thinking. With just a few tweaks to use more semantic HTML tags and rely on reusable classes in CSS, your code will be even more efficient and accessible.
Keep up the great work! Each challenge like this helps you sharpen your skills, and you're definitely on the right track. Well done!
0 - Muhammad Saud• 10
@MuhammadSaud60
Submitted
Th html is only partially semantic.
Semantic elements:
<html>: This is a semantic element that defines the root of an HTML document. <head>: This is a semantic element that defines the document's metadata. <title>: This is a semantic element that specifies the document's title. <img>: While img itself isn't inherently semantic, the alt attribute provides alternative text for the image, making it accessible. <p>: This is a semantic element that defines a paragraph of text. <a>: This is a semantic element that defines a hyperlink.Non-semantic elements:
<div>: The div element is a generic container and doesn't convey any specific meaning.Possible changes:
In your code, you could potentially use more semantic elements like <section> or <article> depending on the intended structure.
Here's how you can improve the semantic nature of the code:
Replace the outer div with a more specific element. If it represents a card-like structure, you could use <section class="card">. If the content within the card represents a call to action, you could consider using <article class="card">.
By using more semantic elements, your code becomes clearer to both humans and machines (like screen readers). It also enhances the maintainability of the code in the long run.
0
