Map block: fix script loading to work with FSE #19552

glendaviesnz · 2021-04-16T02:56:19Z

Description

Possible fix for: #19482

Currently the Jetpack Map block fails to load in the Site Editor context because the mapbox-gl.js file is loaded in the main window context, but the block editor is running in an iframe when in the site editor context.

To get around this issue this PR adds a supporting library which can load editor asset files into the correct window context of the current editor.

This issue as been raised for discussion at WordPress/gutenberg#31022 in order to see if there is a broader solution that might be possible within the core Gutenberg plugin.

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Check out this PR and run in local env along with latest Gutenberg plugin and an FSE compatible theme
Add a Map block in both a post and in the site editor and check that the map loads in the editor in both places, and markers can be added
Check the maps display in the front end in post body and in site headers or footers

Please note: This is still a PoC solution and open for review on the general approach. Waiting for feedback on WordPress/gutenberg#31022 before merging

matticbot · 2021-04-16T02:56:27Z

Caution: This PR has changes that must be merged to WordPress.com
Hello glendaviesnz! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D60271-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

github-actions · 2021-04-16T02:57:05Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: June 1, 2021.
Scheduled code freeze: May 24, 2021.

glendaviesnz · 2021-04-16T02:58:30Z

projects/plugins/jetpack/extensions/blocks/map/component.js

-		} );
+
+		const { vendorWindow } = getVendorLoadContext( this.mapRef.current );
+


Key todo with this approach is working out how to load these from local resource instead of remote

Would Jetpack's window.Jetpack_Block_Assets_Base_Url be helpful here?

Thanks, that will be handy once I have worked out how to either get the webpack hashed filename, or manually set the vendor bundle hashed filename, slowly making progress on that.

simison · 2021-04-16T09:18:14Z

If this works, seems like a great utility for Gutenberg plugin!

simison · 2021-04-16T09:22:20Z

projects/plugins/jetpack/extensions/blocks/map/component.js

 	loadMapLibraries() {
 		const { apiKey } = this.props;
-		Promise.all( [
-			import( /* webpackChunkName: "map/mapbox-gl" */ 'mapbox-gl' ),
-			import( /* webpackChunkName: "map/mapbox-gl" */ 'mapbox-gl/dist/mapbox-gl.css' ),


Note that by default Webpack dynamic imports are lazy, i.e. loaded asynchronously. Just noting in case you wanna ensure the same loading method with <script async>?

Thanks. With the way this is currently setup the script tag is not injected into the iframe header until after the page has loaded so adding async may not make any difference, but will do some double checking on that.

glendaviesnz · 2021-04-19T04:24:29Z

If this works, seems like a great utility for Gutenberg plugin!

For sure, that is the plan 😄

ramonjd · 2021-04-22T10:30:45Z

This is working well on a standalone Wordpress install.

In the Site Editor on WPCOM however, the map container isn't recognised:

Suspecting this line, but ran out of time for debugging.

Will browser test tomorrow. 👍

glendaviesnz · 2021-04-22T21:35:22Z

In the Site Editor on WPCOM however, the map container isn't recognised:

It initialised the map ok for me on wpcom site editor:

I wonder what is different about your site setup?

ramonjd · 2021-04-23T01:15:31Z

I wonder what is different about your site setup?

I didn't notice that the mapbox-gl js had not been copied over to the D60271-code diff.

Thanks @glendaviesnz for updating it! Works a treat now.

ramonjd

I have tested this in both the block and site editors in Chrome, FF, Safari (MacOS and iOS) and Edge.

Maps load as expected in the editors and on the frontend.

I cleaned up my build dir and rebuilt the project. The map dependencies are copied over to _inc/blocks/editor-assets as expected.

jeherve

Approving this for now, this tests well for me. Marking the PR as on hold pending Gutenberg discussion.

glendaviesnz · 2021-05-12T03:40:02Z

There is nothing in core gutenberg at the moment that can be used in place if this approach, so probably best to merge this and then circle back if core adds some APIs for this. Will rebase and get it sorted for merging tomorrow

andrewserong

Smoke tested again after the rebase in the site editor (iframed in wpcom and via wp-admin) and on a non-FSE site in the post editor. LGTM! And good idea getting this in as a shorter-term fix until there's a more general approach for handling loading 3rd party scripts in Gutenberg.

jeherve

This tests well for me. 🚢

glendaviesnz · 2021-05-13T21:57:27Z

@jeherve the wpcom diff failed because I used fs-extra in one of the scripts and this wasn't included in the package.json (not sure why it worked locally without installing this dependency). I have added it as a dev dependency and D60271-code built ok after that. Not sure if it is ok to add this extra dependency, let me know if not and I will look at options for handling the file copying without it.

glendaviesnz · 2021-05-17T06:28:43Z

An api was just added to core that we may be able to use for this - WordPress/gutenberg#31873, so just looking into that first, should no one way or the other tomorrow

jeherve · 2021-05-18T16:16:09Z

An api was just added to core that we may be able to use for this

Noting that Jetpack supports the current version of WordPress -1, so we may not be able to use this just yet.

samiff · 2021-05-19T22:19:09Z

@glendaviesnz Hey 👋 Is this PR ready to merge, or are you still researching?

glendaviesnz · 2021-05-20T01:36:57Z

@samiff, @jeherve WordPress/gutenberg#31873 has been merged and will work as a better long term solution to this problem. It could be used now as Jetpack doesn't need to know about the api, it hooks into the existing enqueue hooks in the background, so the resources just wouldn't load in the site editor iframe in older versions of WP without the gutenberg plugin, but these sites are not going to be able to use the newer versions of the site editor anyway.

However, given that it is only just added to trunk and as a new api may be subject to some immediate changes, I think we should just stick with this solution as is, and I will add an issue to follow up at some point in the future an refactor this to make use of the new api.

glendaviesnz · 2021-05-20T01:55:00Z

Issue for follow up refactor added at #19896

github-actions · 2021-05-20T02:16:32Z

Great news! One last step: head over to your WordPress.com diff, D60271-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

glendaviesnz · 2021-05-20T02:47:47Z

rWP225980

glendaviesnz added [Status] In Progress [Block] Map labels Apr 16, 2021

glendaviesnz self-assigned this Apr 16, 2021

matticbot added Touches WP.com Files labels Apr 16, 2021

github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Apr 16, 2021

glendaviesnz commented Apr 16, 2021

View reviewed changes

simison reviewed Apr 16, 2021

View reviewed changes

glendaviesnz mentioned this pull request Apr 21, 2021

Site Editor: custom block editor javascript assets don't load in iframe context WordPress/gutenberg#31022

Closed

glendaviesnz requested a review from a team April 21, 2021 02:15

glendaviesnz added [Status] Needs Team Review and removed [Status] In Progress labels Apr 21, 2021

github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Apr 21, 2021

glendaviesnz added DO NOT MERGE don't merge it! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Apr 21, 2021

ramonjd previously approved these changes Apr 23, 2021

View reviewed changes

glendaviesnz added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Team Review labels Apr 23, 2021

jeherve previously approved these changes Apr 23, 2021

View reviewed changes

jeherve added [Status] Blocked / Hold and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 23, 2021

glendaviesnz removed the DO NOT MERGE don't merge it! label May 12, 2021

Add missing doc comment

98a3342

glendaviesnz added [Status] Needs Team Review and removed [Status] Blocked / Hold labels May 12, 2021

andrewserong previously approved these changes May 12, 2021

View reviewed changes

andrewserong added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Team Review labels May 12, 2021

jeherve previously approved these changes May 13, 2021

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels May 13, 2021

jeherve added this to the jetpack/9.8 milestone May 13, 2021

Add missing dependency

9946a98

glendaviesnz dismissed stale reviews from jeherve and andrewserong via 9946a98 May 13, 2021 21:25

jeherve approved these changes May 14, 2021

View reviewed changes

glendaviesnz mentioned this pull request May 20, 2021

Map block: update to use new gutenberg API #19896

Open

glendaviesnz merged commit a5f167c into master May 20, 2021

glendaviesnz deleted the update/map-block-for-fse branch May 20, 2021 02:15

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 20, 2021

glendaviesnz mentioned this pull request May 20, 2021

Site Editor Compatibility: Map block #19482

Closed

glendaviesnz mentioned this pull request Dec 1, 2022

Post editor: iframe for block-based themes WordPress/gutenberg#46212

Merged

miksansegundo mentioned this pull request May 23, 2023

Site Assembler - Patterns with maps don't render sometimes because the domReady is undefined Automattic/wp-calypso#76936

Closed

anomiex mentioned this pull request Sep 23, 2024

Slideshow block has duplicated caption in the Editor for sites with FSE themes. #39459

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Map block: fix script loading to work with FSE #19552

Map block: fix script loading to work with FSE #19552

glendaviesnz commented Apr 16, 2021 •

edited

Loading

matticbot commented Apr 16, 2021

github-actions bot commented Apr 16, 2021 •

edited

Loading

glendaviesnz Apr 16, 2021

simison Apr 16, 2021

glendaviesnz Apr 19, 2021

simison commented Apr 16, 2021

simison Apr 16, 2021

glendaviesnz Apr 19, 2021

glendaviesnz commented Apr 19, 2021

ramonjd commented Apr 22, 2021

glendaviesnz commented Apr 22, 2021

ramonjd commented Apr 23, 2021

ramonjd left a comment

jeherve left a comment

glendaviesnz commented May 12, 2021

andrewserong left a comment

jeherve left a comment

glendaviesnz commented May 13, 2021

glendaviesnz commented May 17, 2021

jeherve commented May 18, 2021

samiff commented May 19, 2021

glendaviesnz commented May 20, 2021

glendaviesnz commented May 20, 2021

github-actions bot commented May 20, 2021

glendaviesnz commented May 20, 2021

		} );

		const { vendorWindow } = getVendorLoadContext( this.mapRef.current );

Map block: fix script loading to work with FSE #19552

Map block: fix script loading to work with FSE #19552

Conversation

glendaviesnz commented Apr 16, 2021 • edited Loading

Description

Does this pull request change what data or activity we track or use?

Testing instructions:

matticbot commented Apr 16, 2021

github-actions bot commented Apr 16, 2021 • edited Loading

glendaviesnz Apr 16, 2021

Choose a reason for hiding this comment

simison Apr 16, 2021

Choose a reason for hiding this comment

glendaviesnz Apr 19, 2021

Choose a reason for hiding this comment

simison commented Apr 16, 2021

simison Apr 16, 2021

Choose a reason for hiding this comment

glendaviesnz Apr 19, 2021

Choose a reason for hiding this comment

glendaviesnz commented Apr 19, 2021

ramonjd commented Apr 22, 2021

glendaviesnz commented Apr 22, 2021

ramonjd commented Apr 23, 2021

ramonjd left a comment

Choose a reason for hiding this comment

jeherve left a comment

Choose a reason for hiding this comment

glendaviesnz commented May 12, 2021

andrewserong left a comment

Choose a reason for hiding this comment

jeherve left a comment

Choose a reason for hiding this comment

glendaviesnz commented May 13, 2021

glendaviesnz commented May 17, 2021

jeherve commented May 18, 2021

samiff commented May 19, 2021

glendaviesnz commented May 20, 2021

glendaviesnz commented May 20, 2021

github-actions bot commented May 20, 2021

glendaviesnz commented May 20, 2021

glendaviesnz commented Apr 16, 2021 •

edited

Loading

github-actions bot commented Apr 16, 2021 •

edited

Loading