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

Disable negative margins by default #30585

Merged
merged 3 commits into from Apr 15, 2020
Merged

Conversation

MartijnCuppens
Copy link
Member

Negative margins were introduced to create/hack responsive gutter widths in BS 4.2. But since we now have the responsive gutter, those classes are now less applicable. Also, since we now have an xxl breakpoint, our file size has increased quite a lot, since there are lots of negative margins.

Negative margins can be useful, but I don't think we should enable them by default, therefore using a $enable- variable feels like a good idea.

@MartijnCuppens MartijnCuppens requested a review from a team as a code owner April 14, 2020 19:53
@MartijnCuppens MartijnCuppens added this to Inbox in v5 via automation Apr 14, 2020
scss/_variables.scss Outdated Show resolved Hide resolved
v5 automation moved this from Inbox to Approved Apr 14, 2020
@XhmikosR XhmikosR merged commit 5c2a50f into master Apr 15, 2020
v5 automation moved this from Approved to Shipped Apr 15, 2020
@XhmikosR XhmikosR deleted the master-mc-disable-negative-margins branch April 15, 2020 04:36
@XhmikosR
Copy link
Member

@mdo @MartijnCuppens maybe we should reconsider this patch? I mean, I know it saves us a lot from the dist files, but it's something that we have in v4 and it seems like a good thing to have by default.

Assuming we'll proceed with using CSS vars more, the dist size will drop anyway.

@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Apr 26, 2020

@XhmikosR, opened the discussion and commented my point of view here: #30649

olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
* Disable negative margins by default

* Use shorter enable variable

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

3 participants