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: remove Photon module dependency #13471

Merged
merged 2 commits into from
Sep 20, 2019

Conversation

simison
Copy link
Member

@simison simison commented Sep 15, 2019

Make Tiled gallery block available without Photon module being active.

Requiring users to turn on "image optimizations" is confusing and arbitrary, and we don't depend on that technically either.

If there's an agency/partner that wants to hide the block, they shouldn't need to disable photon for doing that and there's a separate mechanism in Jetpack for hiding specific blocks.

It's not great to turn on Photon module for the whole page load either (that's what current tiled gallery does). All the other images on the page should be un-affected by Tiled gallery's dependency on Photon servers.

Resolves #13470

Similar question for Markdown block: #13473

Changes proposed in this Pull Request:

  • Remove Photon class & module check in block registration

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Changing

Testing instructions:

image

image

Proposed changelog entry for your changes:

  • Tiled gallery block is now available in block picker even if "image accelerator" feature is disabled.

@simison simison added [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Focus] FixTheFlows [Block] Tiled Gallery labels Sep 15, 2019
@simison simison requested a review from a team September 15, 2019 14:28
@jetpackbot
Copy link

jetpackbot commented Sep 15, 2019

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: October 1, 2019.
Scheduled code freeze: September 24, 2019

Generated by 🚫 dangerJS against c551d9f

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello simison! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D32831-code before merging this PR. Thank you!

@mmtr mmtr self-requested a review September 15, 2019 14:48
@simison simison added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Sep 15, 2019
Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jeherve
Copy link
Member

jeherve commented Sep 17, 2019

See my comment in #13473. I guess the same applies here as well :)

@simison
Copy link
Member Author

simison commented Sep 17, 2019

See my comment in #13473. I guess the same applies here as well :)

Replied at #13473 (comment)

@jeherve
Copy link
Member

jeherve commented Sep 19, 2019

Works for me. Should we update the block description to mention that it relies on our CDN? This would avoid confusion, and folks wouldn't have to contact support when they disable the Image CDN but keep seeing it being used in their galleries (this was a common issue back when tiled galleries were available as a single module).

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 19, 2019
@simison
Copy link
Member Author

simison commented Sep 19, 2019 via email

@jeherve
Copy link
Member

jeherve commented Sep 19, 2019

Yeah, I think it would make sense.

@simison
Copy link
Member Author

simison commented Sep 20, 2019

Filed another PR for description to not let this being blocked, pending copy review; #13502

Also rebased this.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 20, 2019
@jeherve jeherve merged commit 179396b into master Sep 20, 2019
@jeherve jeherve deleted the update/tiled-gallery-photon-dep branch September 20, 2019 10:17
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Tiled Gallery [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Focus] FixTheFlows Touches WP.com Files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tiled gallery block: make the block available without Photon module
5 participants