Add iOS support for custom bundles #107
Conversation
| let url = URL(string: siteUrl)! | ||
| .appendingPathComponent("wp-json") | ||
| .appendingPathComponent("__experimental") | ||
| .appendingPathComponent("wp-block-editor") | ||
| .appendingPathComponent("v1") | ||
| .appendingPathComponent( "editor-assets" ) |
There was a problem hiding this comment.
To simplify using different endpoints—e.g., it varies based on environment—we might use this path as a default if a full URL is not provided.
There was a problem hiding this comment.
This can/should be improved a bunch. Because it's the demo app, I just whipped up something quick.
In the real world, we'd probably use https://github.com/Automattic/wordpress-rs to find this endpoint in a programmatic way by scanning the wp-json endpoint for its entry. If it's not there, we'd assume the site doesn't support editor assets and default back to the bundled editor.
I think ... LMK if this doesn't seem like a good plan?
| URL(string: siteApiRoot)! | ||
| .appendingPathComponent("__experimental") | ||
| .appendingPathComponent("wp-block-editor") | ||
| .appendingPathComponent("v1") | ||
| .appendingPathComponent( "editor-assets" ) |
There was a problem hiding this comment.
To simplify using different endpoints—e.g., it varies based on environment—we might use this path as a default if a full URL is not provided.
There was a problem hiding this comment.
It would be far better to inject this, I think.
We already have it in the demo app, so I think it'd just be a matter of passing it in here?
| [!--- Scripts ---] | ||
| [!--- Styles ---] |
There was a problem hiding this comment.
After inserting the fetched editor assets, we need to insert the editor UI scripts and styles as well. The "remote" editor has a separate bundle from the bundled editor. Without this, the editor is never instantiated and displayed.
How might we ensure the asset tags and corresponding asset files are available and loaded?
There was a problem hiding this comment.
An aside: Once this new bundle feature is in place, we might one day consider aligning the asset loading strategy for both the bundled and remote editors to operate in the same way, where WordPress packages are externalized for both.
The primary differences is that the former bundles both the WordPress packages and editor UI into a single JavaScript file, the latter relies upon WordPress packages available in window.wp globals and its bundle only includes editor UI. See pbArwn-6yv-p2.
Aligning them might reduce the likelihood of bugs originating from build complexity or environment differences. I don't think aligning them is necessary for merging this functionality.
There was a problem hiding this comment.
It is likely unnecessary, but we might consider using valid HTML comments to avoid a situation where these strings are accidentally displayed in the WebView. I.e., <!--- Scripts ---> rather than [!--- Scripts ---].
There was a problem hiding this comment.
It is likely unnecessary, but we might consider using valid HTML comments to avoid a situation where these strings are accidentally displayed in the WebView. I.e., <!--- Scripts ---> rather than [!--- Scripts ---].
Yeah this is probably best
There was a problem hiding this comment.
How might we ensure the asset tags and corresponding asset files are available and loaded?
I think pretty much all of these use the same format (we add type="module" crossorigin to scripts and rel="stylesheet" crossorigin to styles), so we could probably build this up from the raw asset URLs ourselves?
where WordPress packages are externalized for both
Thanks for the explanation – I don't think I was familiar with it before. I think treating everything as though it's a remote editor will definitely avoid nasty surprises later. It should still be pretty predictable for the bundled editor (all of the "remote" things are still on-disk), so there doesn't seem to be much downside AFAICT.
| @@ -17,13 +18,15 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro | |||
| public weak var delegate: EditorViewControllerDelegate? | |||
|
|
|||
| /// A custom URL for the editor. | |||
There was a problem hiding this comment.
| /// A custom URL for the editor. |
| // If you want to load something that's not a manifest, specify the environment variable | ||
| if let editorURL = ProcessInfo.processInfo.environment["GUTENBERG_EDITOR_URL"].flatMap(URL.init) { |
There was a problem hiding this comment.
Unlikely a priority to start, but I imagine we'll need a way to load a fetched asset set/manifest and a local running server for the editor UI bundle. That would allow iterating on the editor UI while relying upon site-specific WordPress packages.
There was a problem hiding this comment.
Ah yeah I didn't consider this case. If you have a fetched asset set, is there any local editor code running? I think I assumed that the remote server's code would completely replace anything running on a dev server
What?
Why?
How?
Testing Instructions
Accessibility Testing Instructions
Screenshots or screencast