Skip to content

Tibo#1

Open
AlexandraKoz wants to merge 10 commits intomasterfrom
tibo
Open

Tibo#1
AlexandraKoz wants to merge 10 commits intomasterfrom
tibo

Conversation

@AlexandraKoz
Copy link
Owner

No description provided.

Copy link

@olga-arkhipenko olga-arkhipenko left a comment

Choose a reason for hiding this comment

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

Do more commits. Read about margins and paddings, seems to me, u don't understand the difference.
Restructuring styles folders required.

index.html Outdated
<link rel="stylesheet" type="text/css" href="./styles/send-message-form.css">
<link rel="stylesheet" type="text/css" href="./styles/footer.css">
<link rel="stylesheet" type="text/css" href="./styles for icons/fontawesome-all.css">
<link rel="stylesheet" type="text/css" href="./styles for layout/reset.css">

Choose a reason for hiding this comment

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

styles for layout? Do u have any other styles in this project?
It's not an appropriate folder name.

index.html Outdated
<link rel="stylesheet" type="text/css" href="./styles/team.css">
<link rel="stylesheet" type="text/css" href="./styles/send-message-form.css">
<link rel="stylesheet" type="text/css" href="./styles/footer.css">
<link rel="stylesheet" type="text/css" href="./styles for icons/fontawesome-all.css">

Choose a reason for hiding this comment

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

U already have a "styles for layout" folder, what do u think about merging them?

index.html Outdated
<li class="navigation__button"><a href="#">work</a></li>
<li class="navigation__button"><a href="#">about</a></li>
<li class="navigation__button"><a href="#">home</a></li>
<li class="navigation__item"><a href="#">contact</a></li>

Choose a reason for hiding this comment

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

Split it into separate lines - each tag on the next line, it will be more readable.

Choose a reason for hiding this comment

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

Class name should no describe what element looks like.
Read here

Choose a reason for hiding this comment

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

Why CAPS here?

index.html Outdated
<div class="news__do-you-like-rabbits-text">DO YOU LIKE RABBITS?</div>
<div class="news__do-you-like-oranges-text">DO YOU LIKE ORANGES?</div>
<section class="advertising">
<div class="advertising__first-transparent-text">DO YOU LIKE RABBITS?</div>

Choose a reason for hiding this comment

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

NO.

index.html Outdated
<div class="news__do-you-like-oranges-text">DO YOU LIKE ORANGES?</div>
<section class="advertising">
<div class="advertising__first-transparent-text">DO YOU LIKE RABBITS?</div>
<div class="advertising__second-transparent-text">DO YOU LIKE ORANGES?</div>

Choose a reason for hiding this comment

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

Class name should no describe what element looks like.


grid-template-columns: 33.333% 33.333% 33.333%;
grid-template-rows: 33.333% 33.333% 33.333%;
}

Choose a reason for hiding this comment

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

All you need is to center it according the whole page, and then center every of the animals according its box.

@@ -1,14 +1,24 @@
body {
body

Choose a reason for hiding this comment

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

That is not reset.css.
Please just use normal reset.css, don't write it yourself.

body {
body
{
max-width: 1400px;

Choose a reason for hiding this comment

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

Why do u limit body width?
What should people with big device width see: anything like this or maybe this?

@@ -1,14 +1,24 @@
body {
body

Choose a reason for hiding this comment

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

And yes, it should not be in the same folder with blocks.

@@ -0,0 +1,76 @@
.send-message
{
max-width: 1200px;

Choose a reason for hiding this comment

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

The problem with this max-width (why not width?) is that: if it changes u'll need to change it in every block.

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