Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Editing Toolkit: Add a workaround to prevent slider blocks breaking the editor at full width #46951

Merged
merged 2 commits into from
Oct 30, 2020
Merged

Editing Toolkit: Add a workaround to prevent slider blocks breaking the editor at full width #46951

merged 2 commits into from
Oct 30, 2020

Conversation

Copons
Copy link
Contributor

@Copons Copons commented Oct 30, 2020

Changes proposed in this Pull Request

  • Add a 100% max width to .interface-interface-skeleton__editor to prevent some "slider" blocks to incorrectly calculate their width to 33554400px when aligned at full width.

This is a workaround derived from WordPress/gutenberg#26552, and already included in Jetpack (Automattic/jetpack#17645, to be released on 2020-01-10).

The reason to add it to the Editing Toolkit is to be able to immediately release it to Atomic sites.

Testing instructions

Note: this is already fixed on Simple sites, so please test it on Atomic or local WordPress installs with Jetpack and/or CoBlocks installed.

  • Before applying this change, open the editor.
  • Add a Jetpack Slideshow block or a CoBlocks Post Carousel block.
  • Set it to full width, and notice how it "breaks" the editor width.
  • Apply this change, and reload the editor.
  • Notice that the block, at full width, doesn't break the editor anymore, and is only as wide as the available space.
  • You can also double-check the page code, by searching for the interface-interface-skeleton__editor element, and make sure it has max-width: 100%.

Fixes p9N30q-2GL-p2

@Copons Copons added [Type] Bug When a feature is broken and / or not performing as intended Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin labels Oct 30, 2020
@Copons Copons self-assigned this Oct 30, 2020
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=fix/editor-toolkit-interface-skeleton-width-workaround

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 30, 2020
@Copons Copons requested a review from a team October 30, 2020 13:27
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D52065-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

sirreal
sirreal reviewed Oct 30, 2020
View reviewed changes
apps/editing-toolkit/editing-toolkit-plugin/common/index.php Outdated
* @return bool True if the site needs a temporary fix for the incorrect slider width.
*/
function needs_slider_width_workaround() {
$gutenberg_path = ABSPATH . 'wp-content/plugins/gutenberg/gutenberg.php';
Copy link
Member

Choose a reason for hiding this comment

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

We can probably check if GUTENBERG_VERSION is defined and read the value.

This path likely won't work well on WPCOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did try GUTENBERG_VERSION on my local install, and it returns undefined. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I think that constant is only generated upon Gutenberg release.
The development version instead has just a define( 'GUTENBERG_DEVELOPMENT_MODE', true );.

I guess using both GUTENBERG_VERSION and GUTENBERG_DEVELOPMENT_MODE should make this safe enough.

sirreal
sirreal approved these changes Oct 30, 2020
View reviewed changes
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

I tested by uplodaing the built plugin to an Atomic site. With the plugin deactivated, the post carousel block is broken. When I activate the plugin, it's fixed.

fullofcaffeine
fullofcaffeine approved these changes Oct 30, 2020
View reviewed changes
Copy link
Contributor

@fullofcaffeine fullofcaffeine left a comment

Choose a reason for hiding this comment

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

:shipit:

@Copons Copons merged commit 7841844 into master Oct 30, 2020
@Copons Copons deleted the fix/editor-toolkit-interface-skeleton-width-workaround branch October 30, 2020 17:04
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 30, 2020
@Copons Copons mentioned this pull request Oct 30, 2020
Merged
ciampo pushed a commit that referenced this pull request Nov 2, 2020
@Copons
Editing Toolkit: Update to 2.8.3
770ee5a 
Release 2.8.3:

* Editor close button should deal with situation where experimental GB feature is unavailable (#46666)
* Premium Content: Remove double banners (#46659)
* Premium Content: Add missing migrations for margin fix (#46924)
* Editing Toolkit: Add a workaround to prevent slider blocks breaking the editor at full width (#46951)
* Focused Launch: added WIP Focused Launch modal behind a flag (#46686)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sirreal sirreal sirreal approved these changes

@fullofcaffeine fullofcaffeine fullofcaffeine approved these changes

@ockham ockham Awaiting requested review from ockham

@mrfoxtalbot mrfoxtalbot Awaiting requested review from mrfoxtalbot

Assignees

@Copons Copons

Labels
Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Copons @matticbot @fullofcaffeine @sirreal