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

Fixing padding issues #6551

Merged
merged 1 commit into from
Jan 12, 2015
Merged

Conversation

downzer0
Copy link
Contributor

This addresses the padding issues, @talbs @andy-armstrong @cahrens.

@@ -1098,6 +1098,15 @@
@include padding($baseline $baseline 0 $baseline);
border: 1px solid $gray-l5;
}

.padded-content {
padding-left: $baseline !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Woah! Why the !important? Any way you can scope this with a more descriptive selector or its order in the cascade?

@downzer0 downzer0 force-pushed the clrux/cohorted-courseware-ui-fix branch from 0238a94 to 9a82cfa Compare January 12, 2015 14:53
@@ -560,6 +560,10 @@

.form-actions {
@include padding($baseline 0 $baseline 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: no need to have all 4 values in this shorthand.

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: no need to use the RTL include padding() mixin here (since all values are equal).

@talbs
Copy link
Contributor

talbs commented Jan 12, 2015

@clrux, thanks for the revisions. Aside from my small nits on RTL, this looks good.

Once you clean those up, 👍

@downzer0 downzer0 force-pushed the clrux/cohorted-courseware-ui-fix branch from 9a82cfa to 360324e Compare January 12, 2015 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants