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

feat(tabset): add nav justification and orientation #1239

Closed
wants to merge 1 commit into from
Closed

feat(tabset): add nav justification and orientation #1239

wants to merge 1 commit into from

Conversation

ktriek
Copy link
Contributor

@ktriek ktriek commented Jan 21, 2017

  • Nav Justification:
    • start: align left .justify-content-start
    • center: align center .justify-content-center
    • end: align right .justify-content-end
    • fill: All horizontal space will be occupied by nav links .nav-fill
    • justified: All horizontal space will be occupied by nav links and every nav item will be the same width .nav-justified
  • Nav orientation:
    • horizontal
    • vertical

Before submitting a pull request, please make sure you have at least performed the following:

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated any applicable tests.
  • added/updated any applicable API documentation.
  • added/updated any applicable demos.

if (className === 'fill' || className === 'justified') {
this.justifyClass = 'nav-' + className;
} else {
this.justifyClass = 'justify-content-' + className;
Copy link
Member

Choose a reason for hiding this comment

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

Could pretty the class setting to

this.justifyClass = `nav-${className}`;

and

this.justifyClass = `justify-content-${className}`;

Copy link
Member

@wesleycho wesleycho left a comment

Choose a reason for hiding this comment

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

LGTM

@wesleycho
Copy link
Member

Looks like this needs to be rebased, but otherwise should be good.

@JordanGS
Copy link

someone wanna rebase it and added it if it's good (assuming that's the only thing stopping the commit)? I'd do it myself if i could but @ktriek hasn't made a comment in almost 2 months.

@coccor
Copy link

coccor commented Jun 15, 2017

This seems to be a very useful feature. Thanks

@lsturtevant
Copy link

What's the status of this PR? I need this to be able to use ng-bootstrap for tabs since it finally implements missing features.

@trafalgarDWaterLaw
Copy link

Hi, when is this coming along?

This was referenced Aug 17, 2017
@pkozlowski-opensource pkozlowski-opensource added this to the 1.0.0-beta.2 milestone Aug 17, 2017
@pkozlowski-opensource
Copy link
Member

Merged, thnx a lot @ktriek !

Sorry it took as so long but we were busy with other items and waiting for Bootstrap 4 beta to see if there are no more changes coming.

Once again, thnx for the PR - we love PRs with tests, demo updates and clean code. Keep them coming!

@ktriek
Copy link
Contributor Author

ktriek commented Aug 17, 2017

Thanks @pkozlowski-opensource for your time, I really appreciate what you are doing.

rmeans pushed a commit to fcsa-teamhammer/ng-bootstrap that referenced this pull request Oct 18, 2017
@sbatezat
Copy link

sbatezat commented Sep 14, 2020

Tabset is deprecated in favor of nav and it seems that nav-justified is not working on nav.

<ul ngbNav justify="justified" class="nav-pills">
or
<ul ngbNav class="nav-pills nav-justified">

does not produce the expected behavior.

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.

None yet

8 participants