Skip to content

Conversation

@MarioAraque
Copy link

I want to contribute refactoring some code. Bourbon give us some addons like padding and margin that I've added in your code in mayor files.

Hope it helps, and congrats for your style framework.

@twetzel
Copy link
Member

twetzel commented Sep 3, 2014

Hi Mario,

Thanks for your help and sorry I was really busy last days.

can you give an example of an advantage of that helpers?
I have to admit I like margin: 5px 10px; more than @include margin( 5px 10px );.
the usual css style is much shorter and also the output of all 4 values, is in my opinion a lot of useless css.

did you ever had trouble with margin: 5px 10px 3px; in some browser ?
If not I would prefer to go with the normal style of padding and margin.

there are some margin and padding helper in magic-styles too and I would prefer them, because thats also the most usual use case for the bourbon helpers comes in my mind

Again big thanks for contributing! Do you have any other impressions for feature requests?

My next steps will be:

  • I want to make a big cleanup in future, so put together classes, remove unneeded styles and so on
  • make compatible with new bootstrap-sass gem or make me own bootstrap-sass clone
    (the new place outside assets folder kills functionality to overwrite)
  • new styles and features

this project only grows with my other projects and only when there is some time left .. so its really slow process

@MarioAraque
Copy link
Author

Well, I prefer to use Bourbon add-ons like @Margin, @padding, or @position because It's "bourbon code". I think if I am using Bourbon, I have to use its mixins. Of course is your decision.

About your helpers, its a valid approach, but remember that Bourbon provides same functionality. It's no necessary to remove and use Bourbon, but it's important to know because there are some mixins that, maybe, we don't know or use.

I like your intention of make a cleanup in the future, if you need help or something we can get in touch and talk about that.

Regards.

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