4 Selection Card

Community feedback

• 

  RogerTito455•170

  @RogerTito455Posted 3 months ago

  Hey there, well done!

  My feedback:

  HTML:

  Semantic HTML: Consider using more semantic tags to improve accessibility and SEO. For instance, wrap the header section in a <header> tag, and the card container in a <section> tag. This helps search engines and assistive technologies understand the structure of your content better.

  Alt Text: The alt attributes for images are empty, which isn’t ideal. Provide descriptive alt text that conveys the purpose of the images. For example, "Supervisor icon" or "Icon representing Karma" would be good.

  Class Naming Conventions: While the class names like .Post1, .Post2, etc., work, consider using more descriptive names that indicate their role or position, such as .card-supervisor, .card-team-builder, etc. This can make your code more readable and maintainable.

  Avoid Inline Styles: The attribution section at the bottom uses inline styles. It’s better to move these styles into your external CSS file to keep the HTML clean and maintainable.

  CSS:

  CSS Reset: It’s good that you’re setting margin and padding to zero. Consider using a more comprehensive CSS reset or normalize file to ensure better cross-browser consistency.

  Box-sizing: Consider adding box-sizing: border-box; to the universal selector (*). This helps to make width and height calculations more intuitive by including padding and borders within the element's total width and height.

  Font Import: You’ve correctly imported the Poppins font. Make sure to include a fallback font, like sans-serif, in your font-family declarations to ensure your text displays properly if the custom font fails to load.

  Grid Layout: The grid layout is well-implemented, but the use of grid-template-rows: auto auto auto auto; in the card container might be redundant since all rows will automatically take up the necessary space. You can simplify this by removing the explicit row definitions.

  Media Queries: You’re using a media query to adjust the layout for larger screens, which is great! However, consider a mobile-first approach where the default styles cater to smaller screens, and media queries adapt the layout for larger screens.

  Color and Typography Classes: The use of utility classes for colors and typography (like .Red, .Cyan, .bold, .darker) is fine for small projects. However, in larger projects, this can lead to cluttered HTML. Consider moving these styles into component-specific classes or using a CSS preprocessor like Sass for better organization.

  Padding and Margin: You’re using padding and margin effectively to create space within and around elements. However, consider using relative units like rem or em instead of px for more responsive and scalable spacing.

  Button and Link Hover Effects: Although you don't have any interactive elements like buttons in this code, it’s good practice to add hover effects or transitions to any interactive elements to improve user experience.

  Use of Units: You’re using a mix of px and % units, which can sometimes lead to inconsistencies, especially with responsiveness. Try to use rem or em for consistent sizing that scales with the user’s default font size.

  Overall, your code is well-organized and demonstrates a good understanding of CSS grid and responsive design principles. With a few tweaks, you can improve accessibility, maintainability, and responsiveness. Keep up the great work!

  Marked as helpful

  0

  ---

  

  phucphan01866•130

  @phucphan01866Posted 3 months ago

  @RogerTito455 Thank you so much :3!

  0