From 125486269ca5d30c194321399d99cc71a497d029 Mon Sep 17 00:00:00 2001 From: Ivailo Hristov Date: Wed, 29 Apr 2020 22:47:48 +0300 Subject: [PATCH 1/2] Change how inversed media queries are handled --- src/non-matching-media-query-remover.js | 33 ++++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/non-matching-media-query-remover.js b/src/non-matching-media-query-remover.js index 4880ab02..fc3269fb 100644 --- a/src/non-matching-media-query-remover.js +++ b/src/non-matching-media-query-remover.js @@ -24,16 +24,17 @@ function _isMatchingMediaQuery (mediaQuery, matchConfig) { // rather than the inverse field, but for our purposes we want to treat // them the same. const isInverse = mq.inverse || mq.type === 'not' - if ( - (!isInverse && mq.type === 'print') || - (isInverse && mq.type === 'screen') - ) { - return false - } // f.e. @media all {} // go for false positives over false negatives, // i.e. accept @media randomThing {} if (mq.expressions.length === 0) { + // the checks below are only valid if the media query has no other properties other than the media type + if ( + (!isInverse && mq.type === 'print') || + (isInverse && mq.type === 'screen') + ) { + return false + } return true } @@ -46,19 +47,27 @@ function _isMatchingMediaQuery (mediaQuery, matchConfig) { */ return mq.expressions.some(function ({ modifier, feature, value }) { if (modifier === 'min') { - const constructedQuery = `${ - isInverse ? 'not ' : '' - }(min-${feature}: ${value})` - return cssMediaQuery.match(constructedQuery, matchConfig) + const constructedQuery = `(min-${feature}: ${value})` + // css-mediaquery does not match inversed queries correctly, hence the if..else below + if (!isInverse) { + return cssMediaQuery.match(constructedQuery, matchConfig) + } else { + return !cssMediaQuery.match(constructedQuery, matchConfig) + } } else { if (mq.expressions.length > 1) { const constructedQuery = mq.expressions .map( ({ modifier, feature, value }) => - `${isInverse ? 'not ' : ''}(${modifier}-${feature}: ${value})` + `(${modifier}-${feature}: ${value})` ) .join(' and ') - return cssMediaQuery.match(constructedQuery, matchConfig) + // css-mediaquery does not match inversed queries correctly, hence the if..else below + if (!isInverse) { + return cssMediaQuery.match(constructedQuery, matchConfig) + } else { + return !cssMediaQuery.match(constructedQuery, matchConfig) + } } else { return true } From ea17f93cefadcde8046f27d928d89b82427fbea6 Mon Sep 17 00:00:00 2001 From: Ivailo Hristov Date: Fri, 1 May 2020 14:57:57 +0300 Subject: [PATCH 2/2] Add inversed media query tests --- test/pre-formatting.test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/pre-formatting.test.js b/test/pre-formatting.test.js index ef2bd0c5..c8bd0965 100644 --- a/test/pre-formatting.test.js +++ b/test/pre-formatting.test.js @@ -33,13 +33,17 @@ describe('penthouse pre formatting tests', () => { `@media oiasjdoiasd {}`, // covering combined queries `@media (min-width: 320px) and (max-width: 400px) {}`, - `@media (min-width: 150px) and (max-width: 1700px) {}` + `@media (min-width: 150px) and (max-width: 1700px) {}`, + `@media not screen and (min-width: 1800px) {}`, + `@media not all and (min-width: 101em) {}` ] // 1300, 1600 const mediaToRemoveAlways = [ `@media print {}`, `@media not screen {}`, - `@media not (min-width: 1px) {}` + `@media not (min-width: 1px) {}`, + `@media not screen and (min-width: 800px) {}`, + `@media not all and (min-width: 50em) {}` ] // 1600 const mediaToRemoveUnlessLarge = [