@mrdonuzzSubmitted about 1 year ago
lucielub
@LucieLuBAll comments
- @LucieLuBPosted about 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 about 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 2 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 2 months ago@LucieLuBPosted 2 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 2 months ago@LucieLuBPosted 2 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
