Skip to content
OverviewSolutions
5
Comments
5
Sonsawan Ngamsom
@murphy6867

All comments

  • Sonsawan Ngamsom•140
    @murphy6867Posted over 1 year ago

    I have a few suggest for you. First of all semantic HTML, I think first <div> tag after <body> it's should be <main> tag and last <div> should become <footer>. It's help for accessible. Second thing is container of content and media query of screen you should style of mobile first and next to large screen, i found a lot of error about responsive design. I hope my suggestion is helpful :D.

    Marked as helpful
    1
  • Romana Bezdekova•50
    @11nenaSubmitted over 1 year ago

    Hello, I did little bit of more work on it. Thank you for all your feedback. :) Let's see another challenge :)

    QR code component

    2
    Sonsawan Ngamsom•140
    @murphy6867Posted over 1 year ago

    Some recommendations regarding your code that could be of interest to you.

    In order to fix the accessibility issues:

    You need to replace <div class="container"> with the <main class="container"> tag and <div class="attribution"> with <footer class="attribution">. You'd better use Semantic HTML, and you can also reach more information about it from Using Semantic HTML Tags Correctly. Each main content needs to include at least h1 element so you should use one <h1> element in the <main> tag. You can replace your <h3> Improve your front-end skills by building projects</h3> element with the <h1>Improve your front-end skills by building projects</h1> element. Finally, you should click generate a new report on this solution page to clear the warnings.

    Hope I am helpful. :)

    0
  • Sonsawan Ngamsom•140
    @murphy6867Posted over 1 year ago

    All of it's rather that would be great, but still missing about size of the box.

    • Imagine In the content-box have 2 components, You build 2 square box in component, I think it would be event better if you did 2 vertical rectangle
    • And the good practice you should have to wrap container in the <main> element and attribution in the <footer> element. You'd better use Semantic HTML, and you can also reach more information about it from Using Semantic HTML Tags Correctly : https://laylacodes.hashnode.dev/what-is-semantic-html

    After committing the changes on GitHub and you need to deploy it as a live site. Finally, you should click generate a new report on this solution page to clear the warnings.

    Hope I am helpful. :)

    Marked as helpful
    0
  • Sonsawan Ngamsom•140
    @murphy6867Posted over 1 year ago

    Your result looking pretty good but i didn't see your code, If you have repository please post it under my comment. And i have some thing to suggest about some thing that you missing it.

    • First i recommend you to re-check about viewport and width / height.
    • You should've to learning about box-shadow css. that can help your project looking better. : Let's go :D
    0
  • Sonsawan Ngamsom•140
    @murphy6867Posted over 1 year ago

    You forget border-radius in QR code and box-shadow.

    Marked as helpful
    1