Skip to content

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

Closed
unrevised6419 wants to merge 2 commits intotwbs:v4-devfrom
unrevised6419:extend-grid-breakpoints
Closed

Sort ascending $grid-breakpoints and $container-max-widths maps#25392
unrevised6419 wants to merge 2 commits intotwbs:v4-devfrom
unrevised6419:extend-grid-breakpoints

Conversation

@unrevised6419
Copy link
Copy Markdown

@unrevised6419 unrevised6419 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
Copy Markdown
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 andresgalante and mdo January 22, 2018 17:18
Comment thread scss/_functions.scss Outdated
@MartijnCuppens
Copy link
Copy Markdown
Member

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

@unrevised6419 unrevised6419 requested a review from a team as a code owner December 6, 2018 08:25
@unrevised6419 unrevised6419 changed the title Extend $grid-breakpoints and $container-max-widths maps Sort ascending $grid-breakpoints and $container-max-widths maps Dec 6, 2018
Andrew Luca 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
@unrevised6419
Copy link
Copy Markdown
Author

unrevised6419 commented Jan 16, 2019

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

@MartijnCuppens
Copy link
Copy Markdown
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.

4 participants