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

Side Nav component #827

Merged
merged 12 commits into from
Jul 22, 2019
Merged

Side Nav component #827

merged 12 commits into from
Jul 22, 2019

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Jun 21, 2019

This adds a "Side Nav" component to Primer CSS.

Basic With different content types
image image

Basic markup

<nav class="SideNav">
  <a class="SideNav-item" href="#url">Account</a>
  <a class="SideNav-item" href="#url" aria-current="page">Profile</a>
  <a class="SideNav-item" href="#url">Emails</a>
  <a class="SideNav-item" href="#url">Notifications</a>
</nav>

Utility and inline styles are optional and depend a bit where the component gets used. There could be modifier classes, but using utilities seems just as simple and allows for more flexibility without the maintenance cost.

Next steps

  • Create component
  • Add documentation
  • Test on .com
  • Change Sub Nav text to blue
  • Cleanup
  • Finalize documentation

.com tests


Issue: https://github.com/github/design-systems/issues/624

@vercel
Copy link

vercel bot commented Jun 21, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@vercel vercel bot temporarily deployed to staging June 27, 2019 07:51 Inactive
Now there is just a single .SideNav component, but with 2 kinds of items.
@simurai simurai marked this pull request as ready for review July 2, 2019 08:07
@simurai simurai mentioned this pull request Jul 2, 2019
16 tasks
@simurai
Copy link
Contributor Author

simurai commented Jul 4, 2019

After feedback from @zainkho + @emilybrick, here the sub nav with a white background. It's only done with bg-white in the docs example. So that the sub nav could still be used stand-alone:

Nested Stand alone
image image

Maybe we can start "baking" it into the component once we have 1-2 more examples that use a sub nav.

@shawnbot shawnbot self-requested a review July 15, 2019 21:01
@broccolini
Copy link
Member

here the sub nav with a white background.

@simurai Oh interesting, so is the intention to always have a white background when the section expands to show level 2 items? I can see how that might help make what is active more clear. Will be good to see in dotcom and see how it feels!

Co-Authored-By: Shawn Allen <shawn.allen@github.com>
@vercel vercel bot temporarily deployed to staging July 22, 2019 02:37 Inactive
Co-Authored-By: Shawn Allen <shawn.allen@github.com>
Copy link
Contributor

@shawnbot shawnbot left a comment

Choose a reason for hiding this comment

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

This is great. Let's ship it! 🚀

@simurai simurai changed the base branch from master to release-12.5.0 July 22, 2019 23:27
@simurai simurai merged commit ed7f326 into release-12.5.0 Jul 22, 2019
@simurai simurai deleted the sidenav branch July 22, 2019 23:28
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.

3 participants