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

BREAKING(Tab): allow right/left without tabular #2265

Merged

Conversation

ryanmcgarvey
Copy link
Contributor

@ryanmcgarvey ryanmcgarvey commented Oct 27, 2017

BREAKING CHANGE

It is currently not possible to align vertical tabs to the left or right unless you use the tabular prop. This PR allows left/right vertical tab alignment without requiring the tabular menu variation.

Before

Right aligned tabs required the tabular prop and therefore the tabular menu variation.

const panes = [ ...{} ]
const menu = {
  fluid: true,
  vertical: true,
  tabular: 'right',
 }

<Tab menu={menu} panes={panes} />

image

After

Right aligned tabs no longer require the tabular prop and can be used with any menu variation.

const panes = [ ...{} ]
const menu = {
  fluid: true,
  vertical: true,
  secondary: true,
  aligned: 'right'
}

<Tab menu={menu} panes={panes} />

image


This allows tab menus to be right aligned without having the tabular styles set.

Resolves #2264

@codecov-io
Copy link

Codecov Report

Merging #2265 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2265   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files         151      151           
  Lines        2624     2624           
=======================================
  Hits         2617     2617           
  Misses          7        7
Impacted Files Coverage Δ
src/modules/Tab/Tab.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01b02f7...5d37099. Read the comment docs.

@levithomason
Copy link
Member

Thanks for the contribution! Could you state the problem that this is solving? I'd like to be sure I understand before merging.

@ryanmcgarvey
Copy link
Contributor Author

@levithomason this PR Resolves #2264. I'm still working on creating a sandbox testcase, but I'm not sure how to do it with my fork.

@brianespinosa
Copy link
Member

@ryanmcgarvey please take a moment to review the pen I forked for you on #2264 as I think it resolves what you were hoping to achieve.

@levithomason
Copy link
Member

I'm awaiting a completed issue template as well. See original issue.

@ryanmcgarvey
Copy link
Contributor Author

@levithomason @brianespinosa

I've updated the original issue as per the template. Is the problem I'm trying to solve more clean now?

Resolves #2264

@levithomason levithomason changed the title Feature/tab right aligned BREAKING(Tab): allow right/left without tabular Nov 25, 2017
@levithomason
Copy link
Member

@ryanmcgarvey very much appreciated. I get it now and agree. We should not couple the left/right alignment to the tabular prop. They can be used separately.

This is a breaking change, so I've updated the description to reflect that. Thanks for the update.

@levithomason levithomason merged commit 93c09a1 into Semantic-Org:master Nov 25, 2017
@levithomason
Copy link
Member

Released in [email protected]

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.

4 participants