Develop #93
Develop #93
Conversation
src/style.css
Outdated
| } | ||
|
|
||
| .nav__list:hover { | ||
| color: aqua; |
There was a problem hiding this comment.
That's not the right color. Please take color from Figma design.
There was a problem hiding this comment.
Okay, changed color to #00acdc per figma design
There was a problem hiding this comment.
As far as tests, I've never been able to get them to pass properly even on the simplest of tasks. I'll ask in chat
src/style.css
Outdated
|
|
||
| .is-active { | ||
| color: aqua; | ||
| text-decoration: underline; |
There was a problem hiding this comment.
It should be done with ::after and without text-decoration: underlane.
There was a problem hiding this comment.
Used bottom-border instead, see change. I'm unclear how pseudo-element ::after is supposed to work, haven't gone over this in lectures.
src/index.html
Outdated
| <li class="nav__list">Photo</li> | ||
| <li class="nav__list">Video</li> | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
nesting for this task , follow below format
<body>
<header class="header">
<a href="#" class="logo">
<img
src=""
alt=""
class=""
>
</a>
<nav class="nav">
<ul class="nav__list">
<li class="nav__item">
<a href="#" class="nav__link is-active">Apple</a>
</li>
<li class="nav__item">
<a href="#" class="nav__link">Samsung</a>
</li>
.........
<li class="nav__item">
<a href="#" class="nav__link">Video</a>
</li>
</ul>
</nav>
</header>
</body>
There was a problem hiding this comment.
I've altered the HTML accordingly. See latest revision and tell me what you think
Dynamizx
requested review from
amitpatiljc and
qoa11a
and removed request for
amitpatiljc and
qoa11a
src/style.css
Outdated
| .logo { | ||
| margin-left: 2.5%; | ||
| flex-grow: 3; | ||
| } |
There was a problem hiding this comment.
- you wont require both
margin-leftandflex-growafter you are done with suggestion forheaderclass - add height and width desired for the logo here
There was a problem hiding this comment.
Removed and added height, width 40px. Cool, it still works
src/style.css
Outdated
| .nav__list { | ||
| display: inline-flex; | ||
| justify-content: space-evenly; | ||
| list-style: none; | ||
| } |
There was a problem hiding this comment.
| .nav__list { | |
| display: inline-flex; | |
| justify-content: space-evenly; | |
| list-style: none; | |
| } | |
| .nav__list { | |
| display: flex; | |
| list-style: none; | |
| } | |
| } |
this will fix the alignment
src/style.css
Outdated
| } | ||
|
|
||
| .is-active { | ||
| color: #00acdc; |
There was a problem hiding this comment.
use ::after instead of border-bottom and have position: absolute and make it position according to nav__link (for that you have to give nav__link position: relative
There was a problem hiding this comment.
Unsure how to get this pseudo-element to work as it should, even when I copy the figma mockup directly I see no change
| .nav__list { | ||
| display: flex; | ||
| list-style: none; | ||
| } |
There was a problem hiding this comment.
you have to reset default paddings/margins for ul
| .nav__link::after { | ||
| position: absolute; | ||
| width: 37px; | ||
| height: 4px; | ||
| left: 509px; | ||
| top: 56px; | ||
| background: #00acdc; | ||
| border-radius: 8px; | ||
| /* padding-bottom: 20px; */ | ||
| /* border-bottom: 4px solid #00acdc; */ | ||
| } | ||
|
|
||
| /* .is-active {} */ |
There was a problem hiding this comment.
- these styles should be applied to
.is-active, not.nav__link - to make that blue dash appear, add
content: ""line in your styles
NB: do not just copy-paste styles from Figma, those are just auto-generated, literally hardcoded
some of them are required (colors, fonts, line-heights, paddings, etc.), some of them should not be copy-pasted (position, etc.)
| @@ -6,9 +6,42 @@ | |||
| content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"> | |||
| <meta http-equiv="X-UA-Compatible" content="ie=edge"> | |||
| <title>Moyo header</title> | |||
| <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Roboto"> | |||
There was a problem hiding this comment.
you've imported the font, but there is not font-weight, so you text does not match
should be something like this one
https://fonts.googleapis.com/css2?family=Roboto:wght@500&display=swap
https://mfckr.github.io/layout_moyo-header-en/
https://mfckr.github.io/layout_moyo-header-en/report/html_report/