Skip to content
OverviewSolutions
4
Comments
39
Danish Mushtaq
@Danish49

All comments

  • Hengul Akash•70
    @habc0d3rSubmitted over 1 year ago

    I found it difficult to match the font size of the paragraph as the original one.

    QR Code component

    2
    Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Your Code may have accessibility issues as there is less use of semantic markup. You could have used a main tag instead of a div container , also there is one line of code unused <div class="qr-code"></div>. image can be directly placed in the main tag and text elements in a separate div. code needs optimization but the design is looking good much close to the original version. keep coding and congratulations on completing your first challenge.

    Marked as helpful
    1
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago
    This comment was deleted over 1 year ago
    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Text-align : centre;

    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Its not a best practice to keep every single element in a separate div rather we should follow the semantic markup for accessibility and better performance, You could have used a main tag instead of using div image can be directly placed in the main tag and text elements in a separate div and that all the required code for this challenge. Congratulations for completing your first challenge keep coding 👍🏻

    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Instead of using repeated divs you could have used a main tag for better accessibility and semantic markup you can keep the image directly in the main tag and text elements in a separate div and thats it no need of keeping everything in a separate div.

    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Nice design and nicely coded keep it up.

    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago
    This comment was deleted over 1 year ago
    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Works well but you should change the background colour and set the height of the body to 720px as most laptops come with 720px viewport height. This will make your design look much nicer but coming to javascript thats nicely implemented very impressive 👍🏻

    Marked as helpful
    0
  • Affan Shaikh•440
    @Terminal239Submitted over 1 year ago

    Please share feedback on the code structure and design layout and how I could improve things.

    Also, am using the rem units for the padding and margin because it helps things on the response side and keeps things clean. This is a controversial choice and after much research on px vs rem, I settled on rem when I found that Kevin Powell, one of the leading CSS professionals, goes for rem as well.

    Age calculator app

    2
    Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    This is perfect set the height of the body ko 720px because this is the viewport height of various laptops it will make the design more accurate. Code is also properly structured well done 👍🏻. Keep it up smart coder.

    Marked as helpful
    0
  • Guilherme-Luis•40
    @Guilherme-LuisSubmitted over 1 year ago

    So, it gets confusing when defining the shading on the main element, it seems that shading is used, but at the same time it seems that it is not.

    Responsive and references

    2
    Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    There os no use of box shadow you have set a different background colour that's why it seems to you that there is a shade in the original version.

    0
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    Till now i haven't seen any design that is properly sized like this so nice 👍🏻 But one this is there which needs attention its the width of text elements

    Marked as helpful
    2
  • Danish Mushtaq•310
    @Danish49Posted over 1 year ago

    I have a suggestion regarding your code. Instead of using a container div use a main tag for accessibility and semantic markup. You can place the image directly in the main tag and text elements in a separate div.

    By the way your Design looks great keep it up

    0