Skip to content

Conversation

@Conary36
Copy link
Owner

@Conary36 Conary36 commented Sep 8, 2019

No description provided.

@Conary36 Conary36 requested review from a user and removed request for a user September 8, 2019 01:58
#vision{
float:left;
width:33.33%;
height:250px;
Copy link

Choose a reason for hiding this comment

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

So far everything looks good. I'll have to see how it turned out tomorrow. The only critique I have is a small one. Make sure your code is neat. Have everything lined up evenly and uniformed. Might seem like a petty thing to critique right now but once we start getting into LESS and even JSX you will see how important it is to have neat code.

<img class="logo" src="img/logo.png" alt="Great Idea! Company logo.">
</container>
</nav>
</container>
Copy link

Choose a reason for hiding this comment

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

As I am sure container worked as far making an actual container, a rule of thumb is the only thing called container is the class of the body. In this instance you should replace the first container with header and the second container with div.

<h1>On</h1>
<h1>Demand</h1>
<button id="started">
<p>Get Started</p>
Copy link

Choose a reason for hiding this comment

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

To save code space, you don't need to put p tags here. You could just put button and it would still work.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Other than the minor things I have mentioned, this looks great. I don't know how it turned it in the browser yet but you have a solid understanding of how to set up your CSS and HTML. Great job! Can't wait to see it in our 1:1 tomorrow.

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