Editor: use site locale in editor iframe #63883
Conversation
ramonjd
added
[Type] Bug
|
Size Change: +178 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in b5e597f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10135965436
|
There was a problem hiding this comment.
This is just a test and doesn't look at anything aside from passing the correct locale attributes to the editor.
There are a few future rabbit holes.
For example, if the user locale is LTR, should block placeholders be direction:ltr?
Core block assets should be loaded for both RTL and LTR? But isolated to their corresponding window contexts 🙃
Line 255 in af2e21a
I only have a bit of time to devote to this, so happy to have folks nudge the ship along.
lib/block-editor-settings.php
Outdated
| $settings['locale'] = array( | ||
| 'site' => array( | ||
| 'lang' => $current_site_locale, | ||
| 'isRTL' => $current_site_is_rtl, |
There was a problem hiding this comment.
This is a first pass so everything is up for debate.
Another option is to just pass the site locale data, e.g.,
$settings['siteLocale'] = array(
'lang' => $current_site_locale,
'isRTL' => $current_site_is_rtl,
);A few things occurred to me:
settings.isRTLalready exists. In the editor, it's the current user locale. Should we ignore it, or add related propertes, e.g.,settings.lang,settings.siteLang, andsettings.siteIsRTL. Seems a bit clunky.- Rather than
isRTL: boolean, what aboutdir: 'ltr'|'rtl'? - Are block editor settings the right place for this data?
There was a problem hiding this comment.
@youknowriad What do you reckon about the locale data model here (and using it in block editor settings)?
Is there a better way to pass site and user locale data to the editor?
I looked at extending the site settings (/wp/v2/settings) endpoint, but it appears limited to site option values.
There was a problem hiding this comment.
I looked at extending the site settings (/wp/v2/settings) endpoint, but it appears limited to site option values.
This would have been my preference personally as this is just a site option (whether it's stored as an option or not is not really important IMO). cc @TimothyBJacobs maybe
There was a problem hiding this comment.
Yeah, it'd be a neat place to house this stuff.
In that endpoint, we'd add a is_rtl: true or dir: 'rtl' or whatever, that corresponds to the site locale.
The only conceptual agony I was going through was that this value can't be updated in the POST like the others, so it isn't neat, but the world is messy so I can get over it. Keen to hear what Timothy thinks.
Here's the output of the GET request the record:
await wp.data.resolveSelect( 'core' ).getSite()
Respone
{
"title": "gutenberg",
"description": "",
"url": "http://localhost:8888",
"email": "wordpress@example.com",
"timezone": "",
"date_format": "F j, Y",
"time_format": "g:i a",
"start_of_week": 1,
"language": "ar",
"use_smilies": true,
"default_category": 1,
"default_post_format": "0",
"posts_per_page": 10,
"show_on_front": "posts",
"page_on_front": 0,
"page_for_posts": 0,
"default_ping_status": "open",
"default_comment_status": "open",
"site_logo": null,
"site_icon": 0
}
There was a problem hiding this comment.
This is exactly what the index is really for. The rest_index filter can be used.
There was a problem hiding this comment.
Thanks for confirming @TimothyBJacobs I'll take a look at using the filter 🙇🏻
| Block placeholders are "technically" part of the editor UI, | ||
| and should therefore match the editor UI's directionality. | ||
| */ | ||
| .block-editor-block-list__layout .components-placeholder { |
There was a problem hiding this comment.
This change is optional (follow up?), and there's probably a better way.
The CSS rule here is in lieu of setting the dir inside the individual blocks.
With a site lang of ar and a user lang of en:
| Before | After |
|---|---|
With a site lang of en and a user lang of `ar:
| Before | After |
|---|---|
There was a problem hiding this comment.
Maybe we could compose <Placeholder /> for blocks and do all the locale trickery in the higher order component?
E.g., const BlockPlaceholder = withLocale( Placeholder ), or whatever it is
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ramonjd
force-pushed
the
try/set-lang-and-dir-iframe-html
branch
from
4395b60
to
7f017d5
Compare
There was a problem hiding this comment.
Do we have an idea about the classic editor and how it handles this case?
Also the other questions I have (maybe things to address separately):
- we have block styles
style.cssandeditor.cssandtheme.cssand If I'm not wrong each one of these has "rtl" versions. I'm guessing style and theme need to use the "site locale" but it's unclear what "editor" should be using. - Do we have any support for automatic RTL for theme.json generated styles and how does it impact this PR?
- What about the uniframed block editor, do we ignore that and consider that most people should migrate to iframed editor eventually?
lib/block-editor-settings.php
Outdated
| $settings['locale'] = array( | ||
| 'site' => array( | ||
| 'lang' => $current_site_locale, | ||
| 'isRTL' => $current_site_is_rtl, |
There was a problem hiding this comment.
I looked at extending the site settings (/wp/v2/settings) endpoint, but it appears limited to site option values.
This would have been my preference personally as this is just a site option (whether it's stored as an option or not is not really important IMO). cc @TimothyBJacobs maybe
| isPreviewMode: settings.__unstableIsPreviewMode, | ||
| }; | ||
| }, [] ); | ||
| const { resolvedAssets, isPreviewMode, siteLocale, userLocale } = useSelect( |
There was a problem hiding this comment.
Why do we need both siteLocale and userLocale? For me userLocale is just isRTL so we don't really need a new setting there.
There was a problem hiding this comment.
Good question.
The block editor should know about both site locale and user locale because:
- it needs both the site lang and site RTL value to add the requisite attributes to the document (so that the content matches the frontend)
- it should have the user locale available in case the editor canvas needs to render any editor-specific UI, such as placeholders.
On point 2, I could be wrong in my assumption, but my instinct tells me that the block placeholders are part of the editor UI, and therefore, the locales should be consistent.
| Before | After |
|---|---|
The iframe may not need to access userLocale here at all, but here I just wanted to provide an example of how to make the placeholder UI conform to the user locale (and not the site). See above.
I'm thinking a HOC placeholder in block-editor would be a better place to add the lang and dir attributes to the <Placeholders />, not the funky CSS I have in the latest commit of this PR. Anyway, that would be a follow up.
There was a problem hiding this comment.
No, my question was simpler than that :P. We already have wp.i18n.isRTL function to know whether the user local is rtl or not, we don't need a new setting for that.
There was a problem hiding this comment.
You know I love typing 
I think I remember realizing we just needed the site locale.
So, yeah, good point. "Locale" comprises language too, but wp.i18n.getLocaleData()[""].lang should get us the value for the lang attribute too should we need it. 👍🏻
It uses the user locale as far as I can see. There are a couple of related tickets: See:
I was discussing the RTL CSS generation/enqueuing question with @andrewserong today, and yeah, it's something that needs to be address, but in a follow up. In my mind whatever is on the editor canvas — the content that represents what we see on the frontend — should follow the site locale. The editor UI should, in theory, do what it does now and use the current user locale (if set). This PR deals with HTML semantics pretty much.
I thought about that for 1 second, and concluded it should follow whatever the "app" does, so the user locale as the primary fallback. All the above sounds easy when I write it down, but I haven't looked too deeply into the how yet. I haven't had much time to spend on this, which probably shows, but I think it's important enough to fix so I'll plug away at it when I can. |
|
Yeah, for me the most challenging thing is going to be the CSS loading and how to choose whether we pick RTL or not ... As it's not clear for some third-party CSS files whether they're targetting content or placeholders... |
ramonjd
force-pushed
the
try/set-lang-and-dir-iframe-html
branch
from
27c7e4a
to
9a53507
Compare
ramonjd
force-pushed
the
try/set-lang-and-dir-iframe-html
branch
from
9506188
to
0053e22
Compare
|
Wondering what's needed to bring this closer to finish line. |
Tentatively adding a rule to ensure that block placeholders, which are part of the editor UI, conform to the directionality of the user locale
Block placeholders are "technically" part of the editor UI, and should therefore match the editor UI's directionality. ! A better approach might be to create a HOC wrapping Placeholder in @wordpress/block-editor that sets the directionality and lang of the Placeholder component
Passing locale data down to the iframe from the editor package instead of through the block settings.
ramonjd
force-pushed
the
try/set-lang-and-dir-iframe-html
branch
from
0053e22
to
a309123
Compare
Probably a way to ensure templates/template parts and other site content is correctly translated into the site language. I'm not sure the best way to go about it. Maybe sending a `locale value to the APIs. Also, loading any I don't have much time to carry on with this unfortunately, but will keep it on the back burner unless someone decides to pick it up. |
What? How?
(Only partly) fixes #52777
A first attempt at settings the lang and dir attributes in the editor iframe.
This PR doesn't look at translations of theme templates.
This PR
langanddirattributes of the html element in the iframertlclassname to the iframe body tag if necessarydir(the user locale)Why?
So that the editor canvas correctly reflects the current site locale and not the user/editor locale.
Testing Instructions
Editor canvas
To test, your user locale should be different to the site locale. By default the user locale is set to the site locale.
Edit your user settings over at
/wp-admin/users.php, and set a LTR language, such as English or German, as your user locale.Set an RTL language, such as Arabic or Hebrew, as the site locale in /wp-admin/options-general.php
In the Post and Site editors, check that the
<html />element contains the correctlanganddirattributes (from the site).Now swap the languages: use the RTL language as your user locale and the LTR as your site locale.
Check that the
<html />element contains the correctlanganddirattributes (from the site).Block/pattern previews
This change should also affect block/pattern previews. To test, either check a theme's pattern library in the Site Editor, insert a Query Block and view the suggested patterns, or open the block or pattern library from the block inserter.
Check that the previews have the correct locale attributes.
Screenshots or screencast
User locale: German (de_DE)
Site locale: Hebrew (he_IL)
Patterns
User locale: English (en_US)
Site locale: Arabic (ar)
Related
langattributes andrtlclass when available sabernhardt/gutenberg#2