Skip to content

Conversation

@gravityrail
Copy link
Contributor

@gravityrail gravityrail commented Jan 31, 2018

A suggestion from @beaulebens, it's handy for users to be able to enable a new feature when we add a UI for it.

So I added the module toggle right into the upgrade notice.

enable-lazy-images

Testing instructions:

  • Upgrade Jetpack, or force showing the upgrade prompt by using the following micro-plugin:
add_action( 'plugins_loaded', function() {
    Jetpack::state( 'message', 'modules_activated' );
});
  • Use the toggle in the screen to enable Lazy Images
  • Close the prompt and confirm that Lazy Images is enabled in the Jetpack settings and that images on the front-end are being lazy loaded (i.e. they have the data-lazy-loaded="1" attribute)

Proposed changelog entry for your changes:

Allow enabling new features from upgrade screen

@gravityrail gravityrail requested a review from a team as a code owner January 31, 2018 23:50
@gravityrail gravityrail self-assigned this Jan 31, 2018
@gravityrail gravityrail added this to the 5.8 milestone Jan 31, 2018
@gravityrail gravityrail added [Status] Needs Review This PR is ready for review. [Status] Needs Design Review Design has been added. Needs a review! labels Feb 1, 2018
@MichaelArestad
Copy link
Contributor

I love adding it as an interactive element! As I mention in Slack, I think it might be good to incklude the whole setting including description. At the very least it will look like an image of where it is and if they try to interact with it, magic!

Copy link
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

LGTM -- pushed a couple commits to fix eslint errors and console warnings

@dereksmart dereksmart 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 Feb 1, 2018
@gravityrail
Copy link
Contributor Author

Since it's approved in its current state, I am going to merge this and add the description in a separate PR.

@gravityrail gravityrail merged commit 4810840 into master Feb 1, 2018
@gravityrail gravityrail removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 1, 2018
@gravityrail gravityrail deleted the fix/enable-lazy-images-from-upgrade-screen branch February 1, 2018 22:19
jeherve pushed a commit that referenced this pull request Feb 2, 2018
* Allow enabling lazy images from the upgrade screen

* Style tweaks

* Add 8px padding to bottom of settings

* fix eslint -- remove unused

* fix console warnings nested div in p
@jeherve
Copy link
Member

jeherve commented Feb 2, 2018

cherry-picked to branch-5.8 in 4e18d6e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Lazy Images [Status] Needs Design Review Design has been added. Needs a review!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants