fix: Properly target internal elements

Added support for optional `maxWidth` and `maxHeight` for `fluid` type images. The internal elements (padding for fluid, bgcolor for fixed) are both lacking class names to target specifically but both are the 1st children. Due to review constraints they are being targeted with a pseudo-selector, if the internal structure changes this will of course break. Note that the `fixed` type image has width/height attributes as well that are hard-coded to the first image in SSR build. Not tested if that causes any problems, I think it might affect `object-fit: cover`, but only with `critical` images where these attr values are used until image request responds(or re-validates if cached).
gatsbyjs · Sep 17, 2020 · 0c0ae42 · 0c0ae42
1 parent e179b56
commit 0c0ae42
Showing 1 changed file with 20 additions and 9 deletions.
diff --git a/packages/gatsby-image/src/art-direction.js b/packages/gatsby-image/src/art-direction.js
@@ -25,14 +25,25 @@ const getShortKey = input => `_` + djb2a(input).toString(36)
 // `emotion` does similar with `<style>`, it is not HTML5 spec compliant,
 // however web browsers do support using `<style>` outside of `<head>`.
 const ResponsiveQueries = ({ imageVariants, selectorClass }) => {
-  const variantRule = ({ width, height, aspectRatio }) => {
+  const variantRule = variant => {
+    const { width, height, maxWidth, maxHeight, aspectRatio } = variant
+
     // '!important' required due to overriding inline styles
     // 'aspectRatio' == fluid data
-    const styleOverride = aspectRatio
-      ? `padding-bottom: ${100 / aspectRatio}% !important;`
-      : `width: ${width}px !important; height: ${height}px !important;`
-
-    return `.${selectorClass} { ${styleOverride} }`
+    if (aspectRatio) {
+      return (
+        `.${selectorClass} {\n` +
+        (maxWidth ? `${maxWidth}px !important;\n` : ``) +
+        (maxHeight ? `${maxHeight}px !important;\n` : ``) +
+        `}.${selectorClass} > :first-child {\n` +
+        `padding-bottom: ${100 / aspectRatio}% !important;\n}`
+      )
+    } else {
+      return (
+        `.${selectorClass}, .${selectorClass} > :first-child {\n` +
+        `width: ${width}px !important; height: ${height}px !important;\n}`
+      )
+    }
   }
 
   // If there is an image without a `media` condition, the first result
@@ -44,7 +55,7 @@ const ResponsiveQueries = ({ imageVariants, selectorClass }) => {
       {imageVariants
         .filter(v => v.media)
         .map(v => (
-          <style key={`${selectorClass}-${v.media}`} media={v.media}>
+          <style key={`${selectorClass} ${v.media}`} media={v.media}>
             {variantRule(v)}
           </style>
         ))}
@@ -60,7 +71,7 @@ const withArtDirection = props => {
 
   const imageVariants = props.fluid || props.fixed
   const uniqueKey = !isBrowser
-    ? getShortKey(imageVariants.map(v => v.srcSet).join(``))
+    ? ` ` + getShortKey(imageVariants.map(v => v.srcSet).join(``))
     : ``
 
   return (
@@ -69,7 +80,7 @@ const withArtDirection = props => {
         imageVariants={imageVariants}
         selectorClass={uniqueKey}
       />
-      <GatsbyImage {...rest} className={`${uniqueKey} ${cn}`} />
+      <GatsbyImage {...rest} className={`${cn}${uniqueKey}`} />
     </>
   )
 }