@Enis67
Submitted
I would be very grateful for tips for better coding
@Wuczek
@Enis67
Submitted
I would be very grateful for tips for better coding
@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
@Ghazanfar429
Submitted
Hello Everyone. Its my first project with JavaScript. Feedbacks are welcome.
@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
@Ahmedalzarka
Submitted
any recommendations
@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
@victorgfz
Submitted
I think the js could a little bit better, more optimized. I found some difficult on making some of the animations with js.
@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
@basitkorai
Submitted
Hello Everyone, I'm Looking forward to your feedback and advice on how I can make my code better.
@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
@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.
@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