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

Load requested language by URL #1703

Closed
threadi opened this issue Aug 23, 2024 · 19 comments
Closed

Load requested language by URL #1703

threadi opened this issue Aug 23, 2024 · 19 comments

Comments

@threadi
Copy link

threadi commented Aug 23, 2024

The path from international WordPress plugin sites to the Playground is currently still somewhat confusing. If you click on a live preview button on https://de.wordpress.org, for example, you always end up in an English environment - not the German one.

Currently, the plugin-directory plugin calls this URL from the Playground, regardless of which language is actually used:

https://playground.wordpress.net/?plugin=%s&login=1&url=%s

Source: https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-template.php?rev=12931#L734

I think it would be easy to read the language in the plugin-directory plugin. But what would the above URL have to look like for playground to automatically load a requested language?

Example: https://de.wordpress.org/plugins/external-files-in-media-library/ - always loads in an English environment.

@brandonpayton
Copy link
Member

Thank you for for reporting this, @threadi. We probably should support a query param for language/locale.

@brandonpayton brandonpayton moved this from Inbox to Needs Triage/Our Reply in Playground Board Aug 24, 2024
@bgrgicak
Copy link
Collaborator

We could use the language query parameter to set the language.

@threadi
Copy link
Author

threadi commented Aug 26, 2024

Thanks for the tip. Unfortunately, the parameter does not work for the call that would be initiated.

Example: https://playground.wordpress.net/?plugin=external-files-in-media-library&language=de_DE&networking=yes&blueprint-url=https://wordpress.org/plugins/wp-json/plugins/v1/plugin/external-files-in-media-library/blueprint.json?rev=3141102

Even without networking=yes it does not work (although this is already set within the blueprint.json.

@brandonpayton
Copy link
Member

We could use the language query parameter to set the language.

Thanks for pointing this out, @bgrgicak 🙇 .

@threadi, it turns out that mixing other Playground query params with a blueprint_url argument is not guaranteed to work because the Blueprint takes precedence. But there is a setSiteLanguage Blueprint step that can be used instead.

Here is an example of your external Blueprint adapted to use this step:
Example in the Blueprint Builder

@threadi
Copy link
Author

threadi commented Aug 26, 2024

I know about the options in blueprint.json, they are not the problem. Please take another look at my initial question. The issue is that when you click on “Live-Preview” on the plugin pages via the code stored in https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-template.php?rev=12931#L734, you are currently not redirected to the playground in a language-specific manner. The transfer of the language parameter for language-specific plugin pages to the playground is missing here. As written, it will certainly be possible to read out the language. The question is how to use this language to load the playground in the language the user is using.

A plugin on the Spanish pages should load the Spanish backend in the Playground during “Live-Preview”.
A plugin on the German pages should load the German backend in the Playground for “Live-Preview”.

If it were possible to access an HTTP request parameter in blueprint.json to read the language parameter from the URL, you could perhaps use setSiteLanguage. But since it is a JSON and not a PHP file, I don't see any chance of that.

What would have to be done to enable the Playground to do this?

@adamziel adamziel moved this from Needs Author's Reply to Inbox in Playground Board Aug 26, 2024
@carstingaxion
Copy link

carstingaxion commented Aug 27, 2024

I was looking for a way to do the same; so I agree absolutely in the need for a language query param that is compatible with a blueprint-url param.

I would love to run a Playground in a matrix of locales, as a Github actions workflow to generate screenshots of a plugin for the w.org repository. The possibility to set the language from outside the blueprint.json is the last missing puzzle-piece in GatherPress' migration from wp-env to wp-playground/cli for generating its screenshots.

@carstingaxion
Copy link

For now, I solved this by merging a dynamic setSiteLanguage step into my blueprint. This works fine in a workflow like so:

  - name: Prepare localized blueprint
    # en_US should not get any additional steps added, as this will result in errors!
    #
    # All other locales need this step!
    run: |
      if [ ${{ matrix.locale }} == 'en_US' ]; then
        cp .github/scripts/wordpress-org-screenshots/blueprint.json localized_blueprint.json
      else
        language_step='[
          {
            "step": "setSiteLanguage",
            "language": "${{ matrix.locale }}"
          }
        ]'
        echo "Use jq to append the site language step to the existing blueprint JSON file."
        jq --argjson languageStep "$language_step" '.steps += $languageStep' .github/scripts/wordpress-org-screenshots/blueprint.json > localized_blueprint.json
      fi

  - name: Starting Playground
    # The "&" is important to allow the next step to start!
    run: |
      ./node_modules/@wp-playground/cli/wp-playground.js server --blueprint=./localized_blueprint.json &

@brandonpayton
Copy link
Member

brandonpayton commented Sep 4, 2024

I know about the options in blueprint.json, they are not the problem. Please take another look at my initial question. The issue is that when you click on “Live-Preview” on the plugin pages via the code stored in...

The question is how to use this language to load the playground in the language the user is using.

Sorry, @threadi, I see now.

I don't have time left today but made added this to my list for tomorrow.

If it were possible to access an HTTP request parameter in blueprint.json to read the language parameter from the URL, you could perhaps use setSiteLanguage. But since it is a JSON and not a PHP file, I don't see any chance of that.

I was thinking about something similar: What if we added support for Blueprint arguments?

Blueprints could declare argument names, types, and default values. Then we could either support text replacement or (probably much better) a structured way to express variable interpolation throughout the Blueprint.

^ @adamziel, @bgrgicak, and @dmsnell, do you have any thoughts on this?

@bgrgicak
Copy link
Collaborator

bgrgicak commented Sep 9, 2024

Blueprints could declare argument names, types, and default values. Then we could either support text replacement or (probably much better) a structured way to express variable interpolation throughout the Blueprint.

This sounds interesting but could add a lot of complexity. There is something nice about having a static JSON file that we can just read and the user can always expect the same result.

Would it work if query strings would override blueprints?
In this example, we could swap the setSiteLanguage step value while building the blueprint if we detect a language query string.
This would make the blueprint dynamic without the extra complexity that variables would add.

@threadi
Copy link
Author

threadi commented Sep 9, 2024

In the meantime I also had another idea (or 2) which doesn't concern the Playground but wordpress.org. The link containing the path to blueprint.json is generated at https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-template.php?rev=12931#L734.

Idea 1: language-specific blueprints.json at wordpress.org. Each plugin developer could provide these separately and thus ensure a switch themselves. There are certainly plugins that are fixed to one region of the world with a specific language, which would then have everything in their hands. They could then also provide individual language-specific content that they define in the blueprint.json (which would not be possible with the idea developed here so far).

Idea 2: wordpress.org could generate a language-specific file from the basic blueprint.json and pass it on to the Playgroud. The challenge here is that such files must also be saved somewhere and somehow.

Both would relieve you of the burden, but I still see major issues with both ideas that need to be clarified “over there”. I'll create a ticket there in the next few days. From my point of view as a plugin developer, my idea 1 would currently be the preferred way and would also take the work off your hands ;)

@adamziel
Copy link
Collaborator

adamziel commented Sep 11, 2024

The easiest solution here would be what @bgrgicak proposed – to still apply the language query arg even when the Blueprint is provided, as long as the Blueprint contains no setSiteLanguage step. I'd just hold off with that until #1731 lands as it profoundly changes the state management logic and I'd rather avoid building that feature twice.

@adamziel adamziel moved this from Inbox to Blocked in Playground Board Sep 11, 2024
brandonpayton added a commit that referenced this issue Sep 27, 2024
… WebApp Redesign (#1731)

## Description

Implements a large part of the [website
redesign](#1561):

![CleanShot 2024-09-14 at 10 24
57@2x](https://github.com/user-attachments/assets/f245c7ac-cb8c-4e5a-b90a-b4aeff802e7b)


High-level changes shipped in this PR:

* Multiple Playgrounds. Every temporary Playground can be saved either
in the browser storage (OPFS) or in a local directory (Chrome desktop
only for now).
* New Playground settings options: Name name, language, multisite
* URL as the source of truth for the application state
* State management via Redux

This work is a convergence of 18+ months of effort and discussions. The
new UI opens relieves the users from juggling ephemeral Playgrounds and
losing their work. It opens up space for long-lived site configurations
and additional integrations. We could bring over all the [PR previewers
and demos](https://playground.wordpress.net/demos/) right into the
Playground app.

Here's just a few features unblocked by this PR:

* #1438 – no
more losing your work by accident 🎉
* #797 – with
multiple sites we can progressively build features we'll eventually
propose for WordPress core:
* A Playground export and import feature, pioneering the standard export
format for WordPress sites.
* A "Clone this Playground" feature, pioneering the [Site Transfer
Protocol](https://core.trac.wordpress.org/ticket/60375).
   * A "Sync two Playgrounds" feature, pioneering the Site Sync Protocol
* #1445 – better
git support is in top 5 most highly requested features. With multiple
Playgrounds, we can save your work and get rid of the "save your work
before connecting GitHub or you'll lose it" and cumbersome "repo setup"
forms on every interaction. Instead, we can make git operations like
Pull, Commit, etc. very easy and even enable auto-syncing with a git
repository.
* #1025 – as we
bring in more PHP plumbing into this repository, we'll replace the
TypeScript parts with PHP parts to create a WordPress core-first
Blueprints engine
* #1056 – Site
transfer protocol will unlocks seamlessly passing Playgrounds between
the browser and a local development environment
* #1558 – we'll
integrate [the Blueprints directory] and offer single-click Playground
setups, e.g. an Ecommerce store or a Slide deck editor.
#718.
* #539 – the
recorded Blueprints would be directly editable in Playground and perhaps
saved as a new Playground template
* #696 – the new
interaction model creates space for additional integrations.
* #707 – you
could create a "GitHub–synchronized" Playground
* #760 – we can
bootstrap one inside Playground using a Blueprint and benefit the users
immediately, and then gradually work towards enabling it on
WordPress.org
* #768 – the new
UI has space for a "new in Playground" section, similar to what Chrome
Devtools do
* #629  
* #32
* #104
* #497
* #562
* #580 

### Remaining work

- [ ] Write a release note for https://make.wordpress.org/playground/
- [x] Make sure GitHub integration is working. Looks like OAuth
connection leads to 404.
- [x] Fix temp site "Edit Settings" functionality to actually edit
settings (forking a temp site can come in a follow-up PR)
- [x] Fix style issue with overlapping site name label with narrow site
info views
- [x] Fix style issue with bottom "Open Site" and "WP Admin" buttons
missing for mobile viewports
- [x] Make sure there is a path for existing OPFS sites to continue to
load
- [x] Adjust E2E tests.
- [x] Reflect OPFS write error in UI when saving temp site fails
- [x] Find a path forward for
[try-wordpress](https://github.com/WordPress/try-wordpress) to continue
working after this PR
- [x] Figure out why does the browser get so choppy during OPFS save. It
looks as if there was a lot of synchronous work going on. Shouldn't all
the effort be done by a worker a non-blocking way?
- [x] Test with Safari and Firefox. Might require a local production
setup as FF won't work with the Playground dev server.
- [x] Fix Safari error: `Unhandled Promise Rejection: UnknownError:
Invalid platform file handle` when saving a temporary Playground to
OPFS.
- [x] Fix to allow deleting site that fails to boot. This is possible
when saving a temp site fails partway through.
- [x] Fix this crash:

```ts
		/**
		 * @todo: Fix OPFS site storage write timeout that happens alongside 2000
		 *        "Cannot read properties of undefined (reading 'apply')" errors here:
		 * I suspect the postMessage call we do to the safari worker causes it to
		 * respond with another message and these unexpected exchange throws off
		 * Comlink. We should make Comlink ignore those.
		 */
		// redirectTo(PlaygroundRoute.site(selectSiteBySlug(state, siteSlug)));
```
- [x] Test different scenarios manually, in particular those involving
Blueprints passed via hash
- [x] Ensure we have all the aria, `name=""` etc. accessibility
attributes we need, see AXE tools for Chrome.
- [x] Update developer documentation on the `storage` query arg (it's
removed in this PR)
- [x] Go through all the `TODOs` added in this PR and decide whether to
solve or punt them
- [x] Handle errors like "site not found in OPFS", "files missing from a
local directory"
- [x] Disable any `Local Filesystem` UI in browsers that don't support
them. Don't just hide them, though. Provide a help text to explain why
are they disabled.
- [x] Reduce the naming confusion, e.g. `updateSite` in redux-store.ts
vs `updateSite` in `site-storage.ts`. What would an unambiguous code
pattern look like?
- [x] Find a reliable and intuitive way of updating these deeply nested
redux state properties. Right now we do an ad-hoc recursive merge that's
slightly different for sites and clients. Which patterns used in other
apps would make it intuitive?
- [x] Have a single entrypoint for each logical action such as "Create a
new site", "Update site", "Select site" etc. that will take care of
updating the redux store, updating OPFS, and updating the URL. My ideal
scenario is calling something like `updateSite(slug, newConfig)` in a
React Component and being done without thinking "ughh I still need to
update OPFS" or "I also have to adjust that .json file over there"
- [x] Fix all the tiny design imperfections, e.g. cut-off labels in the
site settings form.

### Follow up work

- [ ] Mark all the related blocked issues as unblocked on the project
board, e.g.
#1703,
#1731, and more –
[see the All Tasks
view](https://github.com/orgs/WordPress/projects/180/views/2?query=sort%3Aupdated-desc+is%3Aopen&filterQuery=status%3A%22Up+next%22%2C%22In+progress%22%2C%22Needs+review%22%2C%22Reviewed%22%2C%22Done%22%2CBlocked)
- [ ] Update WordPress/Learn#1583 with info
that the redesign is now in and we're good to record a video tutorial.
- [ ] #1746
- [ ] Write a note in [What's new for developers? (October
2024)](WordPress/developer-blog-content#309)
- [ ] Document the new site saving flow in
`packages/docs/site/docs/main/about/build.md` cc @juanmaguitar
- [ ] Update all the screenshots in the documentation cc @juanmaguitar 
- [ ] When the site fails to load via `.list()`, still return that
site's info but make note of the error. Not showing that site on a list
could greatly confuse the user ("Hey, where did my site go?"). Let's be
explicit about problems.
- [ ] Introduce notifications system to provide feedback about outcomes
of various user actions.
- [ ] Add non-minified WordPress versions to the "New site" modal.
- [ ] Fix `console.js:288 TypeError: Cannot read properties of undefined
(reading 'apply') at comlink.ts:314:51 at Array.reduce (<anonymous>) at
callback (comlink.ts:314:29)` – it seems to happen at trunk, too.
- [ ] Attribute log messages to the site that triggered them.
- [ ] Take note of any interactions that we find frustrating or
confusing. We can perhaps adjust them in a follow-up PR, but let's make
sure we notice and document them here.
- [ ] Solidify the functional tooling for transforming between `URL`,
`runtimeConfiguration`, `Blueprint`, and `site settings form state` for
both OPFS sites and in-memory sites. Let's see if we can make it
reusable in Playground CLI.
- [ ] Speed up OPFS interactions, saving a site can take quite a while.
- [ ] A mobile-friendly modal architecture that doesn't stack modals,
allows dismissing, and understands some modals (e.g. fatal error report)
might have priority over other modals (e.g. connect to GitHub). Discuss
whether modals should be declared at the top level, like here, or
contextual to where the "Show modal" button is rendered.
- [ ] Discuss the need to support strong, masked passwords over a simple
password that's just `"password"`.
- [ ] Duplicate site feature implemented as "Export site + import site"
with the new core-first PHP tools from
adamziel/wxr-normalize#1 and
https://github.com/adamziel/site-transfer-protocol
- [x] Retain temporary sites between site changes. Don't just trash
their iframe and state when the user switches to another site.

Closes #1719

cc @brandonpayton

---------

Co-authored-by: Brandon Payton <[email protected]>
Co-authored-by: Bero <[email protected]>
Co-authored-by: Bart Kalisz <[email protected]>
@adamziel
Copy link
Collaborator

Solved by #1731

@github-project-automation github-project-automation bot moved this from Blocked to Done in Playground Board Sep 27, 2024
@bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak reopened this Sep 30, 2024
@github-project-automation github-project-automation bot moved this from Done to Inbox in Playground Board Sep 30, 2024
@bgrgicak bgrgicak moved this from Inbox to Up next in Playground Board Sep 30, 2024
@adamziel
Copy link
Collaborator

adamziel commented Oct 7, 2024

@bgrgicak oh that was intentional. The logic is as follows – it only sets a site language when the Blueprint doesn't explicitly provide one:

	// Language
	if (query.get('language')) {
		if (
			!blueprint?.steps?.find(
				(step) => step && (step as any).step === 'setSiteLanguage'
			)
		) {
			blueprint.steps?.push({
				step: 'setSiteLanguage',
				language: query.get('language')!,
			});
		}
	}

Do you think it makes more sense to override the Blueprint step? If so, why? And how would that generalize to other query API parameters?

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 8, 2024

Do you think it makes more sense to override the Blueprint step? If so, why? And how would that generalize to other query API parameters?

TL;DR; I would like for the query API to override blueprints.

I've seen a few cases where users asked (or reported bugs) because the query API didn't work.
The underlying issue was that they had a blueprint and wanted to adjust Playground using the query API.

I would prefer to set a policy for the entire query API instead of making exceptions.

Looking at these reports it makes sense to me that the query API can override blueprints.

A blueprint is harder to modify and sometimes you don't even want to change it, for example in Plugin previews, but you still might want to adjust something on the fly. Today the only option is to modify the blueprint, but if the query API overrides blueprints we get a lot more flexibility in how Playground is used.

@adamziel
Copy link
Collaborator

adamziel commented Oct 8, 2024

@bgrgicak you have a point. If the Query API was to have precedence in the browser, the CLI args should also have precedence in @wp-playground/cli. I wonder how much is this a special case of overrides, and how much are we dealing with the problem of either merging Blueprints or having "variables", e.g. there isn't much difference between saying ?language=pl#{"language":"en"} and saying $lang=pl#{"language": $lang}. I'm firmly against introducing variables as long as possible and perhaps these overrides would make a neat replacement.

@adamziel
Copy link
Collaborator

adamziel commented Oct 8, 2024

On the flip side, one could argue that #1797 is caused by a faulty assumption in the site settings form, namely that it uses the query parameters instead of updating the Blueprint. In my mind, that form is the first live version of the visual Blueprints builder and, eventually, it may have UI for editing every single step.

Furthermore, you could override the site language via query params, but the semantic isn't so clear when the Blueprint says {"plugins": ["gutenberg"]} and the query API says ?plugin=hello-dolly. Should the query API plugins replace the Blueprint plugins? Or should they be prepended? Or appended? And is there a way to make that choice crystal clear, so that one doesn't need to check with the documentation?

cc @brandonpayton for thoughts

@bgrgicak
Copy link
Collaborator

bgrgicak commented Oct 9, 2024

Furthermore, you could override the site language via query params, but the semantic isn't so clear when the Blueprint says {"plugins": ["gutenberg"]} and the query API says ?plugin=hello-dolly. Should the query API plugins replace the Blueprint plugins? Or should they be prepended? Or appended? And is there a way to make that choice crystal clear, so that one doesn't need to check with the documentation?

That reminds me of the puzzle app, I had to make the same decisions there while merging blueprints.
We could easily decide what to do in each merge scenario, but as you mentioned, it wouldn't be obvious to users.

@adamziel
Copy link
Collaborator

adamziel commented Oct 16, 2024

I'm closing this for now. The solution today would be: do not include the site language in the Blueprint if you plan to override the language via the query API. If that doesn't help with your use-case, please comment on this issue. We'll revisit this given enough interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants