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

Switch to custom properties to control grid gutter widths #30475

Merged
merged 2 commits into from
Mar 30, 2020

Conversation

MartijnCuppens
Copy link
Member

Not sure yet how we should continue with the grid, so I'm just gonna make some improvements step by step which are easier to review/merge (unlike #28517 😛). In this PR I've switched to CSS custom properties which makes it easier to control the gutter width and saves a lot of CSS.

I've also moved the loop to save some media query prints.

CSS diff see: https://www.diffchecker.com/XRJUJMzm

@ffoodd
Copy link
Member

ffoodd commented Mar 30, 2020

LGTM.

Since we're using more and more custom properties, we'll be using more and more calc() function: shouldn't we allow them in stylelint from now on?

Unrelated thing, there's a flexbug fix L68 which only targets IE10-11. I'll do a PR latter.

@MartijnCuppens
Copy link
Member Author

Since we're using more and more custom properties, we'll be using more and more calc() function: shouldn't we allow them in stylelint from now on?

The reason we disallow them is because calc(... + 0) is invalid. Might be worth reviewing later on if we bump into this more than once.

@MartijnCuppens MartijnCuppens changed the title Switch to custom properties to control grid widths Switch to custom properties to control grid gutter widths Mar 30, 2020
@MartijnCuppens MartijnCuppens merged commit 8414126 into master Mar 30, 2020
@MartijnCuppens MartijnCuppens deleted the master-mc-grid-gutter-custom-properties branch March 30, 2020 13:12
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants