Eventbrite Block: Add alignment support by apeatling · Pull Request #14817 · Automattic/jetpack

apeatling · 2020-02-26T18:21:43Z

Changes proposed in this Pull Request:

Add alignment support to the Eventbrite block. This works for both the modal button and in-page embed styles.

Fixes #14505

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

This fixes an existing feature.

Testing instructions:

Add an Eventbrite block to a post or page, and add an event (example: https://www.eventbrite.ca/e/5th-construction-expo-2020-tickets-31941745621)
Confirm you can see the "left, center, right" alignment option in the block settings bar when using the in-page embed style.
Confirm you can also see alignment options when the Button & Modal style is selected
Choose left, right, and center alignment options, and confirm these work correctly in the editor.

Proposed changelog entry for your changes:

None.

…e Eventbrite block. Fixes #14505

matticbot · 2020-02-26T18:21:49Z

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

jetpackbot · 2020-02-26T18:24:15Z

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 f7dfb6e

creativecoder · 2020-02-26T23:44:53Z

@apeatling I'd recommend adding the Explorers Github team as a reviewer for broader exposure

creativecoder · 2020-02-26T23:47:18Z

Just noting that the align options in the Calendly block have some issues with a wrapper div being removed from Gutenberg since the most recent WordPress release. I'm not sure if that effects Eventbrite--please be sure to test this one with and without the Gutenberg plugin activated.

…t the iframe cannot be interacted with when alignment is set. See #14820

matticbot · 2020-02-27T17:21:13Z

apeatling, Your synced wpcom patch D39459-code has been updated.

apeatling · 2020-02-27T17:21:30Z

@creativecoder Thanks for highlighting that. It was also an issue here and I've added the same fix to this block. Working correctly now.

pablinos

This works well for me!

scruffian · 2020-03-02T13:43:48Z

This doesn't work for me when in the "in-page embed" mode. I'm happy to fix this if you don't mind me making changes to your PR? Otherwise I'm happy to let you do it :)

scruffian

The In-page embed mode isn't working in the front end.

apeatling · 2020-03-02T16:10:25Z

@scruffian Feel free to jump in on these PRs, no issue!

…e server side rendered iframe embed.

matticbot · 2020-03-02T22:14:06Z

apeatling, Your synced wpcom patch D39459-code has been updated.

apeatling · 2020-03-02T22:14:29Z

@scruffian Pushed a fix for the front end alignment. Feel free to adjust if there's a better way.

matticbot · 2020-03-03T14:45:00Z

apeatling, Your synced wpcom patch D39459-code has been updated.

scruffian · 2020-03-03T14:50:12Z

I made a few changes:

We should be supporting the additional classes users can add in the sidebar, which means its best to use the Jetpack_Gutenberg::block_classes helper function. This also simplifies things a lot
This means we should also wrap the classes in esc_attr in case someone adds something silly.
I also updated the class name to reflect what that helper function returns.

apeatling · 2020-03-03T15:35:44Z

@scruffian Thanks! TIL about Jetpack_Gutenberg::block_classes.

PR fixed

scruffian

This looks good now, but someone else should probably review it since I wrote a lot of this code :)

extensions/blocks/eventbrite/index.js

matticbot · 2020-03-04T16:10:47Z

apeatling, Your synced wpcom patch D39459-code has been updated.

scruffian · 2020-03-04T16:11:43Z

I added support for wide and full alignments. Here's how they look:

It's not ideal that the iframe content is restricted in width, but it feels like it's ok to offer these options if someone wants them...

Copons

This seems to work fine for me.
I gave a look at Eventbrite's docs to see if there's a way to control the embedded max width, but apparently not, and so we are stuck to 1080px max.

While I'm not super into the gray background, we also need to consider that the effect would wildly vary based both on the screen size and the theme sizes.
What looks a bit off on Twenty Twenty on desktop, might look better on another theme, or another device.

apeatling · 2020-03-04T21:06:34Z

Looks good thanks for the update @scruffian. 👍

jeherve

LGTM! 👍

scruffian · 2020-03-05T17:23:06Z

r203918-wpcom

* 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

Eventbrite Alignment: Add left, center, right alignment support to th…

aca8baa

…e Eventbrite block. Fixes #14505

apeatling added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. labels Feb 26, 2020

apeatling requested review from a team and creativecoder February 26, 2020 18:21

apeatling self-assigned this Feb 26, 2020

matticbot added the Touches WP.com Files label Feb 26, 2020

jeherve added the [Block] Eventbrite label Feb 27, 2020

jeherve added this to the 8.4 milestone Feb 27, 2020

apeatling requested a review from a team February 27, 2020 16:04

Eventbrite Alignment: Add position relative to block to make sure tha…

ca78d71

…t the iframe cannot be interacted with when alignment is set. See #14820

pablinos previously approved these changes Feb 27, 2020

View reviewed changes

scruffian previously requested changes Mar 2, 2020

View reviewed changes

scruffian added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 2, 2020

Eventbrite Alignment: Make sure the alignment classes are added to th…

f8e4853

…e server side rendered iframe embed.

apeatling dismissed pablinos’s stale review via f8e4853 March 2, 2020 22:13

apeatling requested a review from scruffian March 2, 2020 22:15

apeatling added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 2, 2020

Update the CSS to target the new class name

cbddd60

apeatling requested a review from a team March 3, 2020 18:44

scruffian previously approved these changes Mar 4, 2020

View reviewed changes

jeherve reviewed Mar 4, 2020

View reviewed changes

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

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 4, 2020

Add support for wide and full alignments

f7dfb6e

scruffian dismissed their stale review via f7dfb6e March 4, 2020 16:10

Copons approved these changes Mar 4, 2020

View reviewed changes

scruffian added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 5, 2020

jeherve approved these changes Mar 5, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 5, 2020

scruffian merged commit bf3e4cc into master Mar 5, 2020

scruffian deleted the add/eventbrite-button-alignment branch March 5, 2020 17:10

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

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

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

Changelog: add #14817

cb39e54

Conversation

apeatling commented Feb 26, 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:

Uh oh!

matticbot commented Feb 26, 2020

Uh oh!

jetpackbot commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

creativecoder commented Feb 26, 2020

Uh oh!

creativecoder commented Feb 26, 2020

Uh oh!

matticbot commented Feb 27, 2020

Uh oh!

apeatling commented Feb 27, 2020

Uh oh!

pablinos left a comment

Choose a reason for hiding this comment

Uh oh!

scruffian commented Mar 2, 2020

Uh oh!

scruffian left a comment

Choose a reason for hiding this comment

Uh oh!

apeatling commented Mar 2, 2020

Uh oh!

matticbot commented Mar 2, 2020

Uh oh!

apeatling commented Mar 2, 2020

Uh oh!

matticbot commented Mar 3, 2020

Uh oh!

scruffian commented Mar 3, 2020

Uh oh!

apeatling commented Mar 3, 2020

Uh oh!

scruffian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matticbot commented Mar 4, 2020

Uh oh!

scruffian commented Mar 4, 2020

Uh oh!

Copons left a comment

Choose a reason for hiding this comment

Uh oh!

apeatling commented Mar 4, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

scruffian commented Mar 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jetpackbot commented Feb 26, 2020 •

edited

Loading