@EgnodiaSubmitted 8 days ago
lucielub
@LucieLuBAll comments
- @LucieLuBPosted 7 days ago
HTML Structure:
The structure is clear and semantic, using meaningful class names like card, head, and comment. This makes the code easier to understand and maintain. Alt Attributes:
Some alt attributes for images are empty (e.g., alt=""). While this is acceptable for decorative images, you should provide descriptive alt text for images that add value or context, like user photos. Class Naming:
The naming convention for colors in the card class (purple, blue, Darkblue, cream) is inconsistent. For example, "Darkblue" should be "dark-blue" or "darkBlue" to maintain readability and convention. Typography Tags:
Using <p> for headings like Daniel Clifford or Verified Graduate is not semantically correct. Consider replacing them with <h3> or <h4> for better accessibility and SEO. Inline SVGs:
The bg-pattern-quotation.svg could be embedded as an inline SVG for better control and scalability without additional HTTP requests. Repetition of Classes:
The head and descrpt classes are repeated across multiple cards with identical structure. Consider refactoring these into reusable components or templates to reduce redundancy. Accessibility:
Add ARIA roles or labels if necessary, especially for dynamic or interactive components (e.g., links in the footer). Footer Attribution:
The footer attribution is a nice touch, but the link for "Coded by De'vante" is empty (href="#"). Replace it with a valid URL or remove the link entirely. Scalability:
While the layout is functional, consider using CSS Grid or Flexbox for the card container to handle responsiveness effectively. General Improvements:
Ensure consistency in indentation to improve readability. Minimize hardcoding colors (like "purple" or "blue") directly in class names. Use variables in your CSS for better maintainability. Suggestions for Improvement Add meaningful alt text for user images. Use semantic tags (<h3> or <h4>) instead of <p> for titles like "Daniel Clifford". Refactor repeated HTML structures into reusable components if you're using a templating engine. Improve consistency in class naming and adhere to a convention (e.g., kebab-case or camelCase). Ensure responsiveness by testing the layout on various screen sizes. If targeting accessibility, enhance the experience with ARIA attributes and semantic improvements. Overall, this is a solid implementation with clear structure, but minor improvements in semantics, accessibility, and maintainability would make it even better.
0 - @leecockcroftSubmitted 10 days agoWhat challenges did you encounter, and how did you overcome them?
Desktop if I give the body 100vh. the layout renders nearly perfect. here. however viewing this on a lap top the header is missing? (which viewing 1440px should be fine) but its dragged above the screen?
What other approach should I do here?
@LucieLuBPosted 10 days agoHTML Structure: The overall structure is clear and follows a logical order. It’s great to see the use of semantic tags like <section>, <h1>, <h2>, etc., which help with accessibility and SEO. However, consider using a <header> tag instead of <section class="header"> to enhance semantics even more.
Class Names: The class names are easy to understand, though it could be helpful to be more descriptive for specific sections, especially in a multi-component project. For instance, instead of .left, .middle, and .right, consider names like .feature-left, .feature-center, and .feature-right to indicate their function and context within the layout.
Alt Attributes: Currently, the alt attributes in the <img> tags are empty. Adding descriptive alt text for each image, like “Supervisor Icon,” will make the content more accessible, especially for screen readers.
Consistent Spacing: Some HTML elements have inconsistent spacing around them. Removing unnecessary blank lines and organizing code into clearly defined sections would improve readability. For example, there are several empty lines in the <section class="middle"> that could be removed.
Comment on Inline Style Comment: The comment <!-- Feel free to remove these styles or customize in your own stylesheet 👍 --> doesn’t seem to have an associated style. If there were initial inline styles intended, consider either adding or removing this comment for clarity.
Viewport Meta Tag: Nice job including the viewport meta tag to ensure the site displays properly on all devices. This is essential for responsive design!
File Structure: Since the project may expand, keeping a logical file structure (e.g., separate folders for images, scripts, and styles) is a good practice. You’ve already started by storing images in an images folder, which is great for maintainability.
Typography: The font is correctly imported from Google Fonts, and Poppins is a good choice. Ensure that font-weight values in the CSS reflect the correct weights loaded in the font link.
Page Title: The <title> tag is descriptive, which is good. However, consider removing the placeholder text Frontend Mentor | Four card feature section when the project is finalized to make it more customized.
HTML5 Compliance: Adding a <footer> if necessary and ensuring consistent usage of <main> and <header> will help meet HTML5 standards as the project expands.
Overall, you've done a solid job! These suggestions should help improve the readability, accessibility, and maintainability of the code. Keep up the good work!
1 - @mrdonuzzSubmitted over 1 year ago@LucieLuBPosted 2 months ago
Tu código se ve bastante bien y parece que has hecho un buen trabajo en crear un diseño responsive. Aquí tienes algunos comentarios y sugerencias para mejorarlo aún más:
HTML Uso de imágenes: Considera usar <picture> para manejar diferentes fuentes de imagen más eficientemente, especialmente si necesitas imágenes de diferentes tamaños para diferentes dispositivos.
Accesibilidad: Asegúrate de que todas las imágenes tengan atributos alt descriptivos. Por ejemplo, la imagen principal podría tener un texto alternativo más informativo.
Enlace "Add to Cart": Considera agregar un atributo href real para que el enlace sea funcional, o usa # como placeholder.
CSS Flexbox: Estás utilizando flexbox correctamente, pero podrías considerar el uso de grid para una mayor flexibilidad en el diseño, especialmente en la sección de contenido.
Medidas responsivas: En lugar de usar vw y vh, podrías considerar el uso de % o rem para hacer el diseño más fluido. Esto puede ayudar a la adaptabilidad en diferentes tamaños de pantalla.
Media Queries: Buen uso de media queries, pero asegúrate de que cubren una variedad de dispositivos. Podrías agregar breakpoints adicionales si es necesario.
General Comentarios en el código: Considera agregar comentarios para aclarar la intención de ciertas partes del código, lo que puede ayudar a otros (o a ti mismo en el futuro) a entender mejor tu trabajo.
0 - @HamzehBajboujSubmitted 2 months ago
- @py-code314Submitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I was able to load local static fonts using @font-face property
What challenges did you encounter, and how did you overcome them?- Had difficulty in measuring the spacing in design files without Figma files. Used Chrome extensions 'Dimensions' and 'PerfectPixel' to get the measurements, still it took a lot of trial and error to get the look as close as possible
- Found another resource to compute
clamp()function values
- I changed background & text colors as I didn't like the colors in design file. I hope the color contrast is acceptable
- I'm still not sure that I'm naming the classes and custom properties right way. I appreciate if anybody could help me with that
- I used
clamp()for changing padding. Is this function can also be used for spacing in addition to change font-size?
@LucieLuBPosted 3 months agoFeedback on CSS:
1- Font Management:
Using @font-face to load custom fonts is a good practice, and you're loading different weights, which is great for flexibility. However, consider using a font-display property (e.g., font-display: swap;) to enhance performance by showing fallback fonts while custom fonts are being loaded.
2- Variables (:root):
Color Variables: You’ve defined a well-organized and clear set of color variables. It’s great that you’ve separated them into background and text colors.
Typography Variables: Defining font sizes and weights as variables improves maintainability and scalability. One suggestion: use rem units consistently for better responsiveness (e.g., --font-size-name: 1.5rem; is good).
Spacing Variables: Your usage of clamp() for dynamic spacing and sizing is excellent, as it provides responsive flexibility across different screen sizes.
3- Global Reset:
The use of a global reset and box-sizing: border-box; is a good practice to standardize padding and margin behavior across browsers.
You may want to consider adding line-height: inherit; to the reset for a more predictable line-height inheritance.
4- Responsive Typography:
It’s good that you use rem and clamp() for scaling text and elements, ensuring your design is responsive. However, you might want to define more media queries if further fine-tuning is needed on different screen sizes.
5-Accessibility:
The use of .sr-only for visually hidden elements is a great accessibility practice, especially for screen readers. Make sure interactive elements like the social media links are fully accessible (e.g., by ensuring good color contrast and focus states).
6-Hover and Focus States:
The hover and focus states are well-defined. You've added color and background changes for interactivity, which enhances user experience. Consider adding a more visible focus outline (instead of just removing it) for better keyboard accessibility.
7- Box Shadows:
You’ve included a custom box shadow on hover (--box-shadow-hover). This adds depth to interactive elements, which is a nice touch.
8- Social Media Links (.social__link):
The button-like styling of the links (display: block, padding, background-color) works well for mobile and desktop environments. Ensure that color contrast ratios between text and background colors meet accessibility standards, especially in the hover and active states.
Feedback on HTML:
1-HTML Semantics:
The structure of the HTML is clean, well-organized, and follows good semantic practices (e.g., header, main, h1, h2). Visually Hidden Heading: The h1 with the .sr-only class is a good practice for screen readers, helping with accessibility without affecting the visual layout.
2- Profile Section:
The structure of the profile section is clear and concise. The use of a blockquote tag for the quote adds semantic value. Consider adding a <figcaption> for images if more context is needed (e.g., profile pictures).
3- Social Media Links:
The ul element for social media links is semantically correct. Each li represents a list item, and the links themselves are appropriately structured. Ensure the href attributes in the anchor tags (<a>) point to valid URLs. They currently contain empty href="#", which should be replaced with real links or href="javascript:void(0)" if no action is expected.
4- Use of Alt Attributes:
You’ve added a descriptive alt attribute for the profile image, which is excellent for accessibility. Ensure all other images (if added later) also have meaningful alt attributes.
General Recommendations:
Performance: Use font-display: swap; in your @font-face rules to improve loading performance and avoid invisible text during font loading.
Accessibility: Consider enhancing focus styles for better keyboard navigation (e.g., more visible outlines or shadows). Ensure color contrast is sufficient, especially for links in different states (hover, focus, active).
Responsiveness: You’re already using clamp() and rem, which is great for responsiveness. You may want to test your layout on various screen sizes to ensure everything scales smoothly. Consider adding media queries for more control over breakpoints if needed.
Consistency: Ensure consistency in the use of units (rem, px, em). For example, you're using rem for text sizes but px for some padding. Stick to rem for padding and margins to ensure scalable design.
Overall, the code is clean, well-structured, and shows great attention to detail with accessibility and responsiveness in mind!
Marked as helpful0 - @DotzinSubmitted 3 months ago@LucieLuBPosted 3 months ago
Your HTML structure and the SVG implementation look good overall, but there are a few points of improvement, best practices, and feedback to enhance your code's readability, maintainability, and performance.
- Naming Consistency and Readability: Use consistent class naming throughout your code. You're mixing English and Portuguese (conteiner, aprendendo, publicacao, titulo, etc.). It's better to stick to one language for class names to maintain consistency and avoid confusion.
For example, rename:
<div class="conteiner"> <!-- Use container instead --> <div class="aprendendo"> <!-- Use learning instead --> <div class="publicacao"> <!-- Use publication instead --> Consider using more descriptive and meaningful class names in English if that's your project's language.- Semantic HTML: You are using div elements for everything, including the title and description, which reduces the semantic meaning of your HTML. To improve this, use semantic HTML tags where appropriate:
Replace the div for the title with a header or h1. Replace the description section with a p tag for paragraph content. Example:
<header class="title"><h1>HTML & CSS foundations</h1></header> <p class="description">These languages are the backbone of every website...</p>- Accessibility: Alt attributes: Always add meaningful alt text to images for accessibility. For the avatar image, you should add a proper description instead of leaving the alt attribute empty.
SVG accessibility: For your SVG graphic, consider adding aria-hidden="true" if it’s decorative and doesn't provide important information for users with screen readers. If it conveys information, add a proper title inside the <svg> tag.
Example:
<svg aria-hidden="true" ...>
- CSS Inline Styles: You have inline styles for the .attribution class. It’s generally a good practice to separate styles from HTML. Move these styles into the style.css file for better separation of concerns.
Example:
.attribution { font-size: 11px; text-align: center; }
.attribution a { color: hsl(228, 45%, 44%); }
- Optimization and Performance: Reduce SVG complexity: The SVG you are using is quite complex. If it’s decorative or not essential for understanding the page, you might want to consider optimizing it using tools like SVGOMG to reduce the size and complexity.
Avoid empty alt attributes unless an image is purely decorative.
-
Responsiveness: You already have the meta viewport tag, which is great for ensuring responsiveness, but you might need to check if your layout is responsive across different screen sizes. Consider adding media queries in your CSS to handle small screens and ensure elements like text, images, and SVGs are scaled appropriately.
-
Minor Corrections: Fix spelling issues like conteiner (should be container), and make sure to follow proper naming conventions.
Final Feedback:
Good points: The use of SVG is nice for scaling and quality, the meta tags are properly included, and the structure of the document is organized.
Areas to improve: Focus on using semantic HTML, improving accessibility, ensuring naming consistency, and separating styles from the HTML file. These changes will make the code cleaner, easier to maintain, and more scalable.
With these improvements, your code will be more efficient, accessible, and maintainable.
0 - @Davidty143Submitted 3 months ago@LucieLuBPosted 3 months ago
Good job, Here's some feedback on your CSS code:
@import for Google Fonts:
The @import statement is fine, but you might consider using a <link> tag in the HTML for performance reasons, as it loads the fonts faster and prevents blocking styles. Body Styles:
place-items and place-content: These are invalid in this context because they only work within a grid or flexbox container. You should use display: flex; along with justify-content: center; and align-items: center; to achieve centering. Consider adding margin: 0; to the body to remove default margins. Image Styling:
Using width: auto; and height: auto; is redundant, as the image already maintains its aspect ratio by default. You can safely remove them. The max-width: 200px; is useful, but depending on the design, consider adding a media query to adjust it for larger screens. Shape Containers (.shape-container, .inner-square-shape, .outer-square-shape):
The display: flex; properties in both the .shape-container and .outer-square-shape are well-implemented, but ensure you’re intentional with the use of flex-direction: column;. If these shapes need to be horizontally aligned on larger screens, consider a media query to adjust the layout. The padding and margin between .outer-square-shape and other elements seem fine, but you might want to test how they behave responsively. Text Styling (.text-style1, .text-style2):
The styling is consistent with the rest of the design and works well with font-family: 'Outfit', sans-serif. You’ve appropriately adjusted font weights and sizes. The width: 170px; applied to both text styles may cause issues in smaller screens. Consider using percentages or max-width for responsiveness. Good use of line-height for readability. Media Queries:
@media (min-width: 1440px): The intention of setting the body width and centering it is good. However, consider using a more flexible layout system (like grid or flexbox) to handle different screen sizes rather than fixing the body width to a specific pixel value.
Marked as helpful0
