Skip to content

Conversation

@TahseenAlaa
Copy link

Task - 1 Done by Tahseen Alaa.
Added to Solutions Branch.

margin: 50px auto auto 50px;
text-align: center;
left: 20%;
}
Copy link
Member

Choose a reason for hiding this comment

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

header
you don't need just remote it left: 20%. Also you can use short hand property for margin: 50px auto;

}

section {
text-align: center;
Copy link
Member

Choose a reason for hiding this comment

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

useless just remote it

padding: 50px 50px 50px 50px;
left: 27%;
text-align: center;
}
Copy link
Member

Choose a reason for hiding this comment

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

backSpace
as you can see in the image below, your layout is broken!! the reason is left: 27% do you know the purpose of left property? you can't use it everywhere be careful with that. Remove position, text-align Also, use shorthand for margin: 100px auto 50px and padding: 50px
screen shot 2018-03-29 at 22 51 43

background-color: #1ECDE2;
color: #fff;
}

Copy link
Member

Choose a reason for hiding this comment

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

why you are using multiple classes for active buttons ?
you use an .active class instead of these.btn1, .btn2, .btn6, .btn11 and then use it in your button tags!

Copy link
Author

Choose a reason for hiding this comment

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

I thought I will need it later, and forget to delete them.

<div class="">
<button class="btn btn1">C</button>
</div>

Copy link
Member

Choose a reason for hiding this comment

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

remove all <div class=""> since they don't have class!

Copy link
Author

Choose a reason for hiding this comment

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

the whole code need minify before deploy as production.
thanks for the review.
I fixed all bugs.

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