Skip to content
OverviewSolutions
8
Comments
4
Vladimir
@Vladmidir

All comments

  • Vladimir•160
    @VladmidirPosted over 1 year ago

    Good job with the challenge!

    Couple of things I want to say. First, you want all of the page content to be enclosed by landmarks to make it accessible. This challenge requires a<main> element, which you can simply substitute for the <div id="box_of_qrcode">. More on landmarks here.

    Another thing is that you should make a separate file for your styles (e.g. styles.css). This is a small challenge that does not require a lot of css, but normally you want to separate your html from css.

    Hopefully this was helpful!

    Marked as helpful
    1
  • Vladimir•160
    @VladmidirPosted over 1 year ago

    Good job with the challenge!

    One thing I would suggest is that you use landmark elements. Instead of making a div with class main, you can use the <main> element to make the page more accessible.

    Here is an article from w3schools on landmark elements.

    Happy coding!

    Marked as helpful
    1
  • Vladimir•160
    @VladmidirPosted over 1 year ago

    Good job with the challenge!

    Here are some things I picked up:

    • You want to use the <picture> element instead of CSS to change the image responsively.
    • For the crossed price, you should use the <sup> element to mark it as a superscript.
    • Add to Cart button should be a <button> element.
    • I like how you used the heading elements!
    • For mobile, you may want to scale it up a little. You should use percentages % to specify the width of the <main> element instead of rem. This will make it more responsive.
    main {
    max-width: 60%; /* take up at most 60% of the window size */
}

    Hopefully, this was helpful!

    Marked as helpful
    0
  • Vladimir•160
    @VladmidirPosted over 1 year ago

    Some of the things I noticed,

    • Your HTML should be enclosed by landmarks to convey the structure of the page. You should change the <div> container that encloses all the content to a <main>.
    • I suggest you use the tag <sup> for the old price superscript.
    • You may want to use the HTML <picture> element to responsively change the image, instead of changing it with CSS.
    • I suggest you put the perfume name inside the heading element <h1>.

    Hopefully, this is helpful. I am new to front-end, but that is some advice I have been getting myself.

    0