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

Site Settings: Disable Photon when in Dev mode #11783

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Mar 6, 2017

This PR will disable the Photon if the current Jetpack site is in development mode. Related with #11386.

Preview when Photon is disabled because of Dev mode:

To test:

  • Checkout this branch
  • Select one of your Jetpack sites and enable dev mode by adding this to your wp-config.php: define( 'JETPACK_DEV_DEBUG', true );
  • Verify the Photon toggle in the Media card in the Writing tab is disabled.
  • Disable the Dev mode by deleting the line we just added to wp-config.php.
  • Verify the Photon toggle is now enabled and can be interacted with.

/cc @rickybanister @MichaelArestad because I have the feeling that we need do something more in addition to just disabling the toggle in that case (as also mentioned here: #11386 (comment)).

@tyxla tyxla added Jetpack [Feature] Site Settings All other general site settings. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Enhancement labels Mar 6, 2017
@tyxla tyxla added this to the Jetpack Module Settings in Calypso milestone Mar 6, 2017
@tyxla tyxla self-assigned this Mar 6, 2017
@matticbot
Copy link
Contributor

@MichaelArestad
Copy link
Contributor

LGTM.

@MichaelArestad MichaelArestad removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Mar 6, 2017
Copy link
Member

@roccotripaldi roccotripaldi left a comment

Choose a reason for hiding this comment

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

LGTM!

@roccotripaldi roccotripaldi added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 6, 2017
@tyxla
Copy link
Member Author

tyxla commented Mar 7, 2017

Holding this one until we merge #11386

@tyxla tyxla force-pushed the update/jetpack-settings-photon-devmode branch from d24dddc to 3399da6 Compare March 8, 2017 13:37
@matticbot matticbot added the [Size] S Small sized issue label Mar 8, 2017
@tyxla tyxla merged commit 92c1cf8 into master Mar 8, 2017
@tyxla tyxla deleted the update/jetpack-settings-photon-devmode branch March 8, 2017 13:43
@rickybanister
Copy link

Quick question about this one—since Photon is a dependency for slideshows, shouldn't slideshows also be disabled when photon is off due to dev mode?

@tyxla
Copy link
Member Author

tyxla commented Mar 9, 2017

Photon is a dependency for slideshows

AFAIK Photon is not a dependency for Carousel, unless I'm missing something?

@MichaelArestad
Copy link
Contributor

I thought it was only a dependency for Tiled Galleries. @beaulebens can you clarify?

@beaulebens
Copy link
Member

I just tested this with Photon turned off, and;

  • Tile Galleries worked fine for me! (tried mosaic and circles)
  • Slideshow worked fine
  • Carousel worked fine

So, now I don't know where the expectation came from that Photon is required to use other things, but it doesn't appear to be accurate.

@MichaelArestad
Copy link
Contributor

@beaulebens hmmm. I know tiled galleries requires it. Maybe it turns it on just for the galleries? Can you check?

If so, I can just remove the Tiled Galleries comment.

@beaulebens
Copy link
Member

I take that back -- Photon gets used regardless on both Carousel and Tiled Galleries, even if Photon is turned off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings. Jetpack [Size] S Small sized issue [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants