Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Including images in FSE HTML templates #31815

Open
carolinan opened this issue May 14, 2021 · 27 comments
Open

Discussion: Including images in FSE HTML templates #31815

carolinan opened this issue May 14, 2021 · 27 comments
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@carolinan
Copy link
Contributor

carolinan commented May 14, 2021

What problem does this address?

I have received the question "How do I include images in templates?" a few times the past two weeks.

To include images -not placeholders- in HTML templates, the image needs to be remote or uploaded to the media library.
This becomes a problem when a theme is distributed as the link may break.

Remote resources are also not allowed in the theme directory. This is to prevent that the images used on peoples websites are suddenly switched to ads, or worse.

So why can't theme designers just use empty blocks?

Because we might be creating a niche theme that relies heavily on the topic of the image,
or have a visual design that is built around specific patterns and colors.

If a theme can not include for example a default header with a cover block background image,
it will be more difficult for users to pick a theme they like and start writing, instead they have to rebuild the theme by replacing all these empty image blocks.

Ways around it

-Add the images using the themes custom CSS. The problem with this is, how do users replace it?
-Leave templates blank and place all images inside block patterns. The problem is that the user has to assemble all the patterns.

What is your proposed solution?

I don't have one, let's try to find solutions together.

@carolinan carolinan added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Feature] Full Site Editing [Type] Discussion For issues that are high-level and not yet ready to implement. labels May 14, 2021
@skorasaurus
Copy link
Member

skorasaurus commented Jul 1, 2021

Thanks for creating this issue. It's a great use case of deciding how to use dynamic values in theme templates.
#20966

@anarieldesign
Copy link

What problem does this address?

I have received the question "How do I include images in templates?" a few times the past two weeks.

To include images -not placeholders- in HTML templates, the image needs to be remote or uploaded to the media library.
This becomes a problem when a theme is distributed as the link may break.

Remote resources are also not allowed in the theme directory. This is to prevent that the images used on peoples websites are suddenly switched to ads, or worse.

So why can't theme designers just use empty blocks?

Because we might be creating a niche theme that relies heavily on the topic of the image,
or have a visual design that is built around specific patterns and colors.

If a theme can not include for example a default header with a cover block background image,
it will be more difficult for users to pick a theme they like and start writing, instead they have to rebuild the theme by replacing all these empty image blocks.

Ways around it

-Add the images using the themes custom CSS. The problem with this is, how do users replace it?
-Leave templates blank and place all images inside block patterns. The problem is that the user has to assemble all the patterns.

What is your proposed solution?

I don't have one, let's try to find solutions together.

Sorry, I'm a bit late here, wanted to open a ticket for this but @annezazu told me @carolinan did it already :). Is there a reason why we can't add images the same way we are adding them for the block patterns. To call them from the theme file folder? I think it's soo important for the users to see the templates with the images and not blank. Users are expecting to have the look they see in the theme demo. And if there are empty images it's not a great user experience.

@carolinan
Copy link
Contributor Author

carolinan commented Jul 19, 2021

I am testing just using the theme path, so in the HTML template, the img src would be "/wp-content/themes/theme-name/assets/images/image.png"

But this wont work if the path is different, if the theme is in a sub folder etc.

@mkaz
Copy link
Member

mkaz commented Oct 20, 2021

Relative links work in the editor, but we need to translate the /wp-content/themes/theme-name part to use the equivalent of get_template_directory_uri()

So I'm wondering if a solution could be to support a variable in place, something like:
{{ template_directory_uri }}/assets/flying-bird.jpg

@iandunn
Copy link
Member

iandunn commented Oct 25, 2021

Related WordPress/pattern-directory#37, https://wordpress.org/openverse/

@noisysocks
Copy link
Member

noisysocks commented Oct 27, 2021

@WordPress/gutenberg-core: What action, if any, should we take here for WP 5.9?

@kjellr
Copy link
Contributor

kjellr commented Oct 27, 2021

Noting that #33217 could potentially solve this for us in the near term. If folks are ok with that solution I think it could be enough for 5.9.

@tomjn
Copy link
Contributor

tomjn commented Oct 27, 2021

in block.json there's a precedent for consistent script/style relative URLs already handled by core:

	"editorScript": "file:./build/index.js",
	"editorStyle": "file:./build/index.css",
	"style": "file:./build/style-index.css"

Which implies:

<img src="file:./assets/flying-bird.jpg"/>

It couldn't be used for quite the scope that {{ template_directory_uri }} has but:

  • it's straight forward
  • has a precedent in core already
  • is confined to the context of a URL
  • doesn't introduce odd situations or feature creep with a generic token feature
  • only makes sense in URL type attributes so the scope for misuse is minimal and better defined

Resolving the URL would be a case of testing that the asset exists in the current theme, and if not, try the parent theme. On pattern download or promotion to a template, sideload the image.

@noisysocks
Copy link
Member

Noting that #33217 could potentially solve this for us in the near term. If folks are ok with that solution I think it could be enough for 5.9.

"Potentially solve" or "will solve"? 🙂

If #33217 is all that Twenty Twenty-two needs then I would prefer that we focus on that PR instead of this issue for 5.9. That way we can think more slowly about #31815 (comment) and #31815 (comment) which are both really good ideas that deserve thought and iteration.

@tomjn
Copy link
Contributor

tomjn commented Oct 28, 2021

I'd also note that with {{ template_directory_uri }}/assets/flying-bird.jpg there's an issue with parent vs child themes and the stylesheet vs template naming convention. template_directory_uri would refer to the parent theme, stylesheet_directory_uri would be necessary to reproduce the behaviour of child themes, but then if a child theme didn't reproduce every asset of the parent theme it would be broken.

@kjellr
Copy link
Contributor

kjellr commented Oct 28, 2021

"Potentially solve" or "will solve"? 🙂

Good question. What I mean there is that the PR will allow us to do this, but that it seems like a hack to me. 😄

It's a clever way to take advantage of the fact that block patterns can be translated, but it feels far less easy to understand than something like {{ template_directory_uri }}/assets/flying-bird.jpg. It seems like a short-term solution, not a long-term one.

If we don't think we can make a more thorough solution for dynamic images/text in templates before 5.9 lands (which is totally understandable given the time we have left!), then I think #33217 seems like a promising alternative.

@Mamaduka
Copy link
Member

I like @tomjn's suggestion of using file: for assets.

The problem with tokens is, it usually doesn't stop with a single token, and we might end up creating a new "templating engine."

@mkaz
Copy link
Member

mkaz commented Oct 28, 2021

@Mamaduka I agree it could get tricky with adding complexity, and we'll end up remaking PHP 😆

I created a simple example PR that proves that it kinda work here: #36059

That is obviously not the end solution, but seems to work as a proof-of-concept.

I think the file: solution would probably be a bit harder to implement, especially if we were hoping to get this ready for 5.9, also the file: would be less flexible in the future if we wanted to add additional features/variables/values.

Open to suggestions, I don't really feel too strong on any solution. Overall, I think it might be difficult to get ready in time.

@tomjn
Copy link
Contributor

tomjn commented Oct 28, 2021

I've had an idea I like to i18n too but I can't find an appropriate ticket for this as the pattern block PR represents i18n in FSE currently.

I've created #36061 to be that issue

@tomjn
Copy link
Contributor

tomjn commented Oct 28, 2021

I think the file: solution would probably be a bit harder to implement, especially if we were hoping to get this ready for 5.9, also the file: would be less flexible in the future if we wanted to add additional features/variables/values.

a lot of the solution is already implemented and in use in core, otherwise with {{token}} we've reinvented shortcodes without the attributes. The file:. solution is also not intended to be expanded with new variables and values, it's not a general token system nor does it prevent a token system being added in the future. remove_block_asset_path_prefix is the function of relevance.

I think the PR link is broken though, here's the PR for token substitution #36059

@mkaz
Copy link
Member

mkaz commented Oct 28, 2021

Right, I had the wrong PR linked, too many open tabs. Fixed, thanks.

It looks like that function wouldn't work or I'm seeing it wrong, looking at remove_block_asset_path_prefix() all it would do is replace the file: part with nothing, so the image would left as <img src="./flying-bird.jpg"/> and this would have the browser look for a different image depending on the page you are browsing and most likely none would work.

The true URL would be: /wp-content/themes/twentytwentytwo/assets/flying-bird.jpg or
for multi-site: /site/wp-content/themes/twentytwentytwo/assets/flying-bird.jpg or
different install location /install/location/wp-content/themes/twentytwentytwo/assets/flying-bird.jpg

A relative link like that would work for CSS file, but then that would need to be a background image and different markup that wouldn't work with the blocks.

@noisysocks
Copy link
Member

The problem with tokens is, it usually doesn't stop with a single token, and we might end up creating a new "templating engine."
@Mamaduka I agree it could get tricky with adding complexity, and we'll end up remaking PHP 😆

Agree, this worries me too.

It looks like that function wouldn't work or I'm seeing it wrong, looking at remove_block_asset_path_prefix() all it would do is replace the file: part with nothing, so the image would left as and this would have the browser look for a different image depending on the page you are browsing and most likely none would work.

I think the idea is to borrow the file: prefix but not necessarily remove_block_asset_path_prefix()? A naive and very broken implementation would be to str_replace( 'file:', get_template_directory_uri() . '/', $template_content ). We could maybe use template:, theme:, asset:, etc. as the prefix if that's clearer.

@noisysocks
Copy link
Member

Popping this out of the 5.9 milestone. Let's merge #33217 as the priority for WP 5.9 as it allows Twenty Twenty-one to work around this issue as well as #36061. We can then take our time to test and iterate on solutions to this issue and #36061 in the Gutenberg plugin. I don't want us to rush such a foundational API 🙂

@tomjn
Copy link
Contributor

tomjn commented Oct 29, 2021

I think the idea is to borrow the file: prefix but not necessarily remove_block_asset_path_prefix()?

Yes!

A naive and very broken implementation would be to str_replace( 'file:', get_template_directory_uri() . '/', $template_content ).

I'm tempted to suggest a regex that matches "file:./ stuffgoeshere", so that we can test for presence of the child/parent theme properly and keep overrides working

We could maybe use template:, theme:, asset:, etc. as the prefix if that's clearer.

hmmmm, naming would be the main issue here, we would want a name that doesn't imply or mislead things

@mkaz
Copy link
Member

mkaz commented Nov 4, 2021

@tomjn I updated my PR to try this approach in 01b00ea but it is not working as expected. I believe it has to do with the pass by references which makes me think the first part of this function may not be working either.

If we can get this to work, I think it would be it.

To test I added the following to the middle of the TwentyTwentyTwo 404 template.

<!-- wp:image {"url":"file:./assets/images/ducks.jpg","sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image size-full"><img src="file:./assets/images/ducks.jpg" alt=""/></figure>
<!-- /wp:image -->

@mkaz
Copy link
Member

mkaz commented Nov 4, 2021

Ok, one more attempt here in this commit 15e7ba8

This does not use block serialization which I think was the problem in the previous, it doesn't regenerate the HTML from the block attributes, so even though we changed the block attribute, the HTML in the template is what really is used.

To test try the same image block code in previous comment.

@Stiofan
Copy link

Stiofan commented Sep 26, 2022

Still no way to reference theme images in template files?

@tomjn
Copy link
Contributor

tomjn commented Sep 26, 2022

If there was this issue would have been closed, links to the relevant implemented issues would show, and the latest default theme would showcase how it's to be done

@alvand-dev
Copy link

alvand-dev commented Oct 23, 2022

I am also looking forward to having a solution. I hope someone finds a good way.

@maharzan
Copy link

I was trying a theme with local image in a multisite and I landed on this thread.. wonder why its still not resolved?

@tomjn
Copy link
Contributor

tomjn commented Oct 16, 2023

I was trying a theme with local image in a multisite and I landed on this thread.. wonder why its still not resolved?

Nobody has been able to devise one, and it hasn't been a blocker to default themes. If you have any suggestions that haven't already been attempted though they'd be much appreciated! A lot of the more obvious ones such as relative paths have gotchas and problems that aren't immediately obvious.

I'd originally hoped my file:/ suggestion would have been what got implemented but aside from its difficulties I can also see in hindsight that it raises the new question of "when the user goes to edit it in the site editor, where do the paths reference?" Especially given that a post in the database has no file path, and could be copy pasted to another site or theme.

Perhaps some sort of named assets registration system might be needed, though there are difficulties to overcome there too.

@WebGuyJeff
Copy link

I worked around it by inlining image content using base64 data URIs in the image src:

<img src="data:image/svg+xml;base64,longStringBlah..." alt="a thing">

It's great because:

  • Templates have no asset dependencies
  • Behaves like a normal image in the editor
  • Once edited by the user, the src is overwritten thus removing the inline bloat

It sucks because

  • Hi-res rasters lead to huge strings
  • If users decide to stick with default images, this content will be cached with markup
  • It's not very 'Gutenbergy'

I've been working the 12-steps with SVG anonymous for a while now, but my addiction means my data strings are reasonable in length. If you're more of a 4k animated gif kinda person, you might end up with some filthy template files. Another workaround is registering PHP parts and dynamically populating images from theme assets (I'm doing this for patterns) but it's no solution as it completely bulldozes the HTML template concept.

One thought was a special Gut' block (or block extension) that automatically referenced the theme assets/ source, and morphed into a normal image block baking the src upon save in the editor. Something like <!-- wp:image-example --> that becomes <!-- wp:image -->. Compared to the token idea, It feels more in-keeping with FSE syntax and less like PHP12.

However, my mind started racing... <!-- wp:audio-example -->, <!-- wp:image-gallery-example -->, <!-- wp:cover-example -->...

Or maybe a wrapper block that says, "Everything in this wrapper has a dynamic link to theme/assets until saved in the editor". This feels more in-line with the block.json example: {}

 <!-- wp:example -->
     <!-- wp:image -->...<!-- /wp:image -->
 <!-- /wp:example -->

Or: <!-- wp:example { "convertsTo":"image" } -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests