Merge pull request #201 from assetgraph/feature/paddingInGroupSelector

Allow specifying the padding of the individual sprites in the group selector as well

.travis.yml

@@ -5,6 +5,7 @@ node_js:
   - 12
   - node
 
+before_install: 'sudo apt-get update && sudo apt-get install -y graphicsmagick'
 cache:
   directories:
     - node_modules

lib/spriteBackgroundImages.js

@@ -55,7 +55,7 @@ async function getImageAssetFromCanvas(canvas, assetType, assetGraph) {
   }
 }
 
-function calculateSpritePadding(paddingStr, asset) {
+function parsePadding(paddingStr, asset) {
   let padding;
   if (paddingStr) {
     // Strip units ('px' assumed)
@@ -78,10 +78,24 @@ function calculateSpritePadding(paddingStr, asset) {
   } else {
     padding = [0, 0, 0, 0];
   }
+  return padding;
+}
 
-  return padding.map((size) =>
-    Math.max(size, Math.max(asset.devicePixelRatio) - 1)
-  );
+function maxPaddingDimensions(...paddings) {
+  // [2, 4, 6, 8], [1, 5, 3, 10], ... => [2, 5, 6, 10]
+  const result = [0, 0, 0, 0];
+  for (const padding of paddings) {
+    if (padding) {
+      for (var i = 0; i < 4; i += 1) {
+        result[i] = Math.max(result[i], padding[i]);
+      }
+    }
+  }
+  return result;
+}
+
+function clampPaddingToDpr(padding, devicePixelRatio) {
+  return padding.map((size) => Math.max(size, devicePixelRatio - 1));
 }
 
 function getRelationSpriteInfoFromIncomingRelation(incomingRelation) {
@@ -91,10 +105,7 @@ function getRelationSpriteInfoFromIncomingRelation(incomingRelation) {
   return {
     groupName: parsedQueryString.sprite || 'default',
     noGroup: 'spriteNoGroup' in parsedQueryString,
-    padding: calculateSpritePadding(
-      parsedQueryString.padding,
-      incomingRelation.to
-    ),
+    padding: parsePadding(parsedQueryString.padding),
     asset: incomingRelation.to,
   };
 }
@@ -138,12 +149,10 @@ module.exports = () =>
         ] = relationSpriteInfo;
       } else {
         imageInfo.incomingRelations.push(relation);
-        for (var i = 0; i < 4; i += 1) {
-          imageInfo.padding[i] = Math.max(
-            relationSpriteInfo.padding[i],
-            imageInfo.padding[i]
-          );
-        }
+        imageInfo.padding = maxPaddingDimensions(
+          relationSpriteInfo.padding,
+          imageInfo.padding
+        );
       }
     }
 
@@ -237,17 +246,23 @@ module.exports = () =>
       const spriteInfo =
         (spriteGroup.placeHolders && spriteGroup.placeHolders[0]) || {};
 
+      const spritePadding = parsePadding(spriteInfo.padding);
       const canvasImages = await Promise.all(
         imageInfos.map((imageInfo) =>
           getCanvasImageFromImageAsset(imageInfo.asset)
         )
       );
       for (const [i, imageInfo] of imageInfos.entries()) {
         const canvasImage = canvasImages[i];
+
         Object.assign(imageInfo, {
           canvasImage,
           width: canvasImage.width,
           height: canvasImage.height,
+          padding: clampPaddingToDpr(
+            maxPaddingDimensions(imageInfo.padding, spritePadding),
+            imageInfo.asset.devicePixelRatio
+          ),
         });
       }
 
@@ -364,6 +379,7 @@ module.exports = () =>
                 '-sprite-image-format',
                 '-sprite-background-color',
                 '-sprite-important',
+                '-sprite-padding',
               ].includes(decl.prop)
             ) {
               decl.remove();

package.json

@@ -33,6 +33,7 @@
     "nyc": "^15.0.0",
     "prettier": "~2.0.2",
     "unexpected": "^11.0.0",
+    "unexpected-image": "^3.1.0",
     "urltools": "^0.4.1"
   },
   "directories": {

test/spriteBackgroundImages.js

@@ -556,4 +556,51 @@ describe('spriteBackgroundImages', () => {
       /are you trying to add an SVG to a sprite/
     );
   });
+
+  describe('with padding', function () {
+    describe('defined in the group selector', function () {
+      it('should apply the padding', async () => {
+        const assetGraph = new AssetGraph({
+          root: pathModule.resolve(
+            __dirname,
+            '..',
+            'testdata',
+            'spriteBackgroundImages',
+            'padding',
+            'inGroupSelector'
+          ),
+        });
+        await assetGraph.loadAssets('style.css');
+        await assetGraph.populate();
+
+        await assetGraph.queue(spriteBackgroundImages());
+        const [spriteAsset] = assetGraph.findAssets({ fileName: 'sprite.png' });
+        await expect(spriteAsset.rawSrc, 'to have metadata satisfying', {
+          size: {
+            width: 12,
+            height: 90,
+          },
+        });
+      });
+
+      it('should remove the -sprite-padding property', async () => {
+        const assetGraph = new AssetGraph({
+          root: pathModule.resolve(
+            __dirname,
+            '..',
+            'testdata',
+            'spriteBackgroundImages',
+            'padding',
+            'inGroupSelector'
+          ),
+        });
+        const [cssAsset] = await assetGraph.loadAssets('style.css');
+        await assetGraph.populate();
+
+        await assetGraph.queue(spriteBackgroundImages());
+
+        expect(cssAsset.text, 'not to contain', '-sprite-padding');
+      });
+    });
+  });
 });

test/unexpected-with-plugins.js

@@ -1,3 +1,4 @@
 module.exports = require('unexpected')
   .clone()
-  .installPlugin(require('assetgraph/test/unexpectedAssetGraph'));
+  .use(require('assetgraph/test/unexpectedAssetGraph'))
+  .use(require('unexpected-image'));

testdata/spriteBackgroundImages/padding/inGroupSelector/style.css

@@ -0,0 +1,14 @@
+.icon {
+  -sprite-selector-for-group: icons;
+  -sprite-location: url(sprite.png);
+  -sprite-packer: vertical;
+  -sprite-padding: 40;
+}
+
+.icon-foo {
+  background-image: url(foo.png?sprite=icons);
+}
+
+.icon-bar {
+  background-image: url(bar.png?sprite=icons);
+}