From cc4ac38288e7132df7503a751e2f0d2abfd97932 Mon Sep 17 00:00:00 2001
From: Nicolas Coden <nicolas@ncoden.fr>
Date: Thu, 19 Jan 2017 17:57:17 +0100
Subject: [PATCH] Fix HiDPI breakpoint with only range

Media Queries does not accept nested logic.

Changes:
- Generate a valid media query using string joining functions.
- Update unit tests

Add required functions:
- `-zf-bp-join`: Return media query string from the given min and/or
max limits.
- `zf-str-join`: Return a join of the two given strings `$str1` and
`$str2`. If the two strings are not empty, they are separated by
`$delimiter`.
---
 scss/util/_breakpoint.scss | 54 ++++++++++++++++++++++----------------
 scss/util/_value.scss      | 29 ++++++++++++++++++++
 test/sass/_breakpoint.scss | 17 ++++++------
 3 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/scss/util/_breakpoint.scss b/scss/util/_breakpoint.scss
index 50c70f755..71fa44c0a 100644
--- a/scss/util/_breakpoint.scss
+++ b/scss/util/_breakpoint.scss
@@ -110,31 +110,17 @@ $breakpoint-classes: (small medium large) !default;
   }
 
   // Generate the media query string from min and max limits.
-  $str: '';
-  @if $bp-min and $bp-min > 0 {
-    @if $hidpi {
-      $bp-min-dpi: $bp-min * 96dpi;
-      $str: $str + '((-webkit-min-device-pixel-ratio: #{$bp-min}), (min-resolution: #{$bp-min-dpi}))'
-    }
-    @else {
-      $str: $str + '(min-width: #{$bp-min})';
-    }
-
-    @if $bp-max and $bp-max > 0 {
-      $str: $str + ' and ';
-    }
+  @if $hidpi {
+    $bp-min-dpi: if($bp-min, $bp-min * 96dpi, $bp-min);
+    $bp-max-dpi: if($bp-max, $bp-max * 96dpi, $bp-max);
+    @return zf-str-join(
+      -zf-bp-join($bp-min, $bp-max, '-webkit-min-device-pixel-ratio', '-webkit-max-device-pixel-ratio'),
+      -zf-bp-join($bp-min-dpi, $bp-max-dpi, 'min-resolution', 'max-resolution'),
+      ', ');
   }
-  @if $bp-max and $bp-max > 0 {
-    @if $hidpi {
-      $bp-max-dpi: $bp-max * 96dpi;
-      $str: $str + '((-webkit-max-device-pixel-ratio: #{$bp-max}), (max-resolution: #{$bp-max-dpi}))'
-    }
-    @else {
-      $str: $str + '(max-width: #{$bp-max})';
-    }
+  @else {
+    @return -zf-bp-join($bp-min, $bp-max);
   }
-
-  @return $str;
 }
 
 /// Wraps a media query around the content you put inside the mixin. This mixin accepts a number of values:
@@ -283,6 +269,28 @@ $breakpoint-classes: (small medium large) !default;
   }
 }
 
+/// Return media query string from the given min and/or max limits.
+/// If a limit is equal to `null` or `0`, it is ignored.
+/// @access private
+///
+/// @param {Number} $min [0] - Min media query limit.
+/// @param {Number} $max [0] - Max media query limit.
+/// @param {String} $min-name ['min-width'] - Name of the min media query limit.
+/// @param {String} $delimiter ['max-width'] - Name of the max media query limit.
+///
+/// @returns {String} Media Query string.
+@function -zf-bp-join(
+  $min: 0,
+  $max: 0,
+  $min-name: 'min-width',
+  $max-name: 'max-width'
+) {
+  @return zf-str-join(
+    if($min and $min > 0, '(#{$min-name}: #{$min})', null),
+    if($max and $max > 0, '(#{$max-name}: #{$max})', null),
+    ' and ');
+}
+
 @if map-has-key($breakpoints, small) {
   $small-up: screen;
   $small-only: unquote('screen and #{breakpoint(small only)}');
diff --git a/scss/util/_value.scss b/scss/util/_value.scss
index cec4408be..514abc59a 100644
--- a/scss/util/_value.scss
+++ b/scss/util/_value.scss
@@ -138,3 +138,32 @@
   @return if(type-of($map) != 'list', ($value,), $map);
 
 }
+
+/// Return a join of the two given strings `$str1` and `$str2`.
+/// If the two strings are not empty, they are separated by `$delimiter`.
+///
+/// @param {String} $str1 [null] - First string to join.
+/// @param {String} $str1 [null] - Second string to join.
+/// @param {String} $delimiter [null] - Delimieter between `$str1` and `$str2`.
+///
+/// @returns {String} Join of `$str1`, `$delimiter` and `$str2`.
+@function zf-str-join(
+  $str1: null,
+  $str2: null,
+  $delimiter: null
+) {
+  $ret: '';
+
+  @if $str1 and str-length($str1) > 0 {
+    $ret: $ret + $str1;
+
+    @if $delimiter and str-length($delimiter) > 0 and $str2 and str-length($str2) > 0 {
+      $ret: $ret + $delimiter;
+    }
+  }
+  @if $str2 and str-length($str2) > 0 {
+    $ret: $ret + $str2;
+  }
+
+  @return $ret;
+}
diff --git a/test/sass/_breakpoint.scss b/test/sass/_breakpoint.scss
index 89911350e..9c9045f9f 100755
--- a/test/sass/_breakpoint.scss
+++ b/test/sass/_breakpoint.scss
@@ -1,6 +1,7 @@
 @import "true";
 
 @import '../../scss/util/unit';
+@import '../../scss/util/value';
 @import '../../scss/util/breakpoint';
 
 @include test-module('Breakpoint') {
@@ -59,15 +60,15 @@
   // --- Breakpoint: HiDPI/Retina ---
 
   @include test('Breakpoint (Retina) [function]') {
-    $expect-2: '((-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi))';
+    $expect-2: '(-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi)';
 
     @include assert-equal(breakpoint(retina), $expect-2,
       'Creates a x2 HiDPI range out of the retina alias breakpoint');
   }
 
   @include test('Breakpoint (HIDPI Default/Up Range) [function]') {
-    $expect-1-5: '((-webkit-min-device-pixel-ratio: 1.5), (min-resolution: 144dpi))';
-    $expect-2: '((-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi))';
+    $expect-1-5: '(-webkit-min-device-pixel-ratio: 1.5), (min-resolution: 144dpi)';
+    $expect-2: '(-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi)';
 
     @include assert-equal(breakpoint(hidpi-1-5 up), $expect-1-5,
       'Creates a x1.5 HiDPI up range out of a x1.5 HiDPI breakpoint');
@@ -83,9 +84,9 @@
   }
 
   @include test('Breakpoint (HIDPI Only Range) [function]') {
-    $expect-1-5: '((-webkit-min-device-pixel-ratio: 1.5), (min-resolution: 144dpi)) and ((-webkit-max-device-pixel-ratio: 1.98958), (max-resolution: 191dpi))';
-    $expect-2: '((-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi)) and ((-webkit-max-device-pixel-ratio: 2.98958), (max-resolution: 287dpi))';
-    $expect-highest: '((-webkit-min-device-pixel-ratio: 3), (min-resolution: 288dpi))';
+    $expect-1-5: '(-webkit-min-device-pixel-ratio: 1.5) and (-webkit-max-device-pixel-ratio: 1.98958), (min-resolution: 144dpi) and (max-resolution: 191dpi)';
+    $expect-2: '(-webkit-min-device-pixel-ratio: 2) and (-webkit-max-device-pixel-ratio: 2.98958), (min-resolution: 192dpi) and (max-resolution: 287dpi)';
+    $expect-highest: '(-webkit-min-device-pixel-ratio: 3), (min-resolution: 288dpi)';
 
     @include assert-equal(breakpoint(hidpi-1-5 only), $expect-1-5,
       'Creates a x1.5 HiDPI only range out of a x1.5 HiDPI breakpoint');
@@ -98,8 +99,8 @@
   }
 
   @include test('Breakpoint (HIDPI Down Range) [function]') {
-    $expect-1-5: '((-webkit-max-device-pixel-ratio: 1.98958), (max-resolution: 191dpi))';
-    $expect-2: '((-webkit-max-device-pixel-ratio: 2.98958), (max-resolution: 287dpi))';
+    $expect-1-5: '(-webkit-max-device-pixel-ratio: 1.98958), (max-resolution: 191dpi)';
+    $expect-2: '(-webkit-max-device-pixel-ratio: 2.98958), (max-resolution: 287dpi)';
     $expect-highest: '';
 
     @include assert-equal(breakpoint(hidpi-1-5 down), $expect-1-5,
-- 
2.47.2