Skip to content

Conversation

@jbadan
Copy link
Contributor

@jbadan jbadan commented Jan 31, 2019

Description

See #318: too many merge conflicts, starting on fresh branch

fixes #298

@jbadan jbadan requested a review from greg-a-smith January 31, 2019 23:07
Copy link
Contributor

@bcullman bcullman left a comment

Choose a reason for hiding this comment

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

I made some edits here, so I will give partial 🚢

@jbadan has given the ok to merge on her behalf on this one, so just waiting on another 👍

Copy link
Contributor

@jeffredodd jeffredodd left a comment

Choose a reason for hiding this comment

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

Looks good to me! ⛵️

@greg-a-smith
Copy link
Contributor

@jbadan, @bcullman, @jeffredodd I pushed some changes as well. The left nav was no longer scrolling because the <nav> element was added around the two <ul> elements and the height and overflow styles were tied to the class in use by the <ul>. I split those styles out to a class for the <nav> and a new class for the <ul>. I also refactored the Routes.js to make it completely dynamically grouped and sorted.

@jeffredodd
Copy link
Contributor

Just reviewed your commits as well @greg-a-smith. Also looks good! 🚢

Copy link
Contributor

@greg-a-smith greg-a-smith left a comment

Choose a reason for hiding this comment

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

I will also give my partial ⛵️ .

@greg-a-smith greg-a-smith merged commit a16eb5c into master Feb 1, 2019
@greg-a-smith greg-a-smith deleted the chore/update-preamble branch February 1, 2019 13:15
greg-a-smith pushed a commit that referenced this pull request Mar 5, 2019
* updating with home page and new nav

* move title to readme

* cleanup list hang a bit more

* Fixed left nav scrolling issue

* Refactored the left nav to group and sort nav items
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.

Improve Docs: Add preamble pages (Home / Installation / Usage)

5 participants