Skip to content

Conversation

@EChesters
Copy link
Contributor

Addresses issue: #18

What this does

  • Adds banner to About page
  • Adds dummy text to About page
  • Moves nav inside container fluid

Screenshots

Desktop version of About page

Mobile version of About page

[x] Removed unused background sass file
@EChesters
Copy link
Contributor Author

Footer is broken, but this needs to be fixed separately.

render() {
return (
<div className="page-wrap">
<Nav />
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be addressed in the pull request for #10. Can it be removed from this pull request pretty please?


return (
<nav className="navbar navbar-fixed-top">
<nav className="navbar navbar-default">
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be addressed in the pull request for #10. Can it be removed from this pull request pretty please?

@@ -1,4 +1,8 @@
---
# These lines are for Jekyll
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these lines needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 6 dashes are a Jekyll thing, so Jekyll knows it's has to do something with the file.

background-position: top center;
background-repeat: no-repeat;
background-size: cover;
background-color: #fff;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are having a background image covering the whole view, is this background-color needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in latest commit.

@@ -0,0 +1,6 @@
@mixin background-size($size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this mixin being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now being used in latest commit.

<div className="pt-5">
<PageHeader>{ about.title }</PageHeader>
<p>{ about.lorem }</p>

Copy link
Contributor

Choose a reason for hiding this comment

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

Blank line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in latest commit.

This was referenced Apr 18, 2017
EChesters added 2 commits May 7, 2017 00:29
- Removed blank line in About.jsx
- Used the background-size mixin
@tanyapowell
Copy link
Contributor

All good this end (once the conflict has been resolved) 😸

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