Skip to content
OverviewSolutions
24
Comments
8
Pietrell
@PiotrPlotast

All comments

  • Pietrell•400
    @PiotrPlotastPosted 12 months ago

    Hey, well done on completing the challange! If you're just starting out keep going! I see you applied very basic css reset and that's good, for bigger projects you could look at: andy bells css reset. The one thing I'd change is h2 font-size to fit in two lines. To add some accessibility(some may prefer bigger font size as their browsers default) padding could be applied with rem or em units, besides that very well done!

    0
  • Pietrell•400
    @PiotrPlotastPosted about 1 year ago

    Hi, first of all congrats on completing the challange! I'll advice you:

    • Instead of <div class="header"> you could use <header> tag. It's part of semantic html(I see you know what it is).
    • instead of 2 different imgs for mobile and desktop view use <picture> tag element with srcset and media attributes. MDN article on picture element.
    • there's a bug on the website, when i click anywhere nav buttons dissapear, you should work on this issue -do not use px units as much and especially for anything related to font as it breaks accessibility(read about realtive units, the most important are em rem and less important ch). For instance font-size: 16px; should be font-size: 1rem; as it scales with users browser font size and px don't.
    • you could clean up unnecessary comments in your css because there's a lot of unused styles.
    • read about mobile first approach(trust me it's easier in the end)
    • socials icons should be links
    • read about aria-attributes for mobile nav menu
    Marked as helpful
    1
  • Pietrell•400
    @PiotrPlotastPosted about 1 year ago

    Hey, great job completing the project! I've done it recently as well so the tips from me would be:

    • add placeholder to the email input html element with the text Your email address...
    • adjust some margins between h1 and p
    • instead of wrapping h1 and p in a div you could just use h1 and h2 semantic structure
    • font size on the button could be bigger.
    • If you need any help write it in the reply :)
    0
  • William Chandra•30
    @williamc712Submitted over 1 year ago

    i dont really know how to utilize the 2 product images so i just put display none in the mobile breakpoint, is that okay? or is there a better way to implement the desktop and mobile product image?

    Product preview card using CSS Flex

    3
    Pietrell•400
    @PiotrPlotastPosted over 1 year ago

    When you need to change image depending on the size of the screen you should use picture html element. Here is helpful article on mdn: The picture element

    2
  • Pietrell•400
    @PiotrPlotastPosted over 1 year ago

    Hi, here's some advice from me:

    • In your project if you have images uploaded and want them to display properly in your GitHub page, the source of the image should start with ./ and not with / -> <img src="./images/image-product-desktop.jpg" alt="" class="desktop">.
    • If your image changes depending on the screen size you should use picture html element.
    • You should do mobile-first approach for your web pages(it's easier trust me).
    • Font size shouldn't be in px units, instead search for em and rem units.
    • Read about Semantic HTML
    Marked as helpful
    1
  • Pietrell•400
    @PiotrPlotastPosted over 1 year ago

    Hello, in this project you don't need to worry about your js experience due to the fact that this is html and css only project which means it's static page with no js needed. Flex is by far the easiest way to do layouts, I'd recommend watch/read about it and its properties. In this particular project flex-direction: column; could be helpful for the layout of your component. You should avoid using px units with font-size, instead read/watch about relative units like em and rem. Overall you did a good job. Stay strong and keep learning! ;)

    0
  • Mihir Bhatkar•80
    @mihirbhatkarSubmitted over 1 year ago

    What are the best practices for spacing out text? I used <br/> tag twice for seperating the text, which feels very noob-like. Well I am a noob anyways...

    Simple QR code scanner

    1
    Pietrell•400
    @PiotrPlotastPosted over 1 year ago

    "What are the best practices for spacing out text?" - you could read this article about text-formatting, it briefly explains text formatting properties that you could use to space out text. Also you should use em and rem units for font size and widths. For responsiveness you could use max-width instead of width.

    Marked as helpful
    1
  • Celestino Trosso•200
    @iamcelestinoSubmitted over 1 year ago

    this project was a little bit simple, I had trouble centralizing my main container.

    My question is about my css code, is there any best practice you recommend?

    qr-code-component

    1
    Pietrell•400
    @PiotrPlotastPosted over 1 year ago

    You could read more about rem and em units which are used for responsiveness of a website and use them on your font-size style and max-width of an element so it can scale easily with users preferences. Here are some great resources: font-size em and rem units

    Marked as helpful
    0