Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • Enis67 140

    @Enis67

    Submitted

    I would be very grateful for tips for better coding

    Kamil 260

    @Wuczek

    Posted

    Great job!

    Your <main> tag isn't closed with closeup tag </main>

    I don't think you needed to use height property on your image, it should be auto, also I don't think it's a good practice to use unit vw in all of your widths (maybe not particularly in this project), instead of it use something like em,px or % Try to give your main container a certain width and your image just width:100% and it should be working a little bit better

    You can check my how I did that challenge here: My github repo

    0
  • Kamil 260

    @Wuczek

    Posted

    Great job!

    You need to make your JS functions more reusable, you wrote 5 functions that are basically doing the same thing. You can see my way of doing it: Github Repo , it's not great but I think you'll get what I meant.

    I would also recommend to wrap those inputs inside <li> tags like this:

    <ul>
    <li><input></li>
    <li><input></li>
    ...
    </ul>
    

    also change border-radius property in input element to border-radius:999px so it will input fields look like circle and not like the egg 🥚

    Marked as helpful

    0
  • Kamil 260

    @Wuczek

    Posted

    Great job!

    You didn't make use of image file image-header-mobile.jpg in images folder

    I would recommend to change your <div class="image-container"> into :

    <picture class="image-container">
      <source srcset="images/image-header-mobile.jpg" media="(max-width: 600px)" />
      <img src="images/image-header-desktop.jpg" alt="" />
    </picture>
    

    so that your image file automatically changes based on certain width, you can read more about <picture> tag here: <picture> tag

    0
  • Kamil 260

    @Wuczek

    Posted

    Great job!

    I would recommend to use letter-spacing:2px; property instead of adding spaces in your HTML in <a> tag : <a data-rating="submit" href="" class="btn-submit">S u b m i t</a>

    Marked as helpful

    1
  • Kamil 260

    @Wuczek

    Posted

    Hello coder 😎 Firstly I have to say u did a really good job :) My only tip is that you didn't include image-desktop into your layout, what I think would be better is a source media inside a picture that changes src of image to another one above certain width:

    <picture> <source media="(min-width:800px)" srcset="image-header-desktop.jpg"> <img src="image-header-mobile.jpg" alt="" style="width:auto;"> </picture>

    Here's more about <picture> tag :

    https://www.w3schools.com/tags/tag_picture.asp

    Marked as helpful

    0
  • @Asfen

    Submitted

    Hello, Any and all suggestion and corrections are welcome. Comments on my last project was grate help for me and I grew as a coder. I am still wondering if there is a way to keep the width of the component linearly responsive to the viewport size instead of using too many @media query.

    Kamil 260

    @Wuczek

    Posted

    Hello coder 😎 I would recommend you to use <picture></picture> element to handle that responsiveness image mobile-desktop :

    <picture> <source media="(min-width: 675px)" srcset="/images/image-product-desktop.jpg"> <img src="/images/image-product-mobile.jpg" alt=""> </picture>

    For more info about picture element: https://www.w3schools.com/tags/tag_picture.asp

    0