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

v4: Simplify navbar further #21462

Merged
merged 18 commits into from
Dec 29, 2016
Merged

v4: Simplify navbar further #21462

merged 18 commits into from
Dec 29, 2016

Conversation

mdo
Copy link
Member

@mdo mdo commented Dec 29, 2016

This PR aims to do a bit more simplification and improvements to the navbar. It addresses the list of todos from #21460 as a follow-up to redoing the navs with flexbox.

  • Makes the .navbar use flex and flex-direction: column to start for fewer overrides and easier mobile-first styling.
  • Reorganizes the entire file for a more logical order, plus adds a contents list at the top.
  • Removes the need to use .nav on our .navbar-nav.
  • Removes the flex-grow: 10 that was rather random from .navbar-nav and uses .mr-auto for easy spacing of flex items. (This was recently added to our flex utils docs.)
  • Clarifies some class names and directions in the docs.
  • Updates all the examples.

mdo added 16 commits December 28, 2016 16:36
it's important, yo
since we're no longer using the .nav as a base class, we need to bring over some base styles for redoing browser list styles and setting flex in motion.

also brings with it some .nav-link styling. we're still using this global class, but with this small modification for alignment of content in responsive modes.
- since we're column to start, need to set row.
- note that flex-direction cannot be inherited, so we have to set it twice.
- apply the horizontal padding again to .nav-link.
- remove the .nav-item styles (un-needed).
- remove previous .nav-link styles as they were un-nested and potentially problematic in old placement should someone mix more navs in here.
this was mad confusing for awhile
.navbar-toggler {
align-self: flex-start; // Prevent toggler from growing to full width when it's the only visible navbar child

Choose a reason for hiding this comment

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

Properties should be ordered padding, font-size, line-height, background, border, align-self

list-style: none;

.nav-link {
padding-left: 0;

Choose a reason for hiding this comment

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

Properties should be ordered padding-right, padding-left

@mdo mdo merged commit 4449167 into v4-dev Dec 29, 2016
@mdo mdo deleted the navbar-tweaks branch December 29, 2016 03:07
@mdo mdo mentioned this pull request Dec 29, 2016
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.

2 participants