Contact Info & Map widget: Add legacy widget → block transform #21232

ivan-ottinger · 2021-09-29T12:38:58Z

Changes proposed in this Pull Request:

This PR adds transform for the legacy / classic "Contact Info & Map" widget:

Screen.Capture.on.2021-10-15.at.11-45-05.mp4

The proposed change can help users move the content from their existing widget into the "Contact Info" block, plus several additional blocks:

Heading (H2) block storing the Title of the widget
Paragraph block storing the Hours field of the widget
Map block that extracts the address from the widget Address field, utilizes the changes proposed in Map block: Add address block attribute #21412 and displays the map with the pin pointing to the location

(Please note that the Show map checkbox in the widget settings needs to be selected in order for the Map block to be created)

The Address field of the widget is transformed into the Street Address block field, leaving other block fields (e.g. Country, Postal/Zip Code, State) empty.

Jetpack product discussion

Widget Blocks: test/remove widgets in Legacy Widget block #21069
p7fD6U-4KX-p2

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

No.

Testing instructions:

Add the "Contact Info & Map" widget (using the Legacy Block) to your testing site.
Select the transform option and choose "Contact Info" block (as can be seen in the screenshot above).
The widget information should be correctly transformed.
If the Show map checkbox is selected and the address is provided, Map block pointing to the correct location should be added during the transform.

matticbot · 2021-09-29T12:39:04Z

Caution: This PR has changes that must be merged to WordPress.com
Hello ivan-ottinger! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D67571-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-09-29T12:40:11Z

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 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

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: November 2, 2021.
Scheduled code freeze: October 25, 2021.

ivan-ottinger · 2021-09-30T10:53:13Z

~~There's an issue which is blocking the build process of the WPCOM patch: p1632857344442400-slack-CBG1CP4EN - so we cannot test the patch on WPCOM yet.~~

→ This has been fixed already. We can now test the patch on WPCOM as well.

arcangelini

Everything worked for me, just need to add the hook to hide the widget and possibly the conditionals for the other fields?

arcangelini · 2021-09-30T14:28:38Z

projects/plugins/jetpack/modules/widgets/contact-info.php

@@ -31,6 +31,7 @@ public function __construct() {
 				'classname'                   => 'widget_contact_info',
 				'description'                 => __( 'Display a map with your location, hours, and contact information.', 'jetpack' ),
 				'customize_selective_refresh' => true,
+				'show_instance_in_rest'       => true,
 			);


Not sure if you left it out on purpose, but the hook to remove it from the Legacy block needs to be added as well. Made testing easier though : )

Thanks! :)

Yes, I left it out on purpose - as I wanted to hear your opinions on also leaving the Contact Info & Map widget there for the users to choose (in case they prefer it over Contact Info block).

My understanding was that we were getting rid of the redundancies in the widget area. I think it is a little confusing to users to have two or three options for the same things. This seemed like a 1:1 transform, does it differ greatly between the widget and block option?

I am not completely sure though, it could be best to get @simison and @yansern opinion.

I think it is a little confusing to users to have two or three options for the same things.

I see what you mean by looking at this from the perspective of user's confusion.

This seemed like a 1:1 transform, does it differ greatly between the widget and block option?

The main difference I can see is the map solution being used: Google Maps (Contact Info & Map widget) vs MapBox (when the user transforms the widget into blocks).

I personally prefer MapBox, but I can see some users that would like to use Google Maps instead.

If we want to remove the redundancy, then yes, I think it would be best to remove the legacy widget.

I see that the only way to use Google Maps is through shortcodes.
[googlemaps https://maps/url/here]

Are we able to construct a Google Maps embed URL with long & lat param?

Is it alright if it's a shortcode block?

Reference: https://wordpress.com/support/google-maps/#embedding-a-google-map

How about creating a block for this?

Also, another aspect we can consider, is to create a Contact Info & Maps block as part of our Gutenberg training. Since the widget code lives in Jetpack, why not create the same block in Jetpack?

Then, we can come back to this transformation exercise and have it do a 1:1 transformation.

In other words, as a first iteration we could have a block that has the same functionality as the current legacy widget. Then we could build on top of that and add extra features, e.g. Mapbox support.

I agree with starting simple. Showing the same form as what the legacy widget did, and use Google Maps, would be the path to least resistance. I imagine literally extract bits & pieces and put them back using Block's library.

It seems that there's a Contact Info block on Jetpack. It also creates a Google Maps.
Perhaps the work needed is to modify this block and give user the option to embed the Google Maps under.
https://github.com/Automattic/jetpack/tree/master/projects/plugins/jetpack/extensions/blocks/contact-info

How does that sound? 🙂

Yeah, we could give that a try. I would, however, suggest that you post on a P2 about this as there are some product discussions to be had about this. See p7fD6U-2XB-p2 for some background.

It also creates a Google Maps

It's just a link though, so nothing super fancy. But I agree we should start there with that block, maybe by leveraging the existing map block as a new possible inner block for that block.

I have just published a P2 post to gather more insights - so we can then decide on the best approach moving forward: p7fD6U-4KX-p2.

Discussion continues in p7fD6U-4KX-p2

arcangelini · 2021-09-30T14:30:21Z

projects/plugins/jetpack/extensions/blocks/contact-info/index.js

+						} ),
+					];
+
+					if ( instance.raw.hours ) {


You might need to make all of the innerBlocks conditional. Otherwise, you end up with blanks blocks if it is empty. This is what it transformed to for me with certain fields blank - https://d.pr/i/8UNAOy

I was initially thinking about making all fields conditional, but then decided to leave some of them unconditional. The reasoning here is that if the user is transforming to the Contact Info block, they will see what fields they can fill in (even if they didn't have them filled in the widget previously).

Also, for example, if the widget title is empty, they will see a nice empty header they can fill in. At least this is how I would like it to be (if I were a user). But I get your point!

I totally see your point. Generally speaking, I imagine the transform being used for someone who already has a fully built widget and we are just preserving/transferring the settings to the new format. In that case I imagine it could be unnecessary.

It could go either way I guess haha.

arcangelini · 2021-09-30T14:32:55Z

projects/plugins/jetpack/extensions/blocks/contact-info/index.js

+						];
+					}
+
+					if ( instance.raw.showmap ) {


You could also refractor this to all be in the array? Something like:

...( instance.raw.phone ? [ createBlock( 'jetpack/phone', { phone: instance.raw.hours } ) ] : [] ), ...( instance.raw.hours ? [ createBlock( 'core/paragraph', { content: instance.raw.hours } ) ] : [] ), ...( instance.raw.showmap ? [ createBlock( 'jetpack/map' ) ] : [] ),

simison · 2021-10-07T09:26:10Z

Quick feedback, lemme know if I missed anything important:

If we wanted to automatically center the map for the user, we would have to first get the coordinates of the location.

Is it possible to use the same geocoder that Map block uses?

is to create a Contact Info & Maps block

Let's not add new blocks; we already have maps block and contact info block separately. It's confusing to have multiple very similar blocks.

Is it alright if it's a shortcode block?

Please don't output shortcodes; they don't work in new site-editor and are otherwise also just technical debt we don't want.

Overall: these transforms don't need to be pixel perfect 1:1, as long as the "sentiment" of the block gets transformed into blocks. It's fully expected that people need to do manual work after transform, or even return back to widget with "undo" if they aren't happy with the result.

ivan-ottinger · 2021-10-07T10:53:23Z

Thank you for your feedback Mikael.

Is it possible to use the same geocoder that Map block uses?

I think it should be possible and will take a closer look.

Overall: these transforms don't need to be pixel perfect 1:1, as long as the "sentiment" of the block gets transformed into blocks. It's fully expected that people need to do manual work after transform, or even return back to widget with "undo" if they aren't happy with the result.

Makes sense. The first iteration of the block transform could be what was initially suggested in this PR:

In other words, the text fields (Heading, Email, Address and Hours) would transfer and the default map gets added (through the Map block / Mapbox). User then needs to add the map pin manually.

The next iteration (once we can get the location coordinates), could add the map pin automatically with something like:

if ( instance.raw.showmap ) {
	innerBlocks = [ ...innerBlocks, createBlock( 'jetpack/map', {
		points: [ {
			title: instance.raw.title,
			placeTitle: instance.raw.title,
			caption: instance.raw.address,
			coordinates: {
				latitude: 48.14472,
				longitude: 17.11278
			}
		} ],
	} ) ];
}

(the example above has hardcoded coordinates - just to illustrate)

This could the look like this (also filling the pin details):

Then, once we have the outcome of the following discussion: p7fD6U-4KX-p2, we can make further changes as necessary.

What do you all think? Does that sound reasonable?

yansern · 2021-10-11T12:49:07Z

What do you all think? Does that sound reasonable?

Yup, as agreed in our convos, let's try and see if we can fetch the coordinates from the address, then insert it as a Mapbox block. :)

yansern · 2021-10-15T10:54:01Z

What do you all think? Does that sound reasonable?

Yup, as agreed in our convos, let's try and see if we can fetch the coordinates from the address, then insert it as a Mapbox block. :)

As an update to this one, we're now updating the Mapbox block to resolve coordinates from address instead. :) It's been a rollercoaster. :)

alshakero

Tested and works as expected!

matticbot · 2021-10-21T12:19:41Z

Caution: This PR has changes that must be merged to WordPress.com
Hello ivan-ottinger! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D68822-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

This commit adds transformation of the "Contact Info & Maps" widget into "Contact Info" block, plus extra necessary blocks.

This commit removes unnecessary variables and builds `innerBlocks` array directly.

If the `Show map` checkbox is selected, Map block is added during the transform.

Make sure that the extra Paragraph block is added only when the `Hours` field has some content.

This commit reverts the changes made in the previous commit as it makes more sense to introduce Map block changes in a separate branch/PR.

This commit adds the `address` attribute back after I had incorrectly removed it.

* Add `address` attribute to the Map block Map block component currently doesn't provide a way to create a map from provided address. This commit adds `address` block attribute that can then be geocoded and used to render the map (as opposed to only using coordinates (lat, long)). * Add `getCoordinates` function The `getCoordinates` function will take care of the Mapbox API request. The function accepts physical address and Mapbox API key as parameters. The output is the promise / Mapbox API response that contains geo coordinates. * Fix typo in the changelog file * Add `geoCodeAddress` function `geoCodeAddress` will take care of creating the new map point based on the geo coordinates. This commit also makes sure that the above function is run when the Map component updates (`componentDidUpdate`). The function runs after the Mapbox API key is ready. * Encode `apiKey` in the Mapbox API fetch request * Remove unnecessary `console.error()` call * Replace `console.error()` with `this.onError()` This change will make sure there's specific error message displayed in the front-end - as opposed to only displaying error in the console. * Check whether Mapbox API returned any location / coordintes This commit handles situations where the address provided by the user cannot be properly localized. If the Mapbox API returns empty array of locations (`result.features`), we will display an error message. * Make sure `result.features` won't throw if `features` object doesn't exist * Refactor `newPoint` creation The commit introduces a new const `feature` that is then used as a building block for `newPoint` object values. * Update `jetpack__map.json` structure - add `address` attribute * Contact Info & Map widget: Add legacy widget → block transform (#21232) * Remove Contact Info & Map widget from legacy-widget block * Add changelog file * Add transform of the legacy widget to relevant block This commit adds transformation of the "Contact Info & Maps" widget into "Contact Info" block, plus extra necessary blocks. * Update changelog file * Code refactor - construcing `innerBlocks` directly This commit removes unnecessary variables and builds `innerBlocks` array directly. * Add Map block to the transform If the `Show map` checkbox is selected, Map block is added during the transform. * Add conditional to the `Hours` field Make sure that the extra Paragraph block is added only when the `Hours` field has some content. * Remove `address` attribute from the Map block This commit reverts the changes made in the previous commit as it makes more sense to introduce Map block changes in a separate branch/PR. * Use the new Map block `address` field * Add `address` attribute to the Map block This commit adds the `address` attribute back after I had incorrectly removed it. Co-authored-by: Samiff <samiff@users.noreply.github.com>

ivan-ottinger · 2021-10-26T13:25:18Z

Just noting here that the changes have been successfully deployed to WPCOM. Related comment on the other PR.

matticbot added the Touches WP.com Files label Sep 29, 2021

github-actions bot added [Block] Contact Info [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Feature] Extra Sidebar Widgets labels Sep 29, 2021

github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Sep 29, 2021

ivan-ottinger added the [Status] In Progress label Sep 29, 2021

ivan-ottinger marked this pull request as ready for review September 29, 2021 14:07

ivan-ottinger requested review from yansern and arcangelini September 30, 2021 10:53

ivan-ottinger self-assigned this Sep 30, 2021

ivan-ottinger added the [Status] Needs Team Review Obsolete. Use Needs Review instead. label Sep 30, 2021

arcangelini requested changes Sep 30, 2021

View reviewed changes

github-actions bot added the [Block] Map label Oct 14, 2021

ivan-ottinger removed the [Block] Map label Oct 14, 2021

ivan-ottinger force-pushed the update/contact-info-and-map-widget-block-compatibility branch from 2c0fd29 to 8e74a8e Compare October 14, 2021 13:58

github-actions bot added the [Block] Map label Oct 14, 2021

ivan-ottinger force-pushed the update/contact-info-and-map-widget-block-compatibility branch from c80a87f to 7294532 Compare October 14, 2021 15:41

ivan-ottinger changed the base branch from master to add/map-block-address-attribute October 15, 2021 07:14

ivan-ottinger mentioned this pull request Oct 15, 2021

Map block: Add address block attribute #21412

Merged

alshakero approved these changes Oct 19, 2021

View reviewed changes

ivan-ottinger force-pushed the update/contact-info-and-map-widget-block-compatibility branch 2 times, most recently from 500a8ff to e2f820a Compare October 21, 2021 12:19

ivan-ottinger force-pushed the add/map-block-address-attribute branch from 3ee6839 to 65fb66e Compare October 22, 2021 11:18

ivan-ottinger added 10 commits October 22, 2021 13:21

Remove Contact Info & Map widget from legacy-widget block

d3ff8ab

Add changelog file

2cb90af

Add transform of the legacy widget to relevant block

645531d

This commit adds transformation of the "Contact Info & Maps" widget into "Contact Info" block, plus extra necessary blocks.

Update changelog file

8b5bf4a

Code refactor - construcing innerBlocks directly

9e6f693

This commit removes unnecessary variables and builds `innerBlocks` array directly.

Add Map block to the transform

137c5cb

If the `Show map` checkbox is selected, Map block is added during the transform.

Add conditional to the Hours field

0eec4d7

Make sure that the extra Paragraph block is added only when the `Hours` field has some content.

Remove address attribute from the Map block

b8d924d

This commit reverts the changes made in the previous commit as it makes more sense to introduce Map block changes in a separate branch/PR.

Use the new Map block address field

973c78b

Add address attribute to the Map block

9dcba56

This commit adds the `address` attribute back after I had incorrectly removed it.

ivan-ottinger force-pushed the update/contact-info-and-map-widget-block-compatibility branch from e2f820a to 9dcba56 Compare October 22, 2021 11:21

samiff self-requested a review October 22, 2021 22:54

samiff approved these changes Oct 22, 2021

View reviewed changes

samiff merged commit 2c6b442 into add/map-block-address-attribute Oct 25, 2021

samiff deleted the update/contact-info-and-map-widget-block-compatibility branch October 25, 2021 20:27

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

ivan-ottinger mentioned this pull request Oct 27, 2021

Contact Info and Map widget: widget deprecation #21554

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contact Info & Map widget: Add legacy widget → block transform #21232

Contact Info & Map widget: Add legacy widget → block transform #21232

ivan-ottinger commented Sep 29, 2021 •

edited

Loading

matticbot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021 •

edited

Loading

ivan-ottinger commented Sep 30, 2021 •

edited

Loading

arcangelini left a comment

arcangelini Sep 30, 2021

ivan-ottinger Sep 30, 2021 •

edited

Loading

arcangelini Sep 30, 2021

ivan-ottinger Sep 30, 2021

yansern Oct 1, 2021

yansern Oct 4, 2021

yansern Oct 4, 2021 •

edited

Loading

jeherve Oct 4, 2021

ivan-ottinger Oct 5, 2021 •

edited

Loading

jeherve Oct 5, 2021

arcangelini Sep 30, 2021

ivan-ottinger Sep 30, 2021

arcangelini Sep 30, 2021

arcangelini Sep 30, 2021

simison commented Oct 7, 2021 •

edited

Loading

ivan-ottinger commented Oct 7, 2021

yansern commented Oct 11, 2021

yansern commented Oct 15, 2021

alshakero left a comment

matticbot commented Oct 21, 2021

ivan-ottinger commented Oct 26, 2021

Contact Info & Map widget: Add legacy widget → block transform #21232

Contact Info & Map widget: Add legacy widget → block transform #21232

Conversation

ivan-ottinger commented Sep 29, 2021 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

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

Testing instructions:

matticbot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021 • edited Loading

ivan-ottinger commented Sep 30, 2021 • edited Loading

arcangelini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivan-ottinger Sep 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

How about creating a block for this?

Choose a reason for hiding this comment

yansern Oct 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivan-ottinger Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simison commented Oct 7, 2021 • edited Loading

ivan-ottinger commented Oct 7, 2021

yansern commented Oct 11, 2021

yansern commented Oct 15, 2021

alshakero left a comment

Choose a reason for hiding this comment

matticbot commented Oct 21, 2021

ivan-ottinger commented Oct 26, 2021

ivan-ottinger commented Sep 29, 2021 •

edited

Loading

github-actions bot commented Sep 29, 2021 •

edited

Loading

ivan-ottinger commented Sep 30, 2021 •

edited

Loading

ivan-ottinger Sep 30, 2021 •

edited

Loading

yansern Oct 4, 2021 •

edited

Loading

ivan-ottinger Oct 5, 2021 •

edited

Loading

simison commented Oct 7, 2021 •

edited

Loading