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 solution #5348

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

add solution #5348

wants to merge 4 commits into from

Conversation

izotovav
Copy link

@izotovav izotovav commented Sep 9, 2024

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

Good job 👍
Let's improve your page

  1. Fix the hover effects for all links, you need to add a blue color for text and remove border
image
  1. All links must be 60px high
image

src/index.html Outdated
<h1>Moyo header</h1>
<header class="header">
<a
class="logo"

Choose a reason for hiding this comment

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

Suggested change
class="logo"
class="header__link"

src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated
Comment on lines 82 to 85
<main>
<div class="div"></div>
</main>
<footer></footer>

Choose a reason for hiding this comment

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

Redundant

Suggested change
<main>
<div class="div"></div>
</main>
<footer></footer>

Comment on lines +6 to +8
font-family: Roboto, sans-serif;
font-size: 12px;
font-weight: 500;

Choose a reason for hiding this comment

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

Move font styles to the html selector

src/style.css Outdated
Comment on lines 78 to 81
div:hover::after {
outline: solid 1px;
text-decoration-color: var(--active-color);
}

Choose a reason for hiding this comment

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

Suggested change
div:hover::after {
outline: solid 1px;
text-decoration-color: var(--active-color);
}

src/style.css Outdated
display: block;
height: 4px;
width: 37px;
background-color: #00acdc;

Choose a reason for hiding this comment

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

Create a variable for repeated colors

src/style.css Outdated
content: '';
display: block;
height: 4px;
width: 37px;

Choose a reason for hiding this comment

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

Suggested change
width: 37px;
width: 100%;

src/style.css Outdated
}

.logo {
position: absolut;

Choose a reason for hiding this comment

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

Suggested change
position: absolut;

src/style.css Outdated
color: var(--active-color);
}

.data-qa:hover {

Choose a reason for hiding this comment

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

Suggested change
.data-qa:hover {
.nav__link:hover {

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

Almost done!

src/index.html Outdated
<nav class="nav">
<ul class="nav__list">
<li class="nav__item">
<a class="nav__link color is-active">Apple</a>

Choose a reason for hiding this comment

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

Suggested change
<a class="nav__link color is-active">Apple</a>
<a class="nav__link is-active">Apple</a>

src/style.css Outdated
@@ -1,3 +1,80 @@
body {
margin: 0;

--active-color: #00acdc;

Choose a reason for hiding this comment

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

Create a variable with root selector

src/style.css Outdated
Comment on lines 78 to 80
.header:hover {
outline: solid 3px var(--active-color);
}

Choose a reason for hiding this comment

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

Suggested change
.header:hover {
outline: solid 3px var(--active-color);
}

src/style.css Outdated
color: var(--active-color);
}

a:hover {

Choose a reason for hiding this comment

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

Use class selector here

Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

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

Good job! 👍

src/index.html Outdated
Comment on lines 41 to 42
</a>
<nav class="nav">

Choose a reason for hiding this comment

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

Suggested change
</a>
<nav class="nav">
</a>
<nav class="nav">

src/index.html Outdated
Comment on lines 50 to 51
</li>
<li class="nav__item">

Choose a reason for hiding this comment

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

Suggested change
</li>
<li class="nav__item">
</li>
<li class="nav__item">

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.

2 participants