Skip to content
OverviewSolutions
16
Comments
453
Francisco Carrillo
@frank-itachi

All comments

  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the page's whole main content in the <main> tag.
    • Since the mobile design has a different image, you can use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
    • Make sure that the <img> elements in your HTML code has an alternate (descriptive) short text. The reason for this is that screen readers can’t translate images into text. So to fix this you can do the following <img src=”…” alt=”short text” >

    CSS 🎨:

    • You can use grid or flexbox to center the content no matter the viewport size. Using Grid:
    body {
	min-height: 100vh;
        display: grid;
	place-content : center;
}

    FlexBox

    body {
	min-height: 100vh;
        display: flex;
	align-items: center;
	justify-content: center;
}
    • You can also use the text-transform: uppercase; property to make the perfume word appear in upper case even though you typed it in lower case in the HTML file.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    Marked as helpful
    1
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. You did a good job!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the page's whole main content in the <main> tag.
    • Since the mobile design has a different image, you can use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy <coding /> 😎!

    Marked as helpful
    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Since the mobile design has a different image, you can use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    Marked as helpful
    1
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    • You could use the background: linear-gradient(#color1, #color2); property-value to style the pseudo class :hover of the button.
    • Also try not nest too much rules in SASS/SCSS becuase the more you nest the more time will take to the browser to load the code. This way you keep the code clean as well.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    Marked as helpful
    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    • Your code is readable and well structure but when it comes to functionality and user experience there are some aspects we’ve got to think. For instance, according to de design in the <footer> section there must be some links that should redirect the user to a different page that provides additional information. The About Us link is a good example. So, you should use a <a href=””> tag to achieve such purpose instead of using the <p> one.
    • Remember, we also gotta take into account the user experience and not only the design.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • The icon-star images are decorative so you can add the alt="" attribute to the <img> tag and leave it empty. But the profile images aren't so in that case you need to add a short description.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    Marked as helpful
    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. You did a good job!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the <div class="main-container" content in the <main> tag.

    CSS 🎨:

    • Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy <coding /> 😎!

    Marked as helpful
    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the <div class="card"> content in the <main> tag.
    • You could also use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
    • In this case there is only one <footer> tag so you can get rid of the class="footer" attribute of it and use the selector tag in you CSS stylesheet.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    Marked as helpful
    1
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. You did a good job!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the page's whole main content in the <main> tag.
    • The heading order is important in the html structure so try to always start your headings and/or titles with an <h1> tag and then you can increase by one if you need to use more heading in your html code.
    • The image is not decorative in this case so make sure that the <img> element in your HTML code has an alternate (descriptive) short text. The reason for this is that screen readers can’t translate images into text. So to fix this you can do the following <img src=”…” alt=”short text” >

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy <coding /> 😎!

    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. You did a good job!

    I have some suggestions about your code that might interest you.

    CSS 🎨:

    • You can use grid or flexbox to center the content no matter the viewport size. Using Grid:
    body {
	min-height: 100vh;
        display: grid;
	place-content : center;
}

    FlexBox

    body {
	min-height: 100vh;
        display: flex;
	align-items: center;
	justify-content: center;
}
    • Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy <coding /> 😎!

    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. Congratulation for completing the challenge👍!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • Wrap the <div class="attribution"> section inside the <footer> tag.
    • you could also use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.

    CSS 🎨:

    • Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.

    I hope you find it useful! 😄 Above all, you did a good job!

    Happy <coding /> 😎!

    0
  • Francisco Carrillo•5,540
    @frank-itachiPosted over 1 year ago

    Hello there 👋. You did a good job!

    I have some suggestions about your code that might interest you.

    HTML 📄:

    • You could also use the <picture> tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
    • The perfume images are not decorative so make sure to add a valid descriptive alternate text.

    I hope you find it useful! 😄 Above all, the solution you submitted is great!

    Happy <coding /> 😎!

    0