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

Inline Dynamic Content Solutions #21932

Open
epiqueras opened this issue Apr 27, 2020 · 29 comments
Open

Inline Dynamic Content Solutions #21932

epiqueras opened this issue Apr 27, 2020 · 29 comments
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Technical Feedback Needs testing from a developer perspective. [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@epiqueras
Copy link
Contributor

epiqueras commented Apr 27, 2020


We need to decide on how we are going to handle inline dynamic content in blocks. (Things like base URLs of template images and translatable strings.)

It kind of breaks our current FSE template solution if we can’t find a solution for this.

Proposal

I still think some kind of inline token/block is the worthiest option to explore right now.

I know there are concerns about having demarcations inside elements because that would make the HTML “invalid”. Still, because of the nature of React, at worst, these inline demarcations will be inside attribute values, and that would always be valid markup. Although, I would argue that if you are viewing a document without server rendering, you wouldn’t want blocks that depend on inline dynamic content to render at all, just like how dynamic blocks work.

I proposed a way of doing that in this comment. Blocks that have inline dynamic content are entirely commented out in the markup and only uncommented by the server renderer if it can resolve all of its inline tokens. For things like translatable text, a “default” uncommented block can be on top with the inline tokens resolved to their default values. When server rendering, the renderer can delete that default block and replace it with the commented out one, with its inline tokens resolved.

Some examples:

Base URL

Static

<!-- wp:image -->
<!-- <figure class="wp-block-image"><img src="<!- wp:base-url /->some-path" alt="" /></figure> -->
<!-- /wp:image -->

Rendered

<figure class="wp-block-image">
	<img src="https:://some-site.com/some-directory/some-path" alt="" />
</figure>

I18N

Static

<!-- wp:paragraph -->
<p>Some text.</p>
<!-- <p><!- wp:i18n {"scope":"gutenberg"} ->Some text.<!- /wp:i18n -></p> -->
<!-- /wp:paragraph -->

Rendered

<p>Algun texto.</p>

With this, we can do everything we like about blocks, at the inline-level. For example, we can use the inline token attributes to have specific, local base URL settings, or, have different localization settings for different pieces of text, or none at all.

Alternatives?

I think this is a critical issue/decision, and we should all think about it deeply. I am looking forward to hearing your thoughts, alternative proposals, and feedback.

@epiqueras epiqueras added Needs Technical Feedback Needs testing from a developer perspective. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. [Feature] Full Site Editing labels Apr 27, 2020
@epiqueras epiqueras self-assigned this Apr 27, 2020
@simison
Copy link
Member

simison commented Apr 28, 2020

cc @sirreal and @ockham were involved in solving some of the "base URL" problems in Jetpack plugin using Webpack.

@simison
Copy link
Member

simison commented Apr 28, 2020

<!-- <p><!- wp:i18n {"scope":"gutenberg"} ->Some text.<!- /wp:i18n -></p> -->

Would using regular wp.i18n.__() functions in the save() function suffice and then there could be some kind of notion to tell the editor that an attribute or value can be ignored when validating the block?

So I'd imagine save() output to contain:

<Button aria-label={ __( 'Do the thing' ) }>{ __( 'The thing' ) }</Button>

...and then some kind of decorator HTML to indicate what validator should ignore — not literally these attribute names but you get the point:

  • data-tell-editor-to-ignore-value
  • data-tell-editor-to-ignore-aria-label

Without these, __() output would get invalidated between saving the content and opening it again in a different locale.

The problem with i18n I see is that Editor can be running in a different locale than the site itself is.

@youknowriad
Copy link
Contributor

At the moment, this still seems way too complex for me and we lose the "fallback" value of the regular block HTML when doing so. I'm not certain we won't need it at some point but we should try "one time dynamic values":

  • writing templates with inline php.
  • when saved, it becomes static because it doesn't make sense to mix dynamic and static (edited values).

@epiqueras
Copy link
Contributor Author

@simison

cc @sirreal and @ockham were involved in solving some of the "base URL" problems in Jetpack plugin using Webpack.

I don't think that is the same issue. Here the content does not go through Webpack. E.g., a block template HTML file that contains theme image URLs.

Would using regular wp.i18n.__() functions in the save() function suffice and then there could be some kind of notion to tell the editor that an attribute or value can be ignored when validating the block?

No, because different viewers need to see different languages.

@youknowriad

we lose the "fallback" value of the regular block HTML

I explained in the description of the issue how we could keep the fallback and included examples.

writing templates with inline php.

Here, we would lose the fallback, and we would lose the "dynamicness" after the first save.

when saved, it becomes static because it doesn't make sense to mix dynamic and static (edited values).

What about the theme block template image example, editing a template without changing the images would not work. How would i18n work? I think we need to mix these things.

@mcsf
Copy link
Contributor

mcsf commented Apr 29, 2020

Would using regular wp.i18n.__() functions in the save() function suffice and then there could be some kind of notion to tell the editor that an attribute or value can be ignored when validating the block?

No, because different viewers need to see different languages.

I'm not sure that answers @simison's question.

This also begs the question: could we find a solution that would, at once, handle HTML-based templates and blocks that require i18n? Maybe the domains are different enough to justify different solutions, but I really don't know.

I'm not certain we won't need it at some point but we should try "one time dynamic values":

* writing templates with inline php.

* when saved, it becomes static

@youknowriad, to confirm: you are suggesting templates that keep working dynamically for as long as the user never commits a change to it, correct? I'm asking because of the "one-time dynamic" characterization.

If so, then I think the idea has some merit. It's not too different from the distinction between theme-provided string and user content as put in #21728 (comment).

@mcsf
Copy link
Contributor

mcsf commented Apr 29, 2020

<!-- wp:paragraph -->
<p>Some text.</p>
<!-- <p><!- wp:i18n {"scope":"gutenberg"} ->Some text.<!- /wp:i18n -></p> -->
<!-- /wp:paragraph -->

This is interesting, but I think too complex. It requires some legwork at the levels of the parser and render, and requires breaking some useful premises. We'd have to establish that it's an unavoidably better solution than simpler approaches.

@mtias
Copy link
Member

mtias commented Apr 29, 2020

My biggest concern with this approach is we get deeper into domain-specific language territory for theme building. The block grammar made sense as an invisible and robust storage mechanism because we were dealing with user content and our goal was to prioritize longevity of a readable format while being able to traduce and augment it dynamically. Themes don't fall in the same bucket since there's no sacrosanctity with the source and there's already an expectation of using a general purpose language to write them.

It's been great to start with html files as it allows us to stress test what blocks ara capable of (the dynamic content contained within a static html file) but translatable strings for starter content is a specific and well defined problem that already has a solution in PHP since there are no expectations that modifying content would retain translatability. When starter content is modified, it's stored as static text on a user's templates CPT, and there are no longer concerns about making any of it translatable. I think declaring template files in PHP provided they resolve in a valid block tree seems very sensible to me.

Dynamic urls, on the other hand, could be handled by augmenting src properties in block comment attributes to support a shape like src: { "context": "theme_root"; "path": "/images/header.png" }. This is akin to essentially making the whole block dynamic (no expectation of fallback rendering, which doesn't make sense for a theme anyways), which is the other path I'd suggest exploring: a custom block variation of wp:image that is wp:theme/image and makes assumptions over the source, or even the rendering fallback. We might even assume all wp:image in a template to be wp:theme/image given the template loader context — when saved statically by the user it'll became a regular wp:image pointing to the resource the user has declared.

I think I asked in another thread what other scenarios we have of dynamic content that might not fall in either of these two buckets. Conditions are naturally something that needs to be handled (if there are no comments, don't show the comment count) but I think these could be modelled within the block definition in most cases given the nature of a purely dynamic block.

@epiqueras
Copy link
Contributor Author

@mcsf

I'm not sure that answers @simison's question.

I was mentioning it's not sufficient, because different viewers need to see different languages, and that approach would translate at save time, not view time.

This also begs the question: could we find a solution that would, at once, handle HTML-based templates and blocks that require i18n? Maybe the domains are different enough to justify different solutions, but I really don't know.

That's what my suggestion achieves.

"One-time dynamic" values won't allow for an acceptable UX. Changing a single image in a template or a tiny bit of copy would make you lose all translations.

This is interesting, but I think too complex. It requires some legwork at the levels of the parser and render, and requires breaking some useful premises. We'd have to establish that it's an unavoidably better solution than simpler approaches.

Yeah, I agree, that's why I'm not rushing into it, but I'm finding it hard to come up with an alternative.

@mtias

When starter content is modified, it's stored as static text on a user's templates CPT, and there are no longer concerns about making any of it translatable. I think declaring template files in PHP provided they resolve in a valid block tree seems very sensible to me.

That's a compromise we can make. But, it worsens the UX for both theme creators and users.

Theme creators would need to manually edit their template files to add translations and do so again every time they make modifications in the block editor.

Users would lose all translations for a template after even the slightest change. E.g., the color of something.

Dynamic urls, on the other hand, could be handled by augmenting src properties in block comment attributes to support a shape like

That's one approach I proposed in the original issue. It works and doesn't have any UX drawbacks, but it won't be needed if we don't make the i18n compromise.

@mtias
Copy link
Member

mtias commented Apr 30, 2020

Theme creators would need to manually edit their template files to add translations and do so again every time they make modifications in the block editor.

I don't think it'd be common for the source of a theme to be kept in an editor. Most people would still prefer to keep the source in code and version control. In any case, this seems like a tooling problem for the theme editor and exporter not necessarily an architecture one.

Users would lose all translations for a template after even the slightest change. E.g., the color of something.

By losing translations I gather you mean they "lose the ability to switch language afterwards and have the theme text for that template be updated without them resetting the customizations first", which seems like a fine compromise to me for this stage we are at.

@epiqueras
Copy link
Contributor Author

I don't think it'd be common for the source of a theme to be kept in an editor. Most people would still prefer to keep the source in code and version control. In any case, this seems like a tooling problem for the theme editor and exporter not necessarily an architecture one.

If you need to make changes to a theme you made, how would the editor modify it and save it back to files, without removing the dynamic content in the process?

By losing translations I gather you mean they "lose the ability to switch language afterwards and have the theme text for that template be updated without them resetting the customizations first", which seems like a fine compromise to me for this stage we are at.

Yes, so only non-customized templates would be translated.

If this is a temporary compromise, I think it's a good decision, but I guess we would be committing to supporting inline PHP and whatever solution we come up with in the future, right? Because long term, I don't think this is good enough.

@mcsf
Copy link
Contributor

mcsf commented May 6, 2020

Yes, so only non-customized templates would be translated.

If this is a temporary compromise, I think it's a good decision, but I guess we would be committing to supporting inline PHP and whatever solution we come up with in the future, right? Because long term, I don't think this is good enough.

As argued elsewhere, supporting multiple translations of user-edited content falls within the scope of Phase 4 — multilingual — of Gutenberg. So, in the long term, it's expected that this will be solved. However, until then, I think this is more than a "temporary compromise": it's a real win within the current expectations of WordPress, namely that defaults are translated but content isn't.

I think I asked in another thread what other scenarios we have of dynamic content that might not fall in either of these two buckets. Conditions are naturally something that needs to be handled (if there are no comments, don't show the comment count) but I think these could be modelled within the block definition in most cases given the nature of a purely dynamic block.

Also asking @ockham about this, since I believe you've been exposed to more such challenges.

but translatable strings for starter content is a specific and well defined problem that already has a solution in PHP

The more I look at it, the more I'm convinced of that too.

@epiqueras
Copy link
Contributor Author

it's a real win within the current expectations of WordPress, namely that defaults are translated but content isn't.

But now, templates become content after they are modified. I guess you could make the case that we are not making it any harder to re-translate things manually after customizations.

@ockham
Copy link
Contributor

ockham commented May 11, 2020

Dynamic urls, on the other hand, could be handled by augmenting src properties in block comment attributes to support a shape like src: { "context": "theme_root"; "path": "/images/header.png" }. This is akin to essentially making the whole block dynamic (no expectation of fallback rendering, which doesn't make sense for a theme anyways), which is the other path I'd suggest exploring: a custom block variation of wp:image that is wp:theme/image and makes assumptions over the source, or even the rendering fallback. We might even assume all wp:image in a template to be wp:theme/image given the template loader context — when saved statically by the user it'll became a regular wp:image pointing to the resource the user has declared.

This is similar to what I was suggesting in #21728 (blocks behaving differently, depending on whether they're being used in a post editing or site editing context).

I was specifically thinking that if we can assume that an image block in the site editor (wp:theme/image) will always need to refer to a theme root, we can use block context to make that version of the block watch out for the theme root (and other site-wide settings), as specified by a surrounding block (or just automatically provided in site-editing mode), and have it infer the actual image path from that (without needing to change the semantics of its existing attributes).

The same principle should work for translated strings: the site-editing version of a block always assumes it needs to translate its strings, and listens to a language context, provided by the site.

@epiqueras
Copy link
Contributor Author

That still has all the limitations I am mentioning, plus added complexity.

If we are already compromising on all of those things, we might as well just support inline PHP and avoid all the complexity of blocks with dual render paths.

@kjellr kjellr added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label May 12, 2020
@johnstonphilip
Copy link
Contributor

@epiqueras @youknowriad To clarify here, does support inline PHP mean that the theme files will be PHP files?

@epiqueras
Copy link
Contributor Author

Yeah, like this:

<!-- wp:cover {"minHeight":360,"customGradient":"linear-gradient(253deg,rgba(6,147,227,1) 0%,rgb(84,121,221) 100%)","align":"full"} -->
<div
	class="wp-block-cover alignfull has-background-dim has-background-gradient"
	style="background:linear-gradient(253deg,rgba(6,147,227,1) 0%,rgb(84,121,221) 100%);min-height:360px"
>
	<div class="wp-block-cover__inner-container">
		<!-- wp:paragraph {"align":"center"} -->
		<p class="has-text-align-center">
			<em><?php echo __( 'Category' ); ?></em>
		</p>
		<!-- /wp:paragraph -->
	</div>
</div>
<!-- /wp:cover -->

<!-- wp:query-loop -->
<!-- wp:post-title /-->

<!-- wp:post-content /-->
<!-- /wp:query-loop -->

@mcsf
Copy link
Contributor

mcsf commented May 20, 2020

Seems we're all converging around PHP being the right tool for the job of HTML generation as put forward by Riad in #21932 (comment), as long as we favour the idea that the user owns and manages a template after having touched it.

What else needs solving in order to move forward?

@youknowriad
Copy link
Contributor

I think ideally, we don't load all WP PHP functions when trying to load the templates, we should try to be very strict and limit dynamic aspects to somethings we "whitelist".

@mcsf
Copy link
Contributor

mcsf commented May 20, 2020

Is it a question of whitelisting or of figuring out how much of WP to load? I see different intentions behind the two options.

@youknowriad
Copy link
Contributor

the intentions are more about whitelisting what we want to allow/recommend on templates. (not sure we can limit things much tbh)

  • only allow i18n
  • potentially provide some helpers

@carolinan
Copy link
Contributor

Are there any side effects to using php files?

@epiqueras
Copy link
Contributor Author

@carolinan

#21932 (comment)

When starter content is modified, it's stored as static text on a user's templates CPT, and there are no longer concerns about making any of it translatable. I think declaring template files in PHP provided they resolve in a valid block tree seems very sensible to me.

That's a compromise we can make. But, it worsens the UX for both theme creators and users.

Theme creators would need to manually edit their template files to add translations and do so again every time they make modifications in the block editor.

Users would lose all translations for a template after even the slightest change. E.g., the color of something.

@carolinan
Copy link
Contributor

Thinking as a user, who uses more than one language and sometimes more than one language on the same site, I did not understand.

If I edit something would I loose the ability to change the language of the text that is controlled by the block and not by me, or would I loose the current translations?


Thinking as a theme developer, the file format doesn't matter as long as everything just works.

Having to add the translations back after edits won't work in the long run.
Is it something that could be done automatically when edited templates are exported?
If that is possible.... I'd never add a translation function again manually, I'd let the exporter do it all :p

But:
Using .php files and not being able to use any other PHP than i18n would feel really odd.
I think that the concept of using php files without php would be harder than a clean break.

Using .html files symbolizes the shift from old themes to new.

Is it going to affect which theme file that is being loaded? Are there any side effects when thinking about the template hierarchy (Is it going to require changes and lots of effort)? What happens when a theme author wants to add support for fse to an existing PHP based theme?

Are there other areas that would benefit from the templates being php files?

@youknowriad youknowriad added [Type] Discussion For issues that are high-level and not yet ready to implement. and removed [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Jun 4, 2020
@pascalknecht
Copy link

@epiqueras @mtias For the translation issue I am thinking of the following scenario: Is there any way of having functions like __ once for backend strings and once for frontend strings? The one for the frontend strings could then be hooked in by the popular translation plugins. Translations for the frontend are not dynamic content it can be saved like normal content. We just need a way to have a different set of function for strings with different context.

@epiqueras
Copy link
Contributor Author

@ellatrix @youknowriad @gziolo @mtias

What if we use a RichText format as an inline token for this?

@youknowriad
Copy link
Contributor

There's some related chatter here #23268 (comment)

I'm concerned about the complexity and the fragility of this solution. (Dynamic Formats are fragile like shown in asblocks). Anyway, I think we should probably start exploring php templates and potential tokens to have a clear idea.

@epiqueras
Copy link
Contributor Author

Why do you think they are fragile?

@youknowriad
Copy link
Contributor

This issue is an example of issues I run into #23428
I wonder if there are other similar issues too.

@epiqueras
Copy link
Contributor Author

Seems like Ella found the bug? #23428 (comment)

@gziolo gziolo mentioned this issue Mar 28, 2023
69 tasks
@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. and removed [Feature] Full Site Editing labels Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Technical Feedback Needs testing from a developer perspective. [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests