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

Add margin variable for breadcrumb #23138

Merged
merged 3 commits into from
Aug 11, 2017
Merged

Add margin variable for breadcrumb #23138

merged 3 commits into from
Aug 11, 2017

Conversation

prateekgoel
Copy link
Contributor

I have added a variable for breadcrumb margin to _variables.scss and replaced static margin in _reboot.css with that variable.

Fixes #23016.

@@ -787,6 +787,8 @@ $breadcrumb-padding-y: .75rem !default;
$breadcrumb-padding-x: 1rem !default;
$breadcrumb-item-padding: .5rem !default;

$breadcrumb-margin: 0 0 1rem! default;

Choose a reason for hiding this comment

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

Invalid CSS after "... 0 0 1rem!": expected identifier, was " default;"

@@ -1,6 +1,6 @@
.breadcrumb {
padding: $breadcrumb-padding-y $breadcrumb-padding-x;
margin-bottom: 1rem;
Copy link
Member

@mdo mdo Jul 25, 2017

Choose a reason for hiding this comment

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

Leave it as margin-bottom, please. There's no need to reset a non-existant margin-top, -right, and -left if we have no intention of ever applying those values (which I don't :p).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok then i'll keep margin-bottom only.
Will do the change.

Copy link
Contributor Author

@prateekgoel prateekgoel Jul 25, 2017

Choose a reason for hiding this comment

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

@mdo Changes done.

Copy link
Contributor Author

@prateekgoel prateekgoel Aug 6, 2017

Choose a reason for hiding this comment

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

@mdo Can you please review and close this PR if we don't have any more changes?

@mdo
Copy link
Member

mdo commented Aug 7, 2017

This'll go in for Beta 2 at the earliest.

@prateekgoel
Copy link
Contributor Author

Ok Thanks @mdo

@mdo mdo merged commit 9497c21 into twbs:v4-dev Aug 11, 2017
@mdo mdo mentioned this pull request Aug 11, 2017
@prateekgoel prateekgoel deleted the breadcrumb-margin-variable branch August 13, 2017 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants