Chermann KING
@Chermann-KINGAll comments
- @ngoosenSubmitted 4 months ago@Chermann-KINGPosted 4 months ago
Hi @ngoosen👋🏾🙂,
Congratulations 🎉 on your proposed solution to the challenge.
A little recommendation🤓 for font sizes, it would be better to use the rem unit.
Here is a 3-minute read on the subject units of measurement (px, em and rem) that could be useful to you.
Good code 😉
0 - P@RopenfoldSubmitted 7 months agoWhat are you most proud of, and what would you do differently next time?
I'm particularly proud of the responsiveness of this page and it was good to reintroduce myself to css. I would potentially use sass or tailwind in future projects.
What challenges did you encounter, and how did you overcome them?I only had a few challenges. Initially the switch to mobile gave me some issues until I understood the @media concepts in css. I also had some issues with github pages, in particular the image basepaths but thankfully I worked these out looking online.
What specific areas of your project would you like help with?I'm happy with my project as it is.
@Chermann-KINGPosted 5 months agoHi @Ropenfold,
I took the time to look at the code of your solution and I did the test, I think you did a good job. The only thing I can point out is your style management (borders, colors, shadows) compared to the model but really nothing very objective or blocking.
Well done ;)
Marked as helpful1 - @JarmovdSubmitted 11 months agoWhat are you most proud of, and what would you do differently next time?
Focused on mobile first and made it easier to change to desktop.
What challenges did you encounter, and how did you overcome them?Worked with two imgs in the html. One for mobile and one for desktop and alternated between them via display none through media queries for mobile and desktop.
What specific areas of your project would you like help with?Is there another method to change between these two imgs for mobile and desktop?
@Chermann-KINGPosted 11 months agoHi Jarmo 👋🏾😀
First of all, congratulations 👏🏾 on the work you've done🙂. It's not bad at this level. Now you just have to take into account the elements provided for particular types. That's the best way to improve accessibility.
I recommend that in this type of case where the interface designer provides you with several images depending on which device the user will end up on, you use the picture tag in HTML, which potentially receives
source
tags and theimg
tag. This will simplify stylisation and is mainly designed for this type of case.Here's the link to my solution if you want to see how I solved the challenge 😉
Marked as helpful1 - @Slaks97Submitted almost 2 years ago@Chermann-KINGPosted almost 2 years ago
👋Hi @Slaks97
I think you've already done a great job👌 in terms of accessibility.
Here are a few suggestions for improving your code:
- HTML:
Caching of external resources, such as Google Fonts, could be improved. You can add
rel="preload"
attributes to preload these resources and improve performance.- CSS:
1. Use CSS variables: CSS variables let you define reusable values for text styles, such as font size or color. By using variables, you can guarantee style consistency and facilitate subsequent modifications.
2. Units: Instead of using a fixed font size in pixels (
px
) as in thebody
, you can use a relative font size inem
orrem
. Use relative units of measurement (em, rem) for font size to create flexible and adaptable text styles, simplifying global adjustments of text size in your project.3. Use reusable classes for text styles: If you have similar text styles that are repeated in several elements, you can create reusable classes. This avoids repeating the same styles in different CSS selectors, simplifying code and facilitating subsequent updates.
4. Use more targeted selectors: In some cases, you can use more specific CSS selectors to apply specific styles to certain text elements. This saves you having to add additional classes to your HTML elements. You can use selectors based on the structure of your HTML to precisely target the desired elements.
The aim of these suggestions is to improve the maintainability, consistency and reusability of text styles in your CSS code. By following these best practices, you can make your code cleaner, easier to understand and easier to maintain. This is very useful when working on projects that are likely to evolve, for example.
Apart from these few tips, your code looks well structured and functional.
Good code😉
Marked as helpful0 - @sebytrezaSubmitted almost 2 years ago@Chermann-KINGPosted almost 2 years ago
Hi @sebytreza! ✋
HTML code :
- You have used ids for several elements. If these elements are not supposed to be unique on the page, it would be better to use classes instead. Ids are generally used for unique elements, while classes are used for elements that can be repeated.
Code CSS :
-
It is recommended to separate the CSS styles from the HTML file. You already have a style.css file, but there are also inline styles in the HTML file. Move the inline styles into the style.css file to keep a clear separation between the structure (HTML) and the presentation (CSS).
-
You could use more descriptive and consistent class names, using a naming convention such as BEM (Block, Element, Modifier) to make the code easier to understand and maintain. For example, instead of using identifier names like "eth" and "delay", use class names like "card__price" and "card__time-left" or other more descriptive names.
-
Try to avoid using fixed widths and heights for elements, especially when dealing with containers. Fixed dimensions can cause layout problems on different screens and resolutions. Instead, use relative widths and heights (%, vw, vh) or flexible units (rem, em) for better scalability.
-
In CSS, identifier selectors (#id) have a higher specificity than class selectors (.class). Since you have used identifiers for some elements, this can cause problems if you want to apply additional styles using classes. It is better to replace identifiers with classes to avoid specificity problems and to make the code easier to maintain.
-
The
#overlay:hover img
selector can be replaced by a class to be more consistent with other selectors. For example,.overlay:hover .overlay__img.
-
Use CSS variables to store colors, fonts and other recurring values. This makes maintenance easier and allows you to quickly change the themes or colors of the site.
👇Here are some additional suggestions for improving your code and further optimization:
1 Units used:
- For margins, spacing, and font sizes, it is recommended to use relative units such as rem or em. This allows for better accessibility and more consistent scaling on different devices and screen sizes.
- For widths and heights, it is best to use percentages (%), viewport units (vw, vh, vmin, vmax), or flexible units (fr for CSS grids) for a more adaptive layout.
2 Structure of HTML elements :
- Use semantic HTML elements to describe the content in a more meaningful way. For example, instead of using a
<div>
to encompass the content of the map, you can use an<article>
tag. - You can also use
<header>
,<footer>
, and<section>
tags to organize content more semantically. - For images, always add an
alt
attribute to describe the image. This improves accessibility and helps screen readers understand the content of the image.
3 Accessibility:
- You can improve the accessibility of your site by adding Accessible Rich Internet Applications (ARIA) attributes to elements. For example, add
aria-label
oraria-labelledby
to buttons and links to provide additional information to users who use screen readers. - Make sure there is sufficient color contrast between the text and the background for readability. You can use online tools such as the WCAG Color Contrast to check if colors meet accessibility standards.
4 Image Optimization:
- Optimize images to reduce file size and improve load times. Use online image optimization tools such as TinyPNG or ImageOptim to compress images without quality loss.
- Use the
<picture>
tag withsrcset
andsizes
attributes to provide images suitable for different resolutions and pixel densities. This improves performance and user experience on high pixel density devices.
I hope it will be useful for you for the future. Courage and good code 😉
0 - @JxnfernandxSubmitted almost 2 years ago@Chermann-KINGPosted almost 2 years ago
👋Salut @Jxnfernandx
Here are some tips with case studies for choosing units in CSS
1.Pixels (px):
Practical case:
.container { width: 300px; }
-Advantages: Precise and easy to understand, pixels are commonly used to define fixed dimensions. -Disadvantages: Dimensions defined in pixels do not adapt based on screen size or user preferences, which can lead to readability or layout issues on some devices.2.Percentage (%):
Practical case:
.container { width: 50%; }
-Advantages: Percentages allow for fluid layouts that adapt to the screen size. They are particularly useful for responsive designs. -Disadvantages: It can be difficult to predict the exact appearance of elements with percentage-based dimensions, especially if the parent container dimensions are not clearly defined.3.Em (em):
Practical case:
p { font-size: 1.5em; }
-Advantages: "Em" units are based on the font size of the parent element, allowing for designs that adapt to the user's font size preferences. -Disadvantages: Dimensions defined in "em" can be difficult to manage, especially when nested within multiple levels of parent elements.4.Rem (rem):
Practical case:
p { font-size: 1.2rem; }
-Advantages: "Rem" units are similar to "em," but they are based on the font size of the root element (typically the <html> element). This avoids the nesting issues encountered with "em." -Disadvantages: As with "em," dimensions defined in "rem" can be less predictable than those defined in pixels.5.Viewport units (vw, vh, vmin, vmax):
Practical case:
.container { width: 50vw; }
-Advantages: Viewport units allow for designs that adapt to the dimensions of the browser window, which is useful for fluid and responsive layouts. -Disadvantages: Dimensions defined in viewport units can be less predictable than those defined in pixels and may require additional adjustments to ensure proper layout on different devices.Each of these units has its own advantages and disadvantages, and it is often wise to combine them to create designs that adapt well to various devices and user preferences.
Good code😉
Marked as helpful1 - @CalebeRLSubmitted about 2 years ago@Chermann-KINGPosted about 2 years ago
Hey @CalebeRL, Congratulations on completing this challenge!
Your solution its almost done and I’ve some tips to help you to improve it:
In the
body{ }
you can put your general font-family rather than on the*{ }
(which implies an absolute field of action).If you want to reset some elements you can do it with
*{ }
and choose the elements to reset. Here, your CSS reset is not effective for this project, to optimize it you will have to add elements. Here are some links that address the issue: Link 1 , Link 2 , Link 3Why give a class to your image knowing that in the project, it's the only one that exists and that to select it in CSS you'll just have to to call its img tag. Again a question of accessibility but also of specificity.
As for your h1. For a reason of accessibility the search engine that indexes this page knows that the h1 is the highest heading so it is identified by default. So by default it is identified as a high level heading as a title. If you had put your title in a span or a div, then you would have to add an attribute
role="title"
.The size of your image will never take 90% (
90vw
) of the width of the screen because you have set amax-width: 310px
for your card. In this case you have to set it to 100% of the width of the card.Use relative units like rem or em which is much better than px to improve performance when resizing fonts between different screens and devices. In your case, you use both rem and px, which is not recommended in terms of accessibility.
Instead of a simple div to display the copyright notices you can either put as an attribute to that
<div role="footer"></div>
or use the semantic tag dedicated for that which is<footer></footer>
Psss: Include only elements that your project will need in the reset to avoid loading your code.
Make it Work, Make it Clean.
Here's my solution for this challenge if you wants to see how I build it.
✌️ I hope this helps you and happy coding!
Marked as helpful0 - @Rae1821Submitted about 2 years ago@Chermann-KINGPosted about 2 years ago
Hey Rachel !
Good job. The only thing I can recommend is the use of a CSS class when you feel that the selectors are so numerous that you risk confusing them all. And don't forget that it also affects accessibility
0 - @monsdaSubmitted over 2 years ago@Chermann-KINGPosted over 2 years ago
Hi @monsda, here are my few suggestions you can consider adding to your code:
-
On your element
<p id="perfume">P E R F U M E</p>
, rather than separating the letters in hard you can in your css apply the property *letter-spacing * and assign the value that corresponds to the spacing you are interested in. -
To your selector body you assigned an absolute position , which is not very practical in the context of this project and according to your html structure. Click here if you want to make a small revision related to the positioning of an element of the html page. Pssss: It's not your page that needs to be positioned but rather the product that you wrapped in a
div
with the class container -
On the other hand, on your container you could apply an absolute position, while making sure to manage the
top
and theleft
of this container and then apply thetransform
property. this combination will fix the issue of centering the element in the page.
here is a link to more detail related to the transform property and the translate method which responds well to your need.
If you're interested, click here to see my solution to this challenge :)
Good code to you ;)
Marked as helpful1 -
- @CheosphereSubmitted over 2 years ago@Chermann-KINGPosted over 2 years ago
Hi @Cheosphere! It's great your work. If I have one thing to say it's to think about making your CSS cleaner with the DRY ;).
1 - @sirsky89Submitted over 2 years ago@Chermann-KINGPosted over 2 years ago
Hi @sirsky89, here are some recommendations to improve the code you submitted:
-
The correct structure of a page when you want to use a
<header>
element would be to take it as an introductory container. It must have, as its name indicates, a title from levelh1
toh6
, and if desired, a description or a logo... -
You include your image with a
div
, which is not necessary in this case. we usediv
only when we have several elements to manage the positioning in the page. Note: always remember to have the most sober code possible (clean). -
In your CSS you attribute the value 'column' to the property Display, which is not possible. I'll let you take a look at the possible values if you don't use a Flex Box on the following link -> Display
-
Also, the
<p>
and<h.>
elements areinline
by default, i.e. when displayed, they are positioned one above the other and not next to each other. side of others. So no need in your case to store it by a column property value.
If you wish, you can take a look at my solution to this challenge here
I hope my feedback will be useful to you ;) Good code to you!
Marked as helpful0 -
- @zikoechetabuSubmitted over 2 years ago@Chermann-KINGPosted over 2 years ago
Hi @zikoechetabu, here are some recommendations regarding the code you provided:
-You can make it much simpler already by removing the
section
but if you really want to use them they must have a heading fromh2
toh6
if not, adiv
if you want to include your elements will do.-You can display your
img
,h1
andp
elements in adiv
.Here are links that will show you the small difference between a div and a [section](https://www.w3schools. com/tags/tag_section.asp)
Here my solution to this challenge
I hope it will be useful for you ;)
1 - @Chermann-KINGSubmitted over 2 years ago@Chermann-KINGPosted over 2 years ago
**Hi @vcarames! Thanks for your feedback! ** I've taken your recommendations into account and updated my code. If you want to take a look again and tell me what you think ;)
0 - @AM-pngSubmitted over 2 years ago@Chermann-KINGPosted over 2 years ago
Hey @AM-png, some suggestions to improve you code:
If you notice, it is a card displaying a QR code. The
<aside>
tag that you use here to enclose each element of the map (texts and image) is not recommended for this purpose. You could have used a simple<div>
or even an<article>
. All the same congratulations for the work done.I leave you the link that can show you the good use of <aside> and <article> tags
Marked as helpful0