Skip to content

Add support for configurable navbar title links. #1704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 7, 2018
Merged

Conversation

baconmania
Copy link
Contributor

@baconmania baconmania commented Feb 2, 2018

Looks like
image

This setup defaults to having the nav header title link back to the Dashboard (as is the case today) if no links are configured.

Might have to tweak the CSS class setup slightly, since the Admin dropdown has squared corners, but this one's got rounded corners.

@tpetr
Copy link
Contributor

tpetr commented Feb 4, 2018

yaaaas

@ssalinas
Copy link
Member

ssalinas commented Feb 5, 2018

Will let @kwm4385 and the more-UI-inclined take a look at the react bits, but I agree on making this and the admin dropdown match

Copy link
Contributor

@kwm4385 kwm4385 left a comment

Choose a reason for hiding this comment

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

Made a few changes to get everything working right and for styling. Highlights below

@@ -35,6 +35,26 @@ function isActive(navbarPath, fragment) {
// put into page wrapper, render children
const Navigation = (props) => {
const fragment = props.location.pathname.split('/')[1];
const navTitle = config.navTitleLinks
? (
<ul className="nav navbar-nav">
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrapping this in the same ul structure will give the consistent menu look you seek.

@@ -35,6 +35,26 @@ function isActive(navbarPath, fragment) {
// put into page wrapper, render children
const Navigation = (props) => {
const fragment = props.location.pathname.split('/')[1];
const navTitle = config.navTitleLinks
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed a null check for the config field itself somewhere, otherwise it would break if it wasn't set.

{config.title} <span className="caret" />
</a>
<ul className="dropdown-menu">
{Object.keys(config.navTitleLinks).map((linkTitle, index) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

These links can be inlined

@kwm4385
Copy link
Contributor

kwm4385 commented Feb 5, 2018

image

@ssalinas
Copy link
Member

ssalinas commented Feb 5, 2018

🚢

@ssalinas ssalinas merged commit b7cb83c into master Feb 7, 2018
@ssalinas ssalinas deleted the nav-title-links branch February 7, 2018 14:09
@ssalinas ssalinas added this to the 0.19.0 milestone Feb 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.

4 participants