Skip to content

initial commit - Ari Mercado#175

Closed
ari7946 wants to merge 4 commits intobloominstituteoftechnology:masterfrom
ari7946:master
Closed

initial commit - Ari Mercado#175
ari7946 wants to merge 4 commits intobloominstituteoftechnology:masterfrom
ari7946:master

Conversation

@ari7946
Copy link

@ari7946 ari7946 commented Apr 2, 2018

No description provided.

@designerexpert
Copy link

Great job on the solution, I think you have a good grasp on the concept, and I like that you have implemented a design that is adaptive.

A few suggestions:
On the container having a max-width: 1000px; instead of a width property will limit the page scaling out of proportion on larger resolutions.

And working with the widths a bit will get those items aligned properly.

Great job! I can't wait to see what you do today with flexbox 👍

@designerexpert
Copy link

Great job on implementing a separate CSS file for flexbox and linking that in the HTML, I can see that you implemented flex-box well and are getting the grasp on the concepts taught during lecture.

A few suggestions:
(1) For a final polished look, i recommend removing the red borders around the sections on the middle of the page.
(2) To align the button underneath the text that contains it I suggest targeting the text itself and displaying as flex, then setting the direction to column, and aligning the items to the center, like so

/* suggested code */
.cta .cta-text {
  display: flex;
  flex-direction: column;
  align-items: center;
}

I think if you make those changes this project would get 3 stars out of 3 from me 👍


<link href="https://fonts.googleapis.com/css?family=Bangers|Titillium+Web" rel="stylesheet">
<link rel="stylesheet" href="css/index.css">
<link rel="stylesheet" href="css/flexbox.css">

Choose a reason for hiding this comment

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

Nice job updating the index.css to flexbox.css 🎉

@mixelpixel mixelpixel closed this May 7, 2018
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.

3 participants