-
Notifications
You must be signed in to change notification settings - Fork 800
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
Connection Banners: Add premium services to last slide #10867
Connection Banners: Add premium services to last slide #10867
Conversation
Connection Banners: first commit. beginning to add new content
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: January 10, 2019. |
Connection banner: adding premium services content to last slide
connection banner: styling list. adding images
@benhuberman do you or anyone on your team have the capacity to give this content a quick review? cheers! |
Sure, @jeffgolenski -- I'll aim to take a look by the end of the day. (If it's urgent, let me know.) |
One high-level comment: this looks very much like many other "long lists of things" that we see here and there in the Jetpack dashboard and which often feel overwhelming -- something to be mindful of in future iterations. Assuming we're sticking to this format for now, here are some suggestions/notes:
I'd suggest a few tweaks in the descriptions:
The final bullet on the list seems like it should be on its own line, outside the list itself. There's also a small typo there, plus a suggested tweak to the copy:
Let me know if you have any questions about these edits, @jeffgolenski, or if you'd like to follow up on any item here. |
@benhuberman Thanks so much for the quick response! I updated about 98% of the verbiage, I just left a couple of key words that I want to use. |
Connection Banner: Updates to verbiage based on feedback by @benhuberman
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few quick changes and extra spaces.
class.jetpack-connection-banner.php
Outdated
<div class="jp-wpcom-connect__slide jp-wpcom-connect__slide-six"> | ||
<h2><?php esc_html_e( 'More Jetpack features our users love', 'jetpack' ) ?></h2> | ||
<h2><?php esc_html_e( 'All the tools you’ll ever need for a more powerful WordPress site', 'jetpack' ) ?></h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to encode that apostrophe here. You can use ’
.
class.jetpack-connection-banner.php
Outdated
|
||
<div class="jp-wpcom-connect__content-icon jp-connect-illo"> | ||
<img src="<?php echo plugins_url( 'images/customize-theme-2.svg', JETPACK__PLUGIN_FILE ); ?>" alt="<?php | ||
<img src="<?php echo plugins_url( 'images/jetpack-poweringUp.svg', JETPACK__PLUGIN_FILE ); ?>" alt="<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend that we stick to all lowercase, with -
as separators, for file names. jetpack-powering-up.svg
should be better.
class.jetpack-connection-banner.php
Outdated
esc_attr_e( | ||
'Jetpack includes other features that help you customize your site', | ||
'Jetpack premium services offer even more powerful performance, security, and revenue tools to help you keep your site safe, fast, and help generate income', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we try to have periods at the end of all those sentences?
class.jetpack-connection-banner.php
Outdated
<ul> | ||
<li> | ||
<strong><?php esc_html_e( 'Real-time automated backups & malware scanning:', | ||
'jetpack'); ?></strong> <?php esc_html_e( ' Stay free of any hacking-related worries.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You seem to have an extra space here.
class.jetpack-connection-banner.php
Outdated
'jetpack'); ?> | ||
</li> | ||
<li> | ||
<strong><?php esc_html_e( 'Ad network:', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have an extra space here, and it seems there are more downer.
Pushed some changes here that should address your feedback, @jeherve — would appreciate if you could take another look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me now. Merging!
* 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
Connection Banners: Add premium services to last slide.
Currently the last slide of the connection banner doesn't really offer much valuable information. We're enhancing it to talk about our premium services.
Fixes: #10683
Changes proposed in this Pull Request:
BEFORE:
AFTER:
Testing instructions:
wp-admin/index.php
or/wp-admin/plugins.php
Content:
Proposed changelog entry for your changes:
No changelog entry for this enhancement as it does not affect the direct Jetpack experience for current customers. Just the marketing for potential customer who have not connected to wordpress.com yet.