Skip to content
This repository has been archived by the owner on Jun 14, 2022. It is now read-only.

Add srcset to AB Container background image #238

Open
tristanmason opened this issue Sep 15, 2019 · 4 comments
Open

Add srcset to AB Container background image #238

tristanmason opened this issue Sep 15, 2019 · 4 comments
Labels
enhancement New feature or request

Comments

@tristanmason
Copy link

Is your feature request related to a problem? Please describe.
I would love to see a srcset attribute on the AB Container block's background image, in the interest of improving page load speed on mobile devices, performance scores, etc.

Describe the solution you'd like
Adding a wp-image-[ID] class to the img tag would trigger Wordpress to add the srcset automatically (see WordPress/gutenberg#8593). I forked AB and tried adding the class name on line 232 of /src/blocks/block-contained/index.js. It is tested and working as below:

index 3681935..f3fcf84 100644
--- a/src/blocks/block-container/index.js
+++ b/src/blocks/block-container/index.js
@@ -229,7 +229,7 @@ registerBlockType( 'atomic-blocks/ab-container', {
  <div className="ab-container-image-wrap">
    <img
      className={ classnames(
-        'ab-container-image',
+        'ab-container-image wp-image-' . containerImgID,
           dimRatioToClass( containerDimRatio ),
           {
             'has-background-dim': 0 !== containerDimRatio

Additional context
When I tried adding the same code to line 155 in order to create the same container markup in the editor, Gutenberg threw an error about invalid syntax in the block. It wanted to wrap the image in <figure> tags. I don't know that much about Gutenberg coding and don't know if it's best practice to keep editor and front-end markup exactly the same. But the above change does trigger WP to create the srcset.

Please let me know if you want me to submit a PR for this.

Thanks!

@mindctrl
Copy link
Contributor

Thanks for writing this up @tristanmason! We'll need to do a deprecation to add the class. Are you familiar with how those work in Gutenberg?

@tristanmason
Copy link
Author

@mindctrl I just read some of their docs on this and understand the purpose, but I wouldn't trust myself to code it yet. I'll take a closer look. If one of you more Gutenberg-knowledgable folks gets to it first, don't wait for me. 😄

@mindctrl
Copy link
Contributor

Yeah it's a little strange to wrap your head around at first. Happy to help if you decide to take a crack at it.

Here are a couple of commits where we did deprecations if you'd like to check those out:

be5ec38

e2db0e4

@tristanmason
Copy link
Author

@mindctrl Thanks! I'm checking out the commits you referenced and hope to try this out soon.

@mindctrl mindctrl added the enhancement New feature or request label Mar 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants