Skip to content

Carousel: do not open modal when clicking on a link in a caption#11024

Merged
jeherve merged 2 commits intomasterfrom
fix/carousel-link-caption
Dec 21, 2018
Merged

Carousel: do not open modal when clicking on a link in a caption#11024
jeherve merged 2 commits intomasterfrom
fix/carousel-link-caption

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Dec 20, 2018

Fixes #10997

Changes proposed in this Pull Request:

The new Core gallery block introduces a different markup for captions.
This PR makes sure that when clicking on a link in a caption inside a core gallery, you get redirected
to that link, instead of having the Carousel modal open.

Testing instructions:

  • Start from a site with the Carousel module active, and running WP 5.0+

  • Create a new gallery using the Gallery block.

  • Add a caption to one of your images, with a link like so:
    image

  • Publish your post

  • View post.

  • Click on the link in the caption.

  • Make sure you are redirected to the link location, instead of having the Carousel modal open.

  • Check that clicking on the image itself works.

Proposed changelog entry for your changes:

  • Carousel: do not open modal when clicking on a link in a caption

Fixes #10997

The new Core gallery block introduces a different markup for captions.
This PR makes sure that when clicking on a link in a caption inside a core gallery, you get redirected
to that link, instead of having the Carousel modal open.
@jeherve jeherve added Bug When a feature is broken and / or not performing as intended [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Status] Needs Review This PR is ready for review. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Dec 20, 2018
@jeherve jeherve added this to the 6.9 milestone Dec 20, 2018
@jeherve jeherve self-assigned this Dec 20, 2018
@jeherve jeherve requested a review from a team December 20, 2018 17:28
@matticbot
Copy link
Copy Markdown
Contributor

D22601-code. (newly created revision)

@jetpackbot
Copy link
Copy Markdown
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against b629a58

Copy link
Copy Markdown
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

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

Works well! :shipit:

One thing I noticed, which probably not related to this PR - when closing carousel modal via 'X' button - I get redirected back to wp-admin (which is my previous page)

@brbrr brbrr added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Dec 21, 2018
Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

Code looks good, works as expected!

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 21, 2018

Thanks for the reviews. Merging.

when closing carousel modal via 'X' button - I get redirected back to wp-admin (which is my previous page)

Yep, that's #6244

@jeherve jeherve merged commit cc14b51 into master Dec 21, 2018
@ghost ghost removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 21, 2018
@jeherve jeherve deleted the fix/carousel-link-caption branch December 21, 2018 14:16
jeherve added a commit that referenced this pull request Dec 21, 2018
jeherve added a commit that referenced this pull request Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended [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 Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants