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

Tiled Gallery Block causes pointer cursor on every block that has nesting #13428

Closed
jasmussen opened this issue Sep 4, 2019 · 23 comments · Fixed by #13446
Closed

Tiled Gallery Block causes pointer cursor on every block that has nesting #13428

jasmussen opened this issue Sep 4, 2019 · 23 comments · Fixed by #13446
Assignees
Labels
[Block] Tiled Gallery Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Comments

@jasmussen
Copy link
Member

Steps to reproduce the issue

  1. Go to block editor
  2. Insert a Tiled Gallery block and add some images
  3. Insert any block that features nesting. E.g. Group, Columns, Media & Text.
  4. Preview the page and observe that every nesting block has a pointer cursor

Probably cause

The gallery appears to change the container divs of every block containing child blocks, not just the tiled gallery itself, and adds data-carousel-extra with a few image properties. This on its own seems a bit overreachy, but the actual cursor happens because this CSS is output by Jetpack:

[data-carousel-extra] { cursor: pointer }

Expected behavior

Tiled Gallery should probably just affect itself.

@jasmussen jasmussen added the [Type] Bug When a feature is broken and / or not performing as intended label Sep 4, 2019
@dereksmart dereksmart added this to the 7.8 milestone Sep 4, 2019
@jeherve jeherve added [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. labels Sep 5, 2019
@jeherve jeherve self-assigned this Sep 6, 2019
jeherve added a commit that referenced this issue Sep 6, 2019
Fixes #13428

Until now we would rely on 2 str_replace to add Carousel data to div and ul containers in post content.
This worked, but wasn't smart enough to know to ignore specific divs like Columns blocks.

By using DOMDocument, we can go through the post content in a more readable way, to detect gallery blocks as well as regular galleries and images.
jeherve added a commit that referenced this issue Sep 24, 2019
…13446)

* Carousel: improve detection of containers where we should add data

Fixes #13428

Until now we would rely on 2 str_replace to add Carousel data to div and ul containers in post content.
This worked, but wasn't smart enough to know to ignore specific divs like Columns blocks.

By using DOMDocument, we can go through the post content in a more readable way, to detect gallery blocks as well as regular galleries and images.

* Bail if the server does not support DOMDocument

* Store and return value from libxml_use_internal_errors

See https://github.com/Automattic/jetpack/pull/13446/files#r321866447

Co-Authored-By: Michael D Adams <michael.d.adams@gmail.com>

* No need to escape data key

See https://github.com/Automattic/jetpack/pull/13446/files#r321867238

Co-Authored-By: Michael D Adams <michael.d.adams@gmail.com>

* Wrap DOM usage in libxml_disable_entity_loader

See #13446 (review)
jeherve added a commit that referenced this issue Sep 24, 2019
…uld add data (#13446)

* Carousel: improve detection of containers where we should add data

Fixes #13428

Until now we would rely on 2 str_replace to add Carousel data to div and ul containers in post content.
This worked, but wasn't smart enough to know to ignore specific divs like Columns blocks.

By using DOMDocument, we can go through the post content in a more readable way, to detect gallery blocks as well as regular galleries and images.

* Bail if the server does not support DOMDocument

* Store and return value from libxml_use_internal_errors

See https://github.com/Automattic/jetpack/pull/13446/files#r321866447

Co-Authored-By: Michael D Adams <michael.d.adams@gmail.com>

* No need to escape data key

See https://github.com/Automattic/jetpack/pull/13446/files#r321867238

Co-Authored-By: Michael D Adams <michael.d.adams@gmail.com>

* Wrap DOM usage in libxml_disable_entity_loader

See #13446 (review)
@jeherve jeherve modified the milestones: 7.8, 7.9 Sep 27, 2019
@jeherve jeherve reopened this Sep 27, 2019
@jeherve jeherve removed this from the 7.9 milestone Oct 23, 2019
@donalirl
Copy link

Seen in #16173198-hc

@lezama
Copy link
Contributor

lezama commented Dec 10, 2019

Seen in 2548654-zen

@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" label Dec 10, 2019
@khristiansnyder
Copy link

Seen in #18730714-hc
A Tiled Gallery was within a column. Converting to Masonry Gallery fixed the issue.

@happychait
Copy link

Just had this report in 19440801-hc

@tanjoymor
Copy link

Another case here: #18635664-hc
Converting to Masonry Gallery fixed the issue.

@swoodipto
Copy link

Another report seen in 2820101-zen (during a 1:1 Support session)
I did not find the Masonry Gallery option there, but switching to a Slideshow fixes it.

@ebinnion
Copy link
Contributor

I was just about to create a new issue and Github pointed me here. Here are some steps that I used to reproduce the issue.

  1. Create a new Jetpack site
  2. Enable carousel
  3. Create a post that has columns, with content in each column
  4. Add a cover block with a background image and some nested text
  5. Add tiled gallery block, with or without media
  6. Publish page
  7. View page and observe that pointer cursor shows when hovering over column content

The root issue here seems to be that we're adding data-carousel-extra in a lot of places anddata-carousel-extra is what has the cursor: pointer; rule. Perhaps we should figure out how to not add data-carousel-extra in so many places.

Here's a screenshot of how I set up my test post:

Screen Shot 2020-03-30 at 8 24 35 PM

@ebinnion
Copy link
Contributor

Perhaps an easy win here would be to change the selector to something like .tiled-gallery__item that way we're specifically only applying the cursor to a Tiled Gallery item?

It seems like the latest patch above has been stalled for a bit.

@sourourn
Copy link

Another case reported here: #20603495-hc

@jordesign
Copy link
Contributor

FWIW - I was able to use this CSS to work around the issue in the time being:

div.wp-block-columns, 
div.wp-block-column {
    cursor:default
}

@fresatomica
Copy link

@aleone89
Copy link

Another report: hc-22637348

I had to apply the css above, to pretty much every block on the page to help for the time being.

@jasmussen
Copy link
Member Author

Could a bandaid fix be to simply remove the cursor CSS, or attach it to a class? That would fix the visual issue until the underlying issue can be addressed.

@benchilcote
Copy link

Another case surfaced in #27985992-hc

Gave this CSS as temporary workaround: [data-carousel-extra] {cursor: default !important;}

After I had a chance to check, it indeed was a gallery block that was present on the page. Changing to a masonry block fixed the issue.

Follow up: #3770531-zd

@ivan-ottinger
Copy link
Contributor

Another report: 3939250-zd-woothemes

Applying @benchilcote's CSS workaround helped. Thanks!

@davrox123
Copy link

Another report: zd 3987866

Applying @benchilcote's CSS workaround helped. Thanks!

@happychait
Copy link

4035130-zen

Can be worked around by the regular Gallery block to one of the other galleries that's not a Tiled Gallery block.

@daria2303
Copy link

4092207-zd-woothemes

applied CSS

@samiff
Copy link
Contributor

samiff commented Jun 29, 2021

This is still an issue with JP 9.9-beta and either the Gallery or Tiled Gallery block.

Related: #7771

@kbrown9
Copy link
Member

kbrown9 commented Aug 31, 2021

I fixed the cursor problem in PR #20882 by making the CSS selector more specific. However, we're still adding data-carousel-extra to many elements that shouldn't have it. I'm leaving this issue open so we can continue investigating better gallery parsing.

@jeherve
Copy link
Member

jeherve commented Aug 31, 2021

@kbrown9 Thanks for taking a look at this! ❤️ Do you think we could close this one now, and keep #13531 open for more improvements down the line?

@kbrown9
Copy link
Member

kbrown9 commented Aug 31, 2021

@kbrown9 Thanks for taking a look at this! ❤️ Do you think we could close this one now, and keep #13531 open for more improvements down the line?

I think that makes sense! That issue should cover the future improvements, so I'm closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Tiled Gallery Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet