Skip to content

Conversation

@jeherve
Copy link
Member

@jeherve jeherve commented Dec 18, 2019

Fixes #14257

Changes proposed in this Pull Request:

Twenty Twenty bundles a functionality named "Intrinsic Ratio Embeds", allowing videos (iframe, object, video) to be automatically resized on demand.
This is practical, but can be problematic for some elements on the page that may not behave like a video, or that may not be in the post content at all.

The Notifications iFrame is one such element. Let's use the CSS class provided within Twenty Twenty to be excluded from that behaviour:
https://core.trac.wordpress.org/browser/branches/5.3/src/wp-content/themes/twentytwenty/assets/js/index.js#L301

Testing instructions:

  • Enable the Twenty Twenty theme on your site.
  • Make sure the Notifications module is active.
  • Load your site's frontend
  • Ensure you can load the Notifications panel.

Proposed changelog entry for your changes:

  • Notifications: avoid conflicts with Twenty Twenty's instrinsic video resizes.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Notifications [Status] Needs Review This PR is ready for review. [Pri] Normal labels Dec 18, 2019
@jeherve jeherve added this to the 8.1 milestone Dec 18, 2019
@jeherve jeherve requested a review from a team December 18, 2019 17:42
@jeherve jeherve self-assigned this Dec 18, 2019
@jetpackbot
Copy link
Collaborator

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: January 14, 2020.
Scheduled code freeze: January 7, 2020

Generated by 🚫 dangerJS against d39390b

Copy link
Contributor

@ChaosExAnima ChaosExAnima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and LGTM 🚢

@ChaosExAnima ChaosExAnima 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 Dec 20, 2019
Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brbrr brbrr merged commit e442a0a into master Dec 25, 2019
@brbrr brbrr deleted the fix/twentytwenty-notifications branch December 25, 2019 11:55
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 25, 2019
zinigor added a commit that referenced this pull request Dec 27, 2019
zinigor added a commit that referenced this pull request Dec 30, 2019
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <[email protected]>
zinigor added a commit that referenced this pull request Dec 30, 2019
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Notifications [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications: Twenty Twenty theme JS blocks notifications menu from displaying properly (release candidate)

7 participants