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

Grouping elements in navbar #1073

Merged
merged 2 commits into from
Dec 12, 2016
Merged

Conversation

xtuc
Copy link
Member

@xtuc xtuc commented Dec 10, 2016

Fixes #1060

Desktop:

nav-desktop

Collasped:

nav-mobile

Given #1042 this is a quick fix.

Navigation data has a bit changed:

  • actual item
  • subnav
    • name (optionnaly set a group name)
    • items
      • actual item

- name: Packages
items:
- title: API
url: /docs/core-packages/
Copy link
Member Author

Choose a reason for hiding this comment

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

API is pointing babel's core packages. We can rename it if you want.

@existentialism
Copy link
Member

existentialism commented Dec 10, 2016

As an alternative to the indention, what are your thoughts about:

image

Also, I think renaming "API" -> "Core" under Packages is a little clearer? @hzoo?

Otherwise, LGTM!

@xtuc
Copy link
Member Author

xtuc commented Dec 10, 2016

Yes, Looks better.

Have you tried with the collasped version ?

@existentialism
Copy link
Member

existentialism commented Dec 10, 2016

Yeah, we can do something similar for now... when we attack the sidebar we may have to rethink a bunch of this anyway:

image

@xtuc
Copy link
Member Author

xtuc commented Dec 10, 2016

It seems great 👍. Would you push it there so we can merge this.

@existentialism
Copy link
Member

@xtuc pushed changes, check em out

@xtuc
Copy link
Member Author

xtuc commented Dec 12, 2016

Nice job, thanks!

LGTM, can we merge this ?

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

👍

@existentialism existentialism merged commit c0c24ce into babel:master Dec 12, 2016
@hzoo
Copy link
Member

hzoo commented Dec 12, 2016

Awesome looks good.

Although now I think maybe would could rename a few things?

Should core be babel-core and polyfill babel-polyfill or is the shorthand better?

Maybe should remove the browser in usage since it redirects to setup anyway?

@existentialism
Copy link
Member

Good point re: browsers, agree we should drop.

And yeah, while I think Core and Polyfill look better, it might be better to go with babel-core and babel-polyfill to eliminate any chance someone might be confused (esp since babel-register is below, and Register would be pretty strange by itself).

I'll update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link core-packages page
3 participants