add task solution#1916
Conversation
Luk2asz
commented
Jun 14, 2023
Luk2asz
commented
- DEMO LINK
darokrk
left a comment
darokrk
left a comment
There was a problem hiding this comment.
Looks good 💪🏽 need small improvements in code
| <body> | ||
| <header class="header"> | ||
| <a class="header__logo" href=""> | ||
| <a class="header__logo" href="home"> |
There was a problem hiding this comment.
The href value for the header__logo link should start with a '/' to indicate the root directory (e.g. '/home')
| <ul class="nav__list"> | ||
| <li class="nav__item nav__item--is-active"> | ||
| <a href="" class="nav__link">Apple</a> | ||
| <a href="Apple" class="nav__link">Apple</a> |
There was a problem hiding this comment.
href attribute values should be started with lowercase e.g. apple
change this for the rest links also
| flex-wrap: wrap; | ||
|
|
||
| display: grid; | ||
| gap: 48px; |
There was a problem hiding this comment.
you can add here also justify-content: center to center all cards
| } | ||
| } | ||
|
|
||
| @media (min-width: 488px) { |
There was a problem hiding this comment.
this all @media styling should be placed in .catalog class line 6, please move them up and then you can remove .catalog class from inside, it will automatically points to this class :)
| @media (min-width: 488px) { | |
| @media (min-width: 488px) { | |
| grid-template-columns: repeat(2, 200px); | |
| } |
DorotaLeniecDev
left a comment
DorotaLeniecDev
left a comment
There was a problem hiding this comment.
Looking good, check my suggestion about links href
| <body> | ||
| <header class="header"> | ||
| <a class="header__logo" href=""> | ||
| <a class="header__logo" href="/home"> |
There was a problem hiding this comment.
For this purpose I would rather use links with hashtags, as navigation links in most cases will point to the specific sections on the page
| <a class="header__logo" href="/home"> | |
| <a class="header__logo" href="#home"> |
3 participants
