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

Bundle template images in theme assets and make their urls relative #192

Closed
wants to merge 3 commits into from

Conversation

matiasbenedetto
Copy link
Contributor

@matiasbenedetto matiasbenedetto commented Jan 26, 2023

What?

If a template or a template part includes images which source is an absolute URL:

  • The images are downloaded and bundled in the /assets/images folder of the theme
  • The templates or template parts that include these images are converted to a pattern and added to the pattern folder. The template content is replaced by a reference to the pattern recently created.

Why?

To make the process of launching a theme easier by bundling to theme assets the images inserted using the editor.

To Do:

Include other blocks including images as cover.

How to test:

  • Include one or more image blocks in a template or template part using the editor.
  • Save the changes
  • Export the theme using the plugin
  • See if the resulting theme works as expected

⚠️ Still a draft

Closes: #161

@matiasbenedetto matiasbenedetto marked this pull request as draft January 26, 2023 11:32
@matiasbenedetto
Copy link
Contributor Author

The download of the images and the inclusion on the new local theme or .zip file is working.
The problem we need to solve is: we are only able to use relative URLs in patterns (PHP files), as in this example: https://github.com/Automattic/themes/blob/a71c0df4ee42d8f0f3baa801180611e31ee1d5e0/rainfall/patterns/big-header.php#L37

So we would need to move the template's content to a pattern and then insert the pattern in the template. It sounds a little convoluted. Do you have a better idea of how we could do this?

@mikachan
Copy link
Member

Here are some related GB issues:

WordPress/gutenberg#31815
WordPress/gutenberg#44187

Perhaps it's worth looking into a token system like this: WordPress/gutenberg#42015

$media[] = $tag->getAttribute( 'src' );
$tag->setAttribute(
'src',
'<?php echo esc_url(get_template_directory_uri()); ?>/assets/images/'. basename( $tag->getAttribute( 'src' ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be

Suggested change
'<?php echo esc_url(get_template_directory_uri()); ?>/assets/images/'. basename( $tag->getAttribute( 'src' ) )
'<?php echo esc_url(get_stylesheet_directory_uri()); ?>/assets/images/'. basename( $tag->getAttribute( 'src' ) )

@jffng
Copy link
Contributor

jffng commented Jan 27, 2023

The download of the images and the inclusion on the new local theme or .zip file is working.

🎉

What if we shipped with this feature, and worked on an easier way to make / save new patterns to the theme?

It is a bit of a workaround but also solves something that is needed, which is an easier way to create patterns rather than copying and pasting the block markup into a new php file.

Also I think it will be awhile before something like tokens is available.

---- EDIT

My suggestion would not work yet. Because there is not yet a way to insert the pattern from the site editor and have it be preserved as a pattern — the full markup is inserted and stripped of any php, making it an absolute URL again.

@matiasbenedetto matiasbenedetto added the enhancement New feature or request label Feb 2, 2023
@carolinan
Copy link

carolinan commented Feb 6, 2023

Is this and the pattern feature mentioned above, intended to be tested in the plugin and then moved to Gutenberg and then Core? is there a benefit to that over building it Gutenberg?

@matiasbenedetto
Copy link
Contributor Author

Is this and the pattern feature mentioned above, intended to be tested in the plugin and then moved to Gutenberg and then Core? is there a benefit to that over building it Gutenberg?

Currently, this seems to be the best possible way to achieve the bundling into the theme assets the images inserted into templates and template parts using the editor. The objective is to make this task easier for theme creators. I'm curious about how you imagine it working on Gutenberg.

@matiasbenedetto
Copy link
Contributor Author

I'm closing this PR to continue the discussion on #213 to have a cleaner git diff.

@carolinan
Copy link

Is this and the pattern feature mentioned above, intended to be tested in the plugin and then moved to Gutenberg and then Core? is there a benefit to that over building it Gutenberg?

Currently, this seems to be the best possible way to achieve the bundling into the theme assets the images inserted into templates and template parts using the editor. The objective is to make this task easier for theme creators. I'm curious about how you imagine it working on Gutenberg.

As part of the theme export functionality in the Site Editor, or did I misunderstand your question?

@matiasbenedetto
Copy link
Contributor Author

Yep, it sounds like a good suggestion. I think first we can focus on make this functionality work in the plugin, test if it's handy and useful and after that kick-start a discussion about the addition to the Gutenberg theme export option.

@vcanales vcanales deleted the add/161 branch May 23, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove absolute URLs from images in block markup
4 participants