diff --git a/front/src/js/controllers/ruleCtrl.js b/front/src/js/controllers/ruleCtrl.js index 398c17f..1cdaaa4 100644 --- a/front/src/js/controllers/ruleCtrl.js +++ b/front/src/js/controllers/ruleCtrl.js @@ -48,6 +48,54 @@ ruleCtrl.controller('RuleCtrl', ['$scope', '$rootScope', '$routeParams', '$locat tooltipTemplate: '<%=label%>: <%=value%> KB' }; } + + // Init "Breakpoints" chart + if ($scope.policyName === 'cssBreakpoints' && $scope.rule.value > 0) { + + // Seek for the biggest breakpoint + var max = 0; + $scope.rule.offendersObj.forEach(function(offender) { + if (offender.pixels > max) { + max = offender.pixels; + } + }); + max = Math.max(max + 100, 1400); + + // We group offenders 10px by 10px + var GROUP_SIZE = 20; + + // Generate an empty array of values + $scope.breakpointsLabels = []; + $scope.breakpointsData = [[]]; + for (var i = 0; i <= max / GROUP_SIZE; i++) { + $scope.breakpointsLabels[i] = ''; + $scope.breakpointsData[0][i] = 0; + } + + // Fill it with results + $scope.rule.offendersObj.forEach(function(offender) { + var group = Math.floor((offender.pixels + 1) / GROUP_SIZE); + + if ($scope.breakpointsLabels[group] !== '') { + $scope.breakpointsLabels[group] += '/'; + } + $scope.breakpointsLabels[group] += offender.breakpoint; + + $scope.breakpointsData[0][group] += offender.count; + }); + + $scope.breakpointsColours = ['#9c4274']; + $scope.breakpointsOptions = { + scaleShowGridLines: false, + barShowStroke: false, + showTooltips: false, + pointDot: false, + responsive: true, + maintainAspectRatio: true, + strokeColor: 'rgba(20, 200, 20, 1)', + scaleFontSize: 9 + }; + } } $scope.backToDashboard = function() { diff --git a/front/src/views/rule.html b/front/src/views/rule.html index d888850..22733a5 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -100,6 +100,11 @@
This is the number of different colors defined in CSS.
Your CSS project will be easier to maintain if you keep a small color set.
", + "message": "This is the number of different colors defined in CSS.
Your CSS will be easier to maintain if you keep a small color set.
", "isOkThreshold": 30, "isBadThreshold": 150, "isAbnormalThreshold": 400, @@ -522,6 +522,43 @@ var policies = { }; } }, + "cssBreakpoints": { + "tool": "mediaQueriesChecker", + "label": "Breakpoints count", + "message": "This is the number of different breakpoints found in the stylesheets' media queries.
Please note this rule is based on min-width, max-width, min-device-width and max-device-width media queries only.
Your CSS will be easier to maintain if you keep a reasonable number of breakpoints. Try to make a fluid design - using percents - to avoid the creation of numerous breakpoints.
", + "isOkThreshold": 6, + "isBadThreshold": 40, + "isAbnormalThreshold": 60, + "hasOffenders": true, + "offendersTransformFn": function(offenders) { + var offendersTable = []; + + for (var offender in offenders) { + offendersTable.push({ + breakpoint: offender, + count: offenders[offender].count, + pixels: offenders[offender].pixels + }); + } + + return offendersTable; + } + }, + "cssMobileFirst": { + "tool": "mediaQueriesChecker", + "label": "Not mobile-first media queries", + "message": "This is the number of media queries that address small screens.
The common good practice, when creating a responsive website, is to write it \"mobile-first\". More explanation in this great article.
", + "isOkThreshold": 25, + "isBadThreshold": 200, + "isAbnormalThreshold": 600, + "hasOffenders": true, + "offendersTransformFn": function(offenders) { + return { + count: offenders.length, + list: offenders + }; + } + }, "cssImports": { "tool": "phantomas", "label": "Uses of @import", diff --git a/lib/metadata/scoreProfileGeneric.json b/lib/metadata/scoreProfileGeneric.json index 30230ec..9c2f68f 100644 --- a/lib/metadata/scoreProfileGeneric.json +++ b/lib/metadata/scoreProfileGeneric.json @@ -71,7 +71,9 @@ "cssComplexSelectors": 2, "cssComplexSelectorsByAttribute": 1.5, "cssColors": 0.5, - "similarColors": 0.5 + "similarColors": 0.5, + "cssBreakpoints": 1, + "cssMobileFirst": 1 } }, "badCSS": { diff --git a/lib/runner.js b/lib/runner.js index f35879d..e5fa3c7 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -4,6 +4,7 @@ var debug = require('debug')('ylt:runner'); var phantomasWrapper = require('./tools/phantomas/phantomasWrapper'); var jsExecutionTransformer = require('./tools/jsExecutionTransformer'); var colorDiff = require('./tools/colorDiff'); +var mediaQueriesChecker = require('./tools/mediaQueriesChecker'); var weightChecker = require('./tools/weightChecker/weightChecker'); var rulesChecker = require('./rulesChecker'); var scoreCalculator = require('./scoreCalculator'); @@ -32,6 +33,9 @@ var Runner = function(params) { // Compare colors data = colorDiff.compareAllColors(data); + // Check media queries + data = mediaQueriesChecker.analyzeMediaQueries(data); + // Redownload every file return weightChecker.recheckAllFiles(data); diff --git a/lib/tools/mediaQueriesChecker.js b/lib/tools/mediaQueriesChecker.js new file mode 100644 index 0000000..d66bdf0 --- /dev/null +++ b/lib/tools/mediaQueriesChecker.js @@ -0,0 +1,168 @@ +var debug = require('debug')('ylt:mediaQueriesChecker'); +var parseMediaQuery = require('css-mq-parser'); +var offendersHelpers = require('../offendersHelpers'); + + +var mediaQueriesChecker = function() { + 'use strict'; + + var MOBILE_MIN_BREAKPOINT = 200; + var MOBILE_MAX_BREAKPOINT = 300; + + this.analyzeMediaQueries = function(data) { + debug('Starting to check all media queries...'); + + var offenders = data.toolsResults.phantomas.offenders.cssMediaQueries; + var mediaQueries = (offenders) ? this.parseAllMediaQueries(offenders) : []; + + var notMobileFirstCount = 0; + var notMobileFirstOffenders = []; + + var breakpointsOffenders = {}; + + for (var i = 0; i < mediaQueries.length; i++) { + var item = mediaQueries[i]; + + if (!item) { + continue; + } + + if (item.isForMobile) { + notMobileFirstCount += item.mediaQuery.rules; + notMobileFirstOffenders.push(item.mediaQuery); + } + + for (var j = 0; j < item.breakpoints.length; j++) { + var breakpointString = item.breakpoints[j].string; + if (!breakpointsOffenders[breakpointString]) { + breakpointsOffenders[breakpointString] = { + count: 1, + pixels: item.breakpoints[j].pixels + }; + } else { + breakpointsOffenders[breakpointString].count += 1; + } + } + } + + data.toolsResults.mediaQueriesChecker = { + metrics: { + cssMobileFirst: notMobileFirstCount, + cssBreakpoints: Object.keys(breakpointsOffenders).length + }, + offenders: { + cssMobileFirst: notMobileFirstOffenders, + cssBreakpoints: breakpointsOffenders + } + }; + + debug('End of media queries check'); + + return data; + }; + + this.parseAllMediaQueries = function(offenders) { + return offenders.map(this.parseOneMediaQuery); + }; + + this.parseOneMediaQuery = function(offender) { + var splittedOffender = offendersHelpers.cssOffenderPattern(offender); + var parts = /^@media (.*) \((\d+ rules)\)$/.exec(splittedOffender.css); + + if (!parts) { + debug('Failed to parse media query ' + offender); + return false; + } + + var rulesCount = parseInt(parts[2], 10); + var query = parts[1]; + + var isForMobile = false; + var breakpoints = []; + + try { + + var ast = parseMediaQuery(query); + + var min = 0; + var max = Infinity; + var pixels; + + ast.forEach(function(astItem) { + astItem.expressions.forEach(function(expression) { + if (expression.feature === 'width' || expression.feature === 'device-width') { + if (astItem.inverse === false) { + if (expression.modifier === 'max') { + pixels = toPixels(expression.value); + max = Math.min(max, pixels); + breakpoints.push({ + string: expression.value, + pixels: pixels + }); + } else if (expression.modifier === 'min') { + pixels = toPixels(expression.value); + min = Math.max(min, pixels); + breakpoints.push({ + string: expression.value, + pixels: pixels + }); + } + } else if (astItem.inverse === true) { + if (expression.modifier === 'max') { + pixels = toPixels(expression.value); + min = Math.max(min, pixels); + breakpoints.push({ + string: expression.value, + pixels: pixels + }); + } else if (expression.modifier === 'min') { + pixels = toPixels(expression.value); + max = Math.min(max, pixels); + breakpoints.push({ + string: expression.value, + pixels: pixels + }); + } + } + } + }); + }); + + isForMobile = (min <= MOBILE_MIN_BREAKPOINT && max >= MOBILE_MAX_BREAKPOINT && max !== Infinity); + + } catch(error) { + debug('Failed to parse media query ' + offender); + } + + return { + mediaQuery: { + query: query, + rules: rulesCount, + file: splittedOffender.file, + line: splittedOffender.line, + column: splittedOffender.column + }, + isForMobile: isForMobile, + breakpoints: breakpoints + }; + }; + + // Parses a size in em, pt (or px) and returns it in px + function toPixels(size) { + var splittedSize = /^([\d\.]+)(.*)/.exec(size); + var value = parseFloat(splittedSize[1]); + var unit = splittedSize[2]; + + if (unit === 'em') { + return value * 16; + } + + if (unit === 'pt') { + return value / 12 * 16; + } + + return value; + } +}; + +module.exports = new mediaQueriesChecker(); \ No newline at end of file diff --git a/package.json b/package.json index 771aca9..74e2789 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "color-diff": "0.1.7", "compression": "1.6.0", "cors": "2.7.1", + "css-mq-parser": "0.0.3", "debug": "2.2.0", "express": "4.13.3", "imagemin": "3.2.2", diff --git a/test/core/mediaQueriesCheckerTest.js b/test/core/mediaQueriesCheckerTest.js new file mode 100644 index 0000000..7b3222b --- /dev/null +++ b/test/core/mediaQueriesCheckerTest.js @@ -0,0 +1,55 @@ +var should = require('chai').should(); +var mediaQueriesChecker = require('../../lib/tools/mediaQueriesChecker'); + +describe('mediaQueriesChecker', function() { + + it('should parse mediaQueryes correctly', function() { + mediaQueriesChecker.parseOneMediaQuery('@media screen and (max-width: 1024px) (1 rules)