search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.33k stars 4.12k forks source link

Discussion: Including images in FSE HTML templates #31815

Open carolinan opened 3 years ago

carolinan commented 3 years ago

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.

skorasaurus commented 3 years ago

Thanks for creating this issue. It's a great use case of deciding how to use dynamic values in theme templates.
https://github.com/WordPress/gutenberg/issues/20966

anarieldesign commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

Related https://github.com/WordPress/pattern-directory/issues/37, https://wordpress.org/openverse/

noisysocks commented 2 years ago

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

kjellr commented 2 years ago

Noting that https://github.com/WordPress/gutenberg/pull/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 commented 2 years ago

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:

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 commented 2 years ago

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 https://github.com/WordPress/gutenberg/issues/31815#issuecomment-948117731 and https://github.com/WordPress/gutenberg/issues/31815#issuecomment-953368315 which are both really good ideas that deserve thought and iteration.

tomjn commented 2 years ago

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 commented 2 years ago

"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 https://github.com/WordPress/gutenberg/pull/33217 seems like a promising alternative.

Mamaduka commented 2 years ago

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 commented 2 years ago

@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: https://github.com/WordPress/gutenberg/pull/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 commented 2 years ago

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 https://github.com/WordPress/gutenberg/issues/36061 to be that issue

tomjn commented 2 years ago

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 https://github.com/WordPress/gutenberg/pull/36059

mkaz commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

tomjn commented 2 years ago

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 commented 2 years ago

@tomjn I updated my PR to try this approach in https://github.com/WordPress/gutenberg/pull/36059/commits/01b00ea362fff74e685f259987ae9aa800101254 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 commented 2 years ago

Ok, one more attempt here in this commit https://github.com/WordPress/gutenberg/pull/36059/commits/15e7ba8f1031ebed817710527bbc13d1366e90c6

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 commented 1 year ago

Still no way to reference theme images in template files?

tomjn commented 1 year ago

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

alvand1399 commented 1 year ago

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

maharzan commented 11 months ago

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

tomjn commented 11 months ago

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 commented 9 months ago

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:

It sucks because

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" } -->