Develop #4916
Develop #4916
Conversation
Malvina-9
commented
Apr 30, 2024
•
Malvina-9
commented
- DEMO LINK
- TEST REPORT LINK
src/index.html
Outdated
| href="https://fonts.gstatic.com" | ||
| /> | ||
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap" |
There was a problem hiding this comment.
Evere font-weight here it's separate file for browser to download, so always include only those which you actually use. In code you have font-weight: 500 and 400 which is default for text.
src/style.css
Outdated
| } | ||
|
|
||
| .menu__link:hover { | ||
| color: #00acdc; |
There was a problem hiding this comment.
Please, see the checklist
CSS Variable is used for a blue color
move it to a css variable
src/index.html
Outdated
| class="menu__link menu__link:hover is-active" | ||
| href="index.html" | ||
| > | ||
| APPLE |
There was a problem hiding this comment.
Small note
CSS is used to show all letters in Uppercase (don't type them in HTML)
| box-sizing: border-box; | ||
| } | ||
|
|
||
| .header { |
There was a problem hiding this comment.
Content is vertically centered (for any header height)
I believe logo also should be centered
src/index.html
Outdated
| <a href="index.html"> | ||
| <img | ||
| src="images/logo.png" | ||
| alt="moyo_1" |
There was a problem hiding this comment.
alt attribute should describe the image content.
src/style.css
Outdated
| @@ -1,3 +1,82 @@ | |||
| * { | |||
There was a problem hiding this comment.
Don't use * selector (it impacts performance)
Better use html or body tag.
src/style.css
Outdated
| } | ||
|
|
||
| .header { | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
If you set this value for the whole page, it is better not to duplicate it.
src/style.css
Outdated
| width: 100%; | ||
| bottom: 0; | ||
| height: 4px; | ||
| background: #00acdc; |
There was a problem hiding this comment.
Let's use variables for repeating properties.
src/index.html
Outdated
| <ul class="menu__list"> | ||
| <li class="menu__item"> | ||
| <a | ||
| class="menu__link menu__link:hover is-active" |
There was a problem hiding this comment.
Use hover effect in css.
| class="menu__link menu__link:hover is-active" | |
| class="menu__link is-active" |
| </li> | ||
| <li class="menu__item"> | ||
| <a | ||
| class="menu__link menu__link:hover" |
There was a problem hiding this comment.
| class="menu__link menu__link:hover" | |
| class="menu__link" |
