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

Experiment: Put directives on <figure> element and insert lightbox as its child #51089

Conversation

artemiomorales
Copy link
Contributor

What?

This revises the HTML structure for the experimental image lightbox by ensuring there is no wrapping <div>.

Why?

Addresses #51068 and related issues as detailed in the following comment, namely that containing a wrapping <div> causes existing styles and functionality to break.

How?

Adds directives to the <figure> element using the tag processor, and does a regex for the <figure> closing tag, adding the lightbox HTML as its child.

Testing Instructions

  1. Enable Gutenberg > Experiments > Core Blocks
  2. Add an image to a post
  3. Enable Advanced > Behaviors > Lightbox in the image block settings
  4. Publish the post and inspect the source — see that the data-wp-context and data-wp-island attributes have been added to the <figure> element.
  5. Click on the image; the lighbox should work.

Testing Instructions for Keyboard

N/A

@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. labels May 30, 2023
@artemiomorales artemiomorales marked this pull request as ready for review May 30, 2023 10:12
@artemiomorales artemiomorales requested a review from ajitbohra as a code owner May 30, 2023 10:12
@artemiomorales artemiomorales requested a review from gziolo May 30, 2023 10:13
Copy link
Contributor

@SantosGuillamot SantosGuillamot left a comment

Choose a reason for hiding this comment

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

I've tested it, and it seems to work fine. Thanks for the pull request 🙂

It seems that there is a test not passing though.

@gziolo
Copy link
Member

gziolo commented May 30, 2023

Addresses #51068 and related issues as detailed in the #51068 (comment), namely that containing a wrapping

causes existing styles and functionality to break.

I tested a few scenarios, and it resolved the listed issues.

@artemiomorales
Copy link
Contributor Author

@gziolo Who else should we ping to move this fix forward?

@artemiomorales artemiomorales force-pushed the fix/revise-lightbox-html-structure branch from b28c011 to b01fb40 Compare May 31, 2023 15:06
@artemiomorales
Copy link
Contributor Author

artemiomorales commented May 31, 2023

I realized the directives were being unnecessarily duplicated in the <figure> in the lightbox; that has now been fixed.

@glendaviesnz
Copy link
Contributor

This tests well for me as a fix for the Gallery layout issue.

artemiomorales and others added 5 commits June 1, 2023 14:09
The <figure> directives were rendering not only in the body
content, but also the lightbox. This ensures the directives
do not appear in the lightbox.
@artemiomorales artemiomorales force-pushed the fix/revise-lightbox-html-structure branch from c81c27c to 8b67eb3 Compare June 1, 2023 12:14
@artemiomorales artemiomorales merged commit 60144f6 into WordPress:trunk Jun 1, 2023
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone Jun 1, 2023
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
… its child (WordPress#51089)

* Put directives on <figure> element and insert lightbox as its child

* Fix PHP spacing

* Ensure <figure> directives are only rendered in body content

The <figure> directives were rendering not only in the body
content, but also the lightbox. This ensures the directives
do not appear in the lightbox.

* Improve syntax

* Change directives suffix and `data-wp-island`

---------

Co-authored-by: Mario Santos <santosguillamot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling lightbox on images in a Gallery block breaks the gallery layout
7 participants