Need to redo this to match r138023-wpcom . I'll try for today if I have time after some meetings.

The wp.com filter name and this one match, so we can ship this now in 4.1, then kick over to the embed API in a future release. Thoughts?

The kicker is in the support docs/interactions we need to ensure folks create a key and enable both the Google Maps JS API and the Google Maps Embed API to have the map work now, with this PR, and if we switch over the the embed business.

@aduth - What do you think? Should we just rip the band-aid off and switch over to the embed API in Jetpack now?

The kicker is in the support docs/interactions we need to ensure folks create a key and enable both the Google Maps JS API and the Google Maps Embed API to have the map work now, with this PR, and if we switch over the the embed business.

Right, that's potentially a source of future confusion, as the user would need to explicitly enable both APIs, now for the JS API, and in the future for the Embeds API (or both now, but if a user enables the first and sees their site working, they might skip the second).

I'll leave it to you to make the final decision, but personally the changes in r137758-wpcom and r138023-wpcom were pretty minor, so I don't think it'd be a difficult port.

@kraftbj I think we could port the changes @aduth mentioned above in 4.1. Will you have the chance to update your PR today?

Ported those changes over. @aduth can you give it a quick once-over to make sure I didn't make any mistakes in transcription?

@kraftbj : Will do a more thorough look this afternoon, but at a glance, looks like dotcom might have diverged some from Jetpack, specifically the logic to enqueue scripts as a separate function (add_action in the customize preview case). Should we keep this function as-is here but strip it down to just the stylesheet (effectively what the dotcom changes are doing anyways).

Aside from my comment above, the port looks good to me.

Works for me. Restored the previous function and corresponding related edits. Once it passes tests, good to merge.

Shouldn't we handle the lack of API key a bit better? When applying this PR and not defining any API key, I get the following error in my logs:

PHP Warning:  constant(): Couldn't find constant GOOGLE_MAPS_API_BROWSER_KEY in /wp-content/plugins/jetpack/modules/widgets/contact-info.php on line 285

I also get the following error instead of the map:

The Google Maps API server rejected your request. This service requires an API key.

Should we not display any map when no constant has been defined?

I didn't get the notice, huh. constant is supposed to return null if the constant doesn't exist. I'll rework it today to use "defined" instead.

I'll look at alternatives. Google is grandfathering older accounts, so in theory, it can work without an API too, but might be worth it to force someone to add it. I don't like doing this much new this close to shipping though. Should we push this to 4.1.1 to nail quality and not rush?

@kraftbj : Should we just remove the constant? I can define the add_filter elsewhere on WordPress.com .

I see this as at least being an intermediate fix for sites which are currently broken and might be willing to go through the process of obtaining an API key and defining a filter, but it's certainly not a great user experience currently. Not sure if best route is to explore alternative embedded maps solutions, have good documentation to explain to users how to obtain the key, and possibly include a UI in wp-admin for configuring their key.

@aduth While putting together #4265, until we're ready to pull it out (which may be now on wp.com), the widget is still trying to geocode using a Google API, which requires a key now. The spec says it is included with the JS API as well.

With the embed API, I think we can rip it all out. The downside is, previously, we verified an address would map. The embed API wouldn't provide this option without using the lower limits of the Geocode api. (2500 per day, meaning 2,500 edits of a contact widget on wp.com).

@kraftbj : Right, I should have thought of the geocoding in that file. It's only really used to validate that the address is "good", so while nice, we could still remove it and allow the map to be rendered via embed, placing responsibility on the user to confirm that the map renders okay with their entered address.

I'd assume that this hasn't been an issue on WordPress.com because of the grandfathering (i.e. geocoding requests are sent from WordPress.com servers, which won't be true for Jetpack).

@kraftbj Could you rebase? Thanks!

@jeherve @kraftbj - rebased. Everything seems to work fine. Will merge when tests pass.

Merged to 4.2

@kraftbj @aduth I believe we could still add in a few improvements to avoid frustration and confusion for both existing and new users of the widget:

• If I used the Contact Info Widget before June 22, the map was still displayed on my site. Starting in Jetpack 4.2, the map won't be displayed anymore.
• If I didn't use the Contact Info Widget, I still see the "Show Map" option in the widget settings. However, that option doesn't do anything unless I add an API key.

Could we do the following?

• If the "Show Map" option is checked in the widget settings, display a new input field where the API key can be added.
• For existing installs, we still remove the map, but when the site owner visits the widget settings, they see that the "Show Map" option is unchecked, and get the API key field when they check that box.

What do you think?

@jeherve I agree with both of these concerns, and the idea of adding an API key input field to the widget settings sounds like a reasonable approach. I'm not sure we have a way to detect sites which have been "grandfathered" (previously using maps without a key). One idea to consider is to issue a request to an endpoint and assume the lack of a failure indicates grandfathering (and vice-versa if an error is returned).

Just my two cents; I don't think there is a need for the Show Map checkbox, but rather a well labeled field for the API key that says something like "API Key (maps will only show if API Key is entered. Click here for more info)" and have the Click here part link to a page that explains how to get an API key if possible.

Anyone who is "grandfathered" should have their map disappear without warning! (harsh, I know) I think this is better than displaying a blank map box though. Having a note in the changelog and the label on the widget should hopefully be enough to guide anyone who wants their map back through getting it set up right.

Also I'm not sure if there is any way to validate if the API key has been entered correctly, something along the lines of making sure the API key actually works and someone didn't just put their phone number in the box before having the maps actually display. Extra points for some sort of visual validation (like a check mark image next to the api key input field) for a valid API key being entered if it's confirmed to be valid. That might be going above and beyond for a simple map feature, and might not even be possible (I'm not too familiar with Google's APIs) but I think that would be the easiest user experience.

@kraftbj : Should we remove the apply_filters part of this condition that you added?

https://github.com/Automattic/jetpack/blob/90239a7/modules/widgets/contact-info.php#L99

For sites which had previously been using maps and are grandfathered, I imagine this might allow the map to continue to be shown. Unless, that is, Google's grandfathering applies distinctly between their API products (since we switched from JS API to Embed API).

@nemchik : I don't know that it'd be very obvious that entering an API key value would control whether the map is shown. The approach I took in #4562 is to leave it the same in that a "Show Map" checkbox is displayed, but if checked, an additional API key field is included immediately below (screenshot).

I also removed the condition introduced here which only displayed a map if a valid API key is provided. Unless I'm incorrect that grandfathering applies in the transition from JS to Embed API, the effect of this should be that there is no breakage or need to make changes for existing sites using the widget. At the same time, users creating new Contact Info Widgets will be made aware through the act of selecting "Show Map" that an API key will be required for the map to display correctly.

@aduth Removing the apply_filters seems reasonable. I'm not sure about the swap in product either.