Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add moyo header #4689

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add moyo header #4689

wants to merge 6 commits into from

Conversation

Follder
Copy link

@Follder Follder commented Mar 6, 2024

Unfortunally, I created pull request to the master branch in my account, and then merged these branches.
May I create new pull request to the master branch ma from my master?

Copy link

@DanilWeda DanilWeda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine.
Add a hover effect to all navigation elements. (now only one navigation element has it )
You can create pull from any branch.
Thanks!

@Follder Follder requested a review from DanilWeda March 11, 2024 16:04
Copy link

@alexander-ignatow alexander-ignatow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 As for your question - as I see it, yes, it should work

  1. On your code - please add the hover effect, as @DanilWeda mentioned
    Per task: link color should be changed on :hover

@Follder
Copy link
Author

Follder commented Mar 12, 2024

1 As for your question - as I see it, yes, it should work

  1. On your code - please add the hover effect, as @DanilWeda mentioned
    Per task: link color should be changed on :hover

But I added hover effect in the last commit, as
[data-qa]:hover,
.nav__link:hover {
transition: .3s;
color: var(--main-color)
}

@Follder
Copy link
Author

Follder commented Mar 12, 2024

Sorry, now everything might be ok

  • DEMO LINK

  • TEST REPORT LINK

  • Header height is set in 1 place (for the links)

  • Content is vertically centered (for any header height)

  • CSS is used to show all letters in Uppercase (don't type them in HTML)

  • Logo is an image wrapped with a link

  • CSS Variable is used for a blue color

  • Pseudo-element is used for a blue line below the active link

  • Code follows all the Code Style Rules ❗️

Comment on lines +30 to +36
font-size: 12px;
font-weight: 500;
display: flex;
text-transform: uppercase;
list-style-type: none;
margin: 0;
padding: 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, order your css properties in a better way

Comment on lines +71 to 74
.nav__link:hover {
transition: 0.3s;
color: var(--main-color);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move it closer to the .nav__link selector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants