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.

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

Slideshow Block: Block breaks in Editor when set to Full Width #17616

Closed
ockham opened this issue Oct 26, 2020 · 27 comments · Fixed by #17645
Closed

Slideshow Block: Block breaks in Editor when set to Full Width #17616

ockham opened this issue Oct 26, 2020 · 27 comments · Fixed by #17645
Assignees
Labels
[Block] Slideshow Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Milestone

Comments

@ockham
Copy link
Contributor

ockham commented Oct 26, 2020

Steps to reproduce the issue

  1. Enable the Gutenberg plugin (version 9.2.2).
  2. Insert the Slideshow block into the editor, and add a few images.
  3. Set the block to full width.

What I expected

The block to work.

What happened instead

The block disappears; only the left/right slideshow controls remain

Screenshots
slideshow JP

Notes

The bug is not present when the Gutenberg plugin (v9.2.2) isn't enabled. This suggests that it might be related to the recent block-supports rewrite (as issues with block-supports tend to affect blocks that are set to full-width alignment).

Note the inline styling in the browser console in the above screencast: When changing the block layout to 'Full Width', two numeric values (for the width and transform attributes) are assinged weird floating point numbers that probably don't work in CSS 😬

AFAIK, that inline styling is provided by the Swiper library that we use to implement the Slideshow block, and added to the HTML element with the swiper-wrapper class. I suspect that the lib is attempting to read some dimensions from the block but gets them wrong and thus ends up settings those weird values.

Also note that the block toolbar isn't aligned with the block (rather than sitting right on top, it is the very left of the editor; it's more visible on a wide screen). This could be related to the same issue -- some dimensions being incorrectly computed.

This bug also affects WP.com. It has been discussed here: p1603739210304100-slack-C015AL3QL7M

cc/ @TheSteveK @eduardozulian @fullofcaffeine @nosolosw

@ockham ockham added [Type] Bug When a feature is broken and / or not performing as intended [Pri] High labels Oct 26, 2020
@ockham ockham added this to the 9.1 milestone Oct 26, 2020
@katiebethbrown
Copy link

Another report in 3284766-hc. Worth noting that it breaks the remaining content in the editor as well. Changing the slideshow to wide width returns things to normal so the page can be edited.

@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". label Oct 26, 2020
@nielslange
Copy link
Member

Another report in 25186634-hc.

@nielslange
Copy link
Member

Another report in 25193575-hc

@oandregal
Copy link

👋 I've bisected this error to this PR WordPress/gutenberg#25884 which was part of the 9.2 release Pinging @jeyip as he may have some more knowledge what happened there that affected this.

@ockham
Copy link
Contributor Author

ockham commented Oct 27, 2020

cc/ @Automattic/cylon 🙂

@Copons
Copy link
Contributor

Copons commented Oct 27, 2020

Screenshot 2020-10-27 at 16 46 20

33554400 pixel wide.

bc90b8e94e2aafe0


This doesn't look exactly right (but I've yet to figure out where these values come from)...

Screenshot 2020-10-27 at 16 58 04

@ockham
Copy link
Contributor Author

ockham commented Oct 27, 2020

This doesn't look exactly right (but I've yet to figure out where these values come from)...

Screenshot 2020-10-27 at 16 58 04

I had a theory that I mentioned in the PR desc, not sure you saw...

@Copons
Copy link
Contributor

Copons commented Oct 27, 2020

I had a theory that I mentioned in the PR desc, not sure you saw...

Apologize, I commented while doing other things and somehow missed the second half of the PR description. 🤦

I didn't even realize it was importing an external library, and looking at node_modules/swiper/swiper-bundle.js, those styles are indeed added by that (to $wrapperEl).

One extremely suspicious detail is that the block width seems to always be 33554400px, which in scientific notation would be 3.35544e7 (or 3.35544e+07, AFAIK are valid values in both JS and CSS), which is what ends up in the wrapper's translate3d and the slide's width.

@edequalsawesome
Copy link

Another report in 4752856-hc

@jartes
Copy link

jartes commented Oct 28, 2020

Another report in 21476315-hc

@jartes
Copy link

jartes commented Oct 28, 2020

And another one here 3439767-zen

@davipontesblog
Copy link

Another report here 24776915-hc

@khristiansnyder
Copy link

Another in 25208641-hc

@eoinsav
Copy link

eoinsav commented Oct 28, 2020

Another report of a user not able to edit their Pages because they all have Slideshow Blocks on them: #20607867-hc

@davipontesblog
Copy link

Another report: 8848011-hc

@Copons
Copy link
Contributor

Copons commented Oct 28, 2020

I've opened a PR in Gutenberg that seems to fix this issue: WordPress/gutenberg#26552.

@Automattic/good-mountain As mentioned elsewhere, we might need to bring it into JP immediately, rather than wait for the PR to get approved and included in the next Gutenberg release.

@davipontesblog
Copy link

Thanks @Copons ! I agree, I don't think this can wait for a Gutenberg release, it completely breaks the editor.

@ockham
Copy link
Contributor Author

ockham commented Oct 28, 2020

Thanks a ton, @Copons! I'll file a JP PR for now.

@JessBoctor
Copy link

Had another report in #25238205-HC

@ockham
Copy link
Contributor Author

ockham commented Oct 28, 2020

Thanks a ton, @Copons! I'll file a JP PR for now.

#17645
WP.com counterpart: D51956-code

I'll merge the WP.com counterpart soon, since this is affecting a lot of users.

@ockham
Copy link
Contributor Author

ockham commented Oct 28, 2020

I'll merge the WP.com counterpart soon, since this is affecting a lot of users.

r216021-wpcom

Edit: This should fix the issue on WordPress.com.

@eduardozulian
Copy link
Member

@ockham can you confirm if this also fixes AT sites? I just chatted on 23423694-hc and the issue persists unless I disable Gutenberg.

@ockham
Copy link
Contributor Author

ockham commented Oct 29, 2020

@ockham can you confirm if this also fixes AT sites? I just chatted on 23423694-hc and the issue persists unless I disable Gutenberg.

It will, but only once a Jetpack version that includes this fix is released (or a Gutenberg version that includes WordPress/gutenberg#26552 is released) and installed on Atomic. The next JP release is scheduled for November 10 -- if we need a fix for AT earlier, we'll have to ship it through a different vector (wpcomsh) 🤔

@Copons
Copy link
Contributor

Copons commented Oct 29, 2020

An update from Gutenberg side: we are pondering if reverting the change (WordPress/gutenberg#25884) that introduced this issue.
It's a bit of a pain, as we have merged a bunch of PRs relying on it, but still.

The workaround used here (#17645) would NOT be affected by a revert, nor would it cause any regressions, as the .interface-interface-skeleton__editor element has been introduced in WordPress/gutenberg#25884.
Once reverted, the JP fix would simply cease working, and it will be possible to just simply delete it.

I'll keep y'all updated with whatever we'll end up doing!

@mikeicode
Copy link

Another 23174703-hc

@mikeicode
Copy link

Another 25305689-hc

@Copons
Copy link
Contributor

Copons commented Nov 20, 2020

Well, I totally forgot to keep y'all updated as promised, but just as a heads up, this should be fixed everywhere for a few weeks now.
The fix has been merged separately in:

  • Gutenberg
  • Jetpack
  • Editing Toolkit

so it should cover any possible site type.
Given it's a one-line CSS fix, I think it's not a big deal to have it in 3 different places at the same time for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Slideshow Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.