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

Fix Lightbox use with Go theme and Carousel Gallery block #1587

Merged
merged 1 commit into from Aug 5, 2020

Conversation

AnthonyLedesma
Copy link
Member

@AnthonyLedesma AnthonyLedesma commented Jul 24, 2020

Description

The CoBlocks Lightbox works by adding click event listeners to document elements. Previous iterations of Lightbox worked by adding a click listener to the img elements which were arguably at the bottom of the document hierarchy. Because the event listener was near the bottom of hierarchy we made it possible for events to fire on elements that reside above our selected target. As a result of this bug, the Lightbox for Go theme when using the Carousel Gallery block would not function.

This PR changes the listener target from the lowest img element to the parent figure element of each respective image. This allows the event bubbling to function as expected even if theme styles result in click events firing on the Figure or the Img elements.

Screenshots

CarouselGoFixed

Types of changes

Minor JavaScript change to use .closest( 'figure' ) function.

How has this been tested?

Tested manually in Chrome, and Firefox.
Tested with and without the Gutenberg plugin active.
Tested with Go, TwentyTwenty, Barebones theme (underscores).

Checklist:

  • My code is tested
  • My code follows accessibility standards
  • My code has proper inline documentation
  • I've included any necessary tests
  • I've included developer documentation
  • I've added proper labels to this pull request

@AnthonyLedesma AnthonyLedesma added [Type] Bug Something that is not working as expected [Priority] High This issue/pull request needs resolving before the next release [Status] Needs Review Tracking pull requests that need another set of eyes labels Jul 24, 2020
@AnthonyLedesma AnthonyLedesma self-assigned this Jul 24, 2020
@richtabor richtabor added this to the Next Release milestone Jul 31, 2020
@richtabor richtabor requested review from jasonlemay and removed request for jrtashjian and EvanHerman August 3, 2020 18:09
src/js/coblocks-lightbox.js Show resolved Hide resolved
src/js/coblocks-lightbox.js Show resolved Hide resolved
@AnthonyLedesma AnthonyLedesma merged commit bfb2cd5 into master Aug 5, 2020
@AnthonyLedesma AnthonyLedesma deleted the fix/lightbox-go branch August 5, 2020 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] High This issue/pull request needs resolving before the next release [Status] Needs Review Tracking pull requests that need another set of eyes [Type] Bug Something that is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants