london9-lovelace-html-css-challemges-Boshra-Mahmoudi#214
Conversation
| background-color: #d5f0fd; | ||
| width: 90%; | ||
| text-align: center; | ||
| border-bottom: solid 5px #d8cefe; | ||
| } | ||
|
|
||
| main { | ||
| width: 90%; | ||
| margin: auto; | ||
| background-color: #ddebf8; |
There was a problem hiding this comment.
For colors, you can use css var to define these colors at the beginning for readability. Especially, when you are reusing them multiple times later in the file. Hint: using ':root'
Improvement 1: define and use css var for colors
| border-bottom: 5px solid #d8d5fd; | ||
| } | ||
|
|
||
| @media screen and (max-width: 900px) { |
There was a problem hiding this comment.
Very nice! Making it responsive here already!
| ><img | ||
| class="color_img" | ||
| alt="Black T-shirt" | ||
| src="https://static.thcdn.com/productimg/1600/1600/13254031-1024901106882108.jpg" | ||
| /><br /> |
There was a problem hiding this comment.
I know you are trying to format it for readability. But you should try keep certain tags like into 1 line if you can. Long as it is readable. Ultimately, the more lines you have the longer it will take browser to load the files so you want to minimise things for performance. Hope this make sense :)
There was a problem hiding this comment.
first of all thank you to review my codes, i was looking why my performance is not 100% i didn't figure out is because of that. I will use local path , but about line when i save my code in Vs code ,it is just changed in this way. I will look to find out why?
| <input | ||
| type="radio" | ||
| id="balck" | ||
| name="colour" | ||
| required | ||
| />Black</label | ||
| > |
| <label for="grey" | ||
| ><img | ||
| class="color_img" | ||
| alt="Grey T-shirt" |
There was a problem hiding this comment.
Well done on adding descriptive 'alt' text to your images! Lots of people miss this or not use descriptive names. As we also need to cater this for visually impaired users who will be user screen readers.
| type="text" | ||
| id="name" | ||
| placeholder="your register name" | ||
| pattern="[Bb]oshra [Mm]ahmoudi|[Mm]ehreen [Aa]ziz|[Ss]hayan [Mm]ahnam|[Hh]ary [Ll]i" |
There was a problem hiding this comment.
Took me a while to get this working on the site. Then I figured you have put 'hary' instead of 'harry'. Just becareful on typos and spellings, especially for rules and patterns. A common but can be devastating mistake. But very nicely done getting the pattern setup!
There was a problem hiding this comment.
sorry about that, spelling is one of my weakness in coding. I should pay attention more
| required | ||
| /></label> | ||
|
|
||
| <label for="eamil">Email</label> |
There was a problem hiding this comment.
think you have a typo here 'eamil' instead of 'email'. This could be hard to spot later if we continued this exercise to build a backend. The 'for' attribute needs to be the same as the 'id' so this would have broken something when submitting the form.
There was a problem hiding this comment.
I will change as your suggestion , thanks a lot
2 participants
No description provided.