-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix Image URL Localization URI #720
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work fine; it could be merged. However, I found some issues (not strictly related to this change) when creating child themes and bundling images. For example, If I added an image in a child theme and when creating the child theme, the image was bundled in the parent theme, that would need to be fixed, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is failing in the following scenario.
Steps:
- Active a block theme such as
remote
and create a child theme. - Now add an image to the child theme, and save.
- Now save changes to theme by selecting
Save Template Changes
,Localize Text
,Localize Images
. - Observe that the template localizes the image with
get_template_directory_uri()
- But the image fails to load in the editor.
@madhusudhand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shimotmk Thanks for addressing the feedback.
Changes looks good to me! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matiasbenedetto For example, If I added an image in a child theme and when creating the child theme, the image was bundled in the parent theme, that would need to be fixed, too.
I think we can address that situation after this fix is merged.
I fixed the image URI path as per this issue.
fix #714