Skip to content
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

docs: fix navbar links document #221

Merged
merged 2 commits into from
May 10, 2019
Merged

Conversation

ittus
Copy link
Contributor

@ittus ittus commented May 10, 2019

Problem

In document, it said that:

interface NavItem {
  title: string
  link?: string
  // Use `links` instead of `link` to display dropdown
  links?: Array<NavItemLink>
}

interface NavItemLink {
  title: string
  link: string
}

but in code, it used item.children instead of item.links

Solution

Change item.children to item.links

@egoist
Copy link
Owner

egoist commented May 10, 2019

Let's use children instead

@ittus
Copy link
Contributor Author

ittus commented May 10, 2019

@egoist
I fixed document instead.

Btw, in sidebar, we're using links instead of children

interface SidebarItem {
  title?: string
  links: Array<SidebarItemLink>
}

Should we change links to children for consistency?

@ittus ittus changed the title Fix navbar links Fix navbar links document May 10, 2019
@egoist
Copy link
Owner

egoist commented May 10, 2019

Should we change links to children for consistency?

Yes, we can make it fallback to links for backward compatibility.

@ittus
Copy link
Contributor Author

ittus commented May 10, 2019

Sound nice! Should we do it in another pull request? In this pull request, I only fix document mistake only.

@egoist egoist changed the title Fix navbar links document docs: fix navbar links document May 10, 2019
@egoist egoist merged commit cdc593e into egoist:master May 10, 2019
@egoist
Copy link
Owner

egoist commented May 11, 2019

🎉 This PR is included in version 4.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ittus ittus deleted the bugfix/navbar-link branch May 11, 2019 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants