vinhnglx / active_bootstrap_skin

Bootstrap skin for Active Admin :rocket: :rocket: :rocket:
MIT License
134 stars 28 forks source link

#wrapper div padding 5px #10

Closed tbuyle closed 8 years ago

tbuyle commented 8 years ago

Whats the purpose of the 5px left and right padding applied to all div inside #wrapper ?

It adds various padding inside the admin (a table index has about 25px padding around the actual table) and mess up with bootstrap grid.

It's added on top of the scss file :

#wrapper {
  ...
  div {
    padding-left: 5px;
    padding-right: 5px;
  }
}
vinhnglx commented 8 years ago

@tbuyle just wanna custom some styles. I know it's not good, but I don't have time to upgrade this to a component, and you can pass the padding that you want.

I'll appreciate your help for this issue :)

tbuyle commented 8 years ago

I'm asking because this simple rule is breaking various grid compoment / alignment.

On my project, I had to add the following rules to revert it back (and I don't think they cover everything).

#wrapper {
  div {
    padding-left: initial;
    padding-right: initial;
  }
  .alert, .flash, .container, .col-xs-1, .col-sm-1, .col-md-1, .col-lg-1, .col-xs-2, .col-sm-2, .col-md-2, .col-lg-2, .col-xs-3, .col-sm-3, #header .header-item.tab, .col-md-3, #active_admin_content.with_sidebar #sidebar, .col-lg-3, .col-xs-4, .col-sm-4, .col-md-4, .col-lg-4, .col-xs-5, .col-sm-5, .col-md-5, .col-lg-5, .col-xs-6, .col-sm-6, .col-md-6, #active_admin_content.without_sidebar #main_content_wrapper, .col-lg-6, .col-xs-7, .col-sm-7, .col-md-7, .col-lg-7, .col-xs-8, .col-sm-8, .col-md-8, .col-lg-8, .col-xs-9, .col-sm-9, .col-md-9, #active_admin_content.with_sidebar #main_content_wrapper, .col-lg-9, .col-xs-10, .col-sm-10, .col-md-10, .col-lg-10, .col-xs-11, .col-sm-11, .col-md-11, .col-lg-11, .col-xs-12, .col-sm-12, .col-md-12, .col-lg-12 {
    padding-left: 15px;
    padding-right: 15px;
  }
}

I think it would be better to remove the 5px rule from active_bootstrap_skin and simply add it on top of the active_admin.scss of your(s) application(s) if needed.

vinhnglx commented 8 years ago

Agree with you, but currently, I don't have time to update. Can you help me make a PR for this? Define a mixin to set padding for the element. You will pass the padding number to mixin if needed.

On Tue, Aug 2, 2016 at 4:50 PM, Thomas notifications@github.com wrote:

I'm asking because this simple rule is breaking various grid compoment / alignment.

On my project, I had to add the following rules to revert it back (and I don't think they cover everything).

wrapper {

div { padding-left: initial; padding-right: initial; } .alert, .flash, .container, .col-xs-1, .col-sm-1, .col-md-1, .col-lg-1, .col-xs-2, .col-sm-2, .col-md-2, .col-lg-2, .col-xs-3, .col-sm-3, #header .header-item.tab, .col-md-3, #active_admin_content.with_sidebar #sidebar, .col-lg-3, .col-xs-4, .col-sm-4, .col-md-4, .col-lg-4, .col-xs-5, .col-sm-5, .col-md-5, .col-lg-5, .col-xs-6, .col-sm-6, .col-md-6, #active_admin_content.without_sidebar #main_content_wrapper, .col-lg-6, .col-xs-7, .col-sm-7, .col-md-7, .col-lg-7, .col-xs-8, .col-sm-8, .col-md-8, .col-lg-8, .col-xs-9, .col-sm-9, .col-md-9, #active_admin_content.with_sidebar #main_content_wrapper, .col-lg-9, .col-xs-10, .col-sm-10, .col-md-10, .col-lg-10, .col-xs-11, .col-sm-11, .col-md-11, .col-lg-11, .col-xs-12, .col-sm-12, .col-md-12, .col-lg-12 { padding-left: 15px; padding-right: 15px; } }

I think it would be better to remove the rule from active_bootstrap_skin and simply add it on top of the active_admin.scss of your(s) application(s) if needed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vinhnglx/active_bootstrap_skin/issues/10#issuecomment-236843519, or mute the thread https://github.com/notifications/unsubscribe-auth/AB55UXuei_133WEILVLCGBsBZ99UFrW8ks5qbwTxgaJpZM4JA83v .

Ruby Developer Betacode Sdn Bhd (1109222-K) Phone: (+60) 167557808 Skype: vinhnglx Email: vinh.nglx@gmail.com

tbuyle commented 8 years ago

Actually, I don't see why this rule (with custom value set by mixin) would be necessary at all.

Adding padding to all div inside #wrapper is doomed to break Bootstrap grid at some point.

If the default 15px padding needs to be changed, the grid-gutter-width variable is what should be changed in sass (or less) file.

vinhnglx commented 8 years ago

Oh. I see, thank you for pointing me.

On Tue, Aug 2, 2016 at 5:10 PM, Thomas notifications@github.com wrote:

Actually, I don't see why this rule (with custom value set by mixin) would be necessary at all.

Adding padding to all div inside #wrapper is doomed to break Bootstrap grid as some point.

If the default 15px padding needs to be changed, the grid-gutter-width variable is what should be changedin sass (or less) file.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vinhnglx/active_bootstrap_skin/issues/10#issuecomment-236848082, or mute the thread https://github.com/notifications/unsubscribe-auth/AB55UWDV-6PpCQMZmTpPX1HmoueCf_N-ks5qbwl2gaJpZM4JA83v .

Ruby Developer Betacode Sdn Bhd (1109222-K) Phone: (+60) 167557808 Skype: vinhnglx Email: vinh.nglx@gmail.com

vinhnglx commented 8 years ago

@tbuyle just released new gem version with your PR. Thank you so much :)

tbuyle commented 8 years ago

You're welcome.