I found it difficult to match the font size of the paragraph as the original one.
Danish Mushtaq
@Danish49All comments
- @habc0d3rSubmitted over 1 year ago@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 helpful1 - @sam0560Submitted over 1 year ago
- @Wandsv0Submitted over 1 year ago
- @cfezSubmitted over 1 year ago@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 - @SharangUkidveSubmitted over 1 year ago@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 - @boluamooSubmitted over 1 year ago
- @SharangUkidveSubmitted over 1 year ago
- @Brunozo2042Submitted over 1 year ago@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 helpful0 - @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 onpx
vsrem
, I settled onrem
when I found that Kevin Powell, one of the leading CSS professionals, goes forrem
as well.@Danish49Posted over 1 year agoThis 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 helpful0 - @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.
@Danish49Posted over 1 year agoThere 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 - @abobykinSubmitted over 1 year ago@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 helpful2 - @Nihal00Submitted over 1 year ago@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