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

Sort ascending $grid-breakpoints and $container-max-widths maps #25392

Closed
wants to merge 2 commits into from
Closed

Sort ascending $grid-breakpoints and $container-max-widths maps #25392

wants to merge 2 commits into from

Conversation

iamandrewluca
Copy link
Contributor

@iamandrewluca iamandrewluca commented Jan 21, 2018

Extend $grid-breakpoints and $container-max-widths maps, as $theme-colors see #26714

When extending $grid-breakpoints and $container-max-widths we must
be sure that they are sorted ascending, added a new function that sorts maps by their values.
Now _assert-ascending call it seems to be obsolete if we sort maps before that

If user extends mentioned vars after variables import
map-sort-by-values must be called again to be sure they are sorted

@mdo
Copy link
Member

mdo commented Jan 22, 2018

Nice, thanks for this. Give me some time to review and test those new functions :).

@mdo mdo requested review from mdo and andresgalante January 22, 2018 17:18
scss/_functions.scss Outdated Show resolved Hide resolved
@MartijnCuppens
Copy link
Member

@mdo, we didn't think about this when we merged #26714.

@iamandrewluca iamandrewluca requested a review from a team as a code owner December 6, 2018 08:25
@iamandrewluca iamandrewluca changed the title Extend $grid-breakpoints and $container-max-widths maps Sort ascending $grid-breakpoints and $container-max-widths maps Dec 6, 2018
iamandrewluca and others added 2 commits December 12, 2018 12:12
…me-colors`

When extending `$grid-breakpoints` and `$container-max-widths` we must
be sure that they are sorted ascending, added a new functions that sorts
maps by their values.
Now `_assert-ascending` it seems to be obsolete if we sort maps before that

If user extends mentioned vars after `variables` import
`map-sort-by-values` must be called again to be sure they are sorted
@iamandrewluca
Copy link
Contributor Author

iamandrewluca commented Jan 16, 2019

@MartijnCuppens any feedback here? As I see, merging maps has been reverted.

@MartijnCuppens
Copy link
Member

Hi @iamandrewluca
#26714 was indeed reverted because it was a breaking change. We restored the original behaviour because this is the easiest behavior to understand for everyone, so we'll pass on this for now.

That being said, I'm quite impressed by the function you made here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants