Map Block: Use the block styles API for choosing map themes #14852

pento · 2020-03-02T07:44:50Z

This PR switches the map block over to using the block styles attribute, rather than the custom theme picker.

Fixes #14559

Changes proposed in this Pull Request:

See discussion on Map Block: UI for map themes #14559 and linked issues.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Change to existing feature.

Testing instructions:

Add a new map block, test that the block style switches the map theme as expected.
Open an post with an old map block, check that in correctly converts to the new style.

Proposed changelog entry for your changes:

Map Block: Improve the map theme picker.

jetpackbot · 2020-03-02T07:46:42Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 07190e9

pento · 2020-03-02T07:48:31Z

Some early notes:

The Styles panel defaults to being closed, there's no way to override that. This is a change in behaviour from the existing theme picker.
I couldn't figure out the purpose of the "Default Style" dropdown which appears in the Style panel. Also not sure how to disable it.
I haven't tested frontend rendering, either of old or new map blocks. They're almost certainly broken at the moment. 🙂

scruffian · 2020-03-02T15:06:38Z

The Styles panel defaults to being closed, there's no way to override that. This is a change in behaviour from the existing theme picker.

Sounds like we should open a core patch for that

scruffian · 2020-03-02T15:09:33Z

I haven't tested frontend rendering, either of old or new map blocks. They're almost certainly broken at the moment. 🙂

It did work before this change, but now it doesn't :)

scruffian

This is a really good direction. We have a custom component which does something similar, but if we can do it this way that would be ideal!

We just need to get it working on the frontend...

scruffian · 2020-03-02T15:15:08Z

There's an annoying margin at the top of the previews, which I have a fix for here: WordPress/gutenberg#19983

pablinos

I agree with @scruffian, this is a better pattern than we have at the moment. Let's get it working along with any core patches needed!

extensions/blocks/map/edit.js

extensions/blocks/map/index.js

apeatling · 2020-03-03T18:58:48Z

Ran through and tested this and it worked as expected and mentioned in the instructions. I like the experience of using block styles for this. Agreed that we should submit a core PR to allow the style panel to be open by default, it'll be incredibly easy to miss this otherwise.

matticbot · 2020-03-04T07:40:25Z

Caution: This PR has changes that must be merged to WordPress.com
Hello pento! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D39810-code before merging this PR. Thank you!

matticbot · 2020-03-04T07:56:53Z

pento, Your synced wpcom patch D39810-code has been updated.

matticbot · 2020-03-04T07:58:57Z

pento, Your synced wpcom patch D39810-code has been updated.

pento · 2020-03-04T08:27:09Z

This PR has generated two Gutenberg PRs:

As several folks suggested, something to keep the Styles panel open by default: Styles Panel: Don't force it to be closed by default. WordPress/gutenberg#20617
A block setting to remove the Default Style selector, since it's not really relevant to this block, and also kind of confusing: Blocks: Allow the Default Style selector to be hidden. WordPress/gutenberg#20620

I've also switched back to storing the map style in the data-map-style attribute (e2927a6), which has the added bonus of no longer needing a deprecated block definition, and we're not changing the save() output, so we don't have to hold merging this to WPCOM until the end of the month. 🙂

extensions/blocks/map/edit.js

matticbot · 2020-03-04T11:45:41Z

pento, Your synced wpcom patch D39810-code has been updated.

scruffian · 2020-03-04T11:46:01Z

I added a commit to bring this into line with how we do things with Tiled Gallery. It also makes the code a bit simpler.

stale

Copons

LGTM!

… block updates to the new one

… when upgrading.

…Name attribute not being set in the old block.

matticbot · 2020-03-26T02:09:12Z

pento, Your synced wpcom patch D39810-code has been updated.

matticbot · 2020-03-26T02:19:56Z

pento, Your synced wpcom patch D39810-code has been updated.

Copons

Hopefully for the last time 😄

matticbot · 2020-03-26T16:07:46Z

pento, Your synced wpcom patch D39810-code has been updated.

kraftbj

I tested it on a fresh install and on an install bringing in an older version of the block. Both worked as expected as described in the ticket.

Fusion is failing because of new files. I'm working on that now to update the phab.

Issues addressed.

pento · 2020-03-26T22:30:04Z

@Automattic/jetpack-crew: Could someone merge this, please? It looks like the tests are now passing on the diff (thanks, @kraftbj!), but it hasn't synced the status, so merge is blocked for me.

kraftbj · 2020-03-26T22:32:34Z

@pento Unblocked! Please merge this (and the corresponding WP.com diff) at your leisure.

* Initial changelog entry * Changelog: add #14904 * Changelog: add #14910 * Changelog: add #14913 * Changelog: add #14916 * Changelog: add #14922 * Changelog: add #14924 * Changelog: add #14925 * Changelog: add #14928 * Changelog: add #14840 * Changelog: add #14841 * Changelog: add #14842 * Changelog: add #14826 * Changelog: add #14835 * Changelog: add #14859 * Changelog: add #14884 * Changelog: add #14888 * Changelog: add #14817 * Changelog: add #14814 * Changelog: add #14819 * Changelog;: add #14797 * Changelog: add #14798 * Changelog: add #14802 * Changelog: add #13676 * Changelog: add #13744 * Changelog: add #13777 * Changelog: add #14446 * Changelog: add #14739 * Changelog: add #14770 * Changelog: add #14784 * Changelog: add #14897 * Changelog: add #14898 * Changelog: add #14968 * Changelog: add #14985 * Changelog: add #15044 * Changelog: add #15052 * Update to remove Podcast since it remains in Beta * Changelog: add #14803 * Changelog: add #15028 * Changelog: add #15065 * Changelog:add #14886 * Changelog: add #15118 * Changelog: add #14990 * Changelog: add #14528 * Changelog: add #15120 * Changelog: add #15126 * Changelog: add #15049 * Chanegelog: add #14852 * Changelog: add #15090 * Changelog: add #15138 * Changelog: add #15124 * Changelog:add #15055 * Changelog: add #15017 * Changelog: add #15109 * Changelog: add #15145 * Changelog:add #15096 * Changelog:add #15153 * Changelog: add #15133 * Changelog: add #14960 * Changelog: add #15127 * Changelog: add #15056 * Copy current changelog to changelog archive. * Clarify changelog description

pento added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Status] Needs Testing We need to add this change to the testing call for this month's release labels Mar 2, 2020

pento requested review from a team March 2, 2020 07:44

pento self-assigned this Mar 2, 2020

matticbot added Touches WP.com Files labels Mar 2, 2020

scruffian previously requested changes Mar 2, 2020

View reviewed changes

pablinos reviewed Mar 2, 2020

View reviewed changes

extensions/blocks/map/edit.js Outdated Show resolved Hide resolved

extensions/blocks/map/edit.js Outdated Show resolved Hide resolved

extensions/blocks/map/index.js Outdated Show resolved Hide resolved

This was referenced Mar 4, 2020

Styles Panel: Don't force it to be closed by default. WordPress/gutenberg#20617

Merged

Blocks: Allow the Default Style selector to be hidden. WordPress/gutenberg#20620

Merged

pento requested a review from scruffian March 4, 2020 08:23

pento added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Mar 4, 2020

scruffian reviewed Mar 4, 2020

View reviewed changes

extensions/blocks/map/edit.js Outdated Show resolved Hide resolved

Copons previously approved these changes Mar 4, 2020

View reviewed changes

scruffian and others added 6 commits March 26, 2020 12:51

Add a version of the old save function so that the old version of the…

e620c31

… block updates to the new one

move the deprecation to a directory

7215883

Update the setting name for disabling the default style picker.

519d101

Migrate old map styles to the class attribute.

4ee820c

Handle a different is-style-* classname being set on an old map block…

f3be18f

… when upgrading.

Add a comment describing the upgrade process, and allow for the class…

285323a

…Name attribute not being set in the old block.

pento dismissed Copons’s stale review via 285323a March 26, 2020 02:09

pento force-pushed the fix/14559-block-style branch from 31537fa to 285323a Compare March 26, 2020 02:09

Clean up whitespace in the className when upgrading.

07190e9

pento requested a review from jeherve March 26, 2020 03:11

pento added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. 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 Mar 26, 2020

Copons approved these changes Mar 26, 2020

View reviewed changes

kraftbj approved these changes Mar 26, 2020

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 Mar 26, 2020

pento merged commit d7fb67e into master Mar 26, 2020

pento deleted the fix/14559-block-style branch March 26, 2020 22:36

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 26, 2020

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Mar 31, 2020

jeherve added a commit that referenced this pull request Mar 31, 2020

Chanegelog: add #14852

4ecb091

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Map Block: Use the block styles API for choosing map themes #14852

Map Block: Use the block styles API for choosing map themes #14852

pento commented Mar 2, 2020

jetpackbot commented Mar 2, 2020 •

edited

Loading

pento commented Mar 2, 2020

scruffian commented Mar 2, 2020

scruffian commented Mar 2, 2020 •

edited

Loading

scruffian left a comment

scruffian commented Mar 2, 2020

pablinos left a comment

apeatling commented Mar 3, 2020

matticbot commented Mar 4, 2020

matticbot commented Mar 4, 2020

matticbot commented Mar 4, 2020

pento commented Mar 4, 2020

matticbot commented Mar 4, 2020

scruffian commented Mar 4, 2020

Copons left a comment

matticbot commented Mar 26, 2020

matticbot commented Mar 26, 2020

Copons left a comment

matticbot commented Mar 26, 2020

kraftbj left a comment •

edited

Loading

pento commented Mar 26, 2020

kraftbj commented Mar 26, 2020

Map Block: Use the block styles API for choosing map themes #14852

Map Block: Use the block styles API for choosing map themes #14852

Conversation

pento commented Mar 2, 2020

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

Proposed changelog entry for your changes:

jetpackbot commented Mar 2, 2020 • edited Loading

pento commented Mar 2, 2020

scruffian commented Mar 2, 2020

scruffian commented Mar 2, 2020 • edited Loading

scruffian left a comment

Choose a reason for hiding this comment

scruffian commented Mar 2, 2020

pablinos left a comment

Choose a reason for hiding this comment

apeatling commented Mar 3, 2020

matticbot commented Mar 4, 2020

matticbot commented Mar 4, 2020

matticbot commented Mar 4, 2020

pento commented Mar 4, 2020

matticbot commented Mar 4, 2020

scruffian commented Mar 4, 2020

Copons left a comment

Choose a reason for hiding this comment

matticbot commented Mar 26, 2020

matticbot commented Mar 26, 2020

Copons left a comment

Choose a reason for hiding this comment

matticbot commented Mar 26, 2020

kraftbj left a comment • edited Loading

Choose a reason for hiding this comment

pento commented Mar 26, 2020

kraftbj commented Mar 26, 2020

jetpackbot commented Mar 2, 2020 •

edited

Loading

scruffian commented Mar 2, 2020 •

edited

Loading

kraftbj left a comment •

edited

Loading