Secondary users: update notices to indicate a Jetpack account is needed by keoshi · Pull Request #10909 · Automattic/jetpack

keoshi · 2018-12-10T12:18:48Z

Fixes #6293

Changes proposed in this Pull Request:

Updates notices for secondary users to indicate a Jetpack account is needed.
Modifies main banner to incorporate Jetpack branding.
Adds plans gridicon (Jetpack icon).

Before

After

Testing instructions:

Fire up this PR.
Login to your site using a secondary account (as opposed to the primary one used to connect Jetpack).
Go to the Dashboard and Settings, and ensure you see the updates notices.

Proposed changelog entry for your changes:

Secondary users: update notices to indicate a Jetpack account is needed.

jetpackbot · 2018-12-10T12:20:43Z

	Warnings
⚠️	The PR is missing at least one [Status] label. Suggestions: `[Status] In Progress`, `[Status] Needs Review`

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against e1c1a2d

brbrr · 2018-12-10T13:24:48Z

@keoshi Could you please elaborate on decisions behind this PR? Why did we decide to use "Jetpack account" instead of "WordPress.com account" wording? As for me - naming update might be a potential source of confusion for users since they will be prompted to create WordPress.com account instead.

keoshi · 2018-12-10T13:35:16Z

@brbrr The full context is in the initial issue this PR fixes, but the important bit is this comment: #6293 (comment)

brbrr · 2018-12-10T13:41:25Z

Thanks! Although, I wasn't able to find an explanation of why the decision was made to use Jetpack account over WordPress.com account wording.

keoshi · 2018-12-10T13:45:59Z

@brbrr We're on the same boat here. :) The discussion happened so long ago that I'm afraid the context was lost in there at some point. But I think that differentiation was a secondary effect of moving from the “connecting” wording into “create an account”.

Even though technically we're creating a WordPress.com account, secondary users might not care to learn about that and just want enable features on their Jetpack site. Introducing more entropy there could be problematic, even though reading about a “Jetpack account” and then jumping into a WP.com account flow is not ideal either.

MichaelArestad

Other than a quick copy review, I think this is good to go.

MichaelArestad · 2018-12-10T20:15:32Z

_inc/client/components/jetpack-notices/index.jsx

 					<JetpackBanner
-						title={ __( 'Connect your account to get the most out of Jetpack' ) }
-						callToAction={ __( 'Connect to WordPress.com' ) }
+						title={ __( 'Jetpack is powering your site, but to access all of its features you’ll need to create a Jetpack account.' ) }


I think it would be good to have a copy review for this. I think it all looks pretty dang good, though. Nice work!

@Automattic/editorial can you help with this, please?

michelleweber · 2018-12-11T10:58:04Z

I'd delete the "Jetpack" in "Jetpack account" and just say "account." It's strongly implied, and the iconography makes it really clear that you're creating an account that is necessary for Jetpack to function. The sentence feels a little stilted with the double Jetpack. Otherwise, looking good!

keoshi · 2018-12-11T11:00:06Z

Thanks, @michelleweber — that makes total sense and helps answering the questions above.

@michelleweber

h/t @michelleweber

jeherve

It looks good on my end, but I find it a bit confusing that we use 2 different Jetpack icons so close to each other:

Is there a reason not to use the usual white on green?

keoshi · 2018-12-13T16:15:25Z

Good catch, @jeherve — do you know if that's using a Gridicon? I didn't see a JP one, so I added the plans gridicon, which includes the circle.

keoshi · 2018-12-13T17:13:53Z

Aw no, the JP logo in the JITMs is a PHP function :\

jetpack/class.jetpack-jitm.php

Lines 50 to 57 in 79c36df

    
           	/** 
        
           	 * Get's the Jetpack emblem 
        
           	 * 
        
           	 * @return string The Jetpack emblem 
        
           	 */ 
        
           	function get_emblem() { 
        
           		return '<div class="jp-emblem">' . Jetpack::get_jp_emblem() . '</div>'; 
        
           	}

jetpack/class.jetpack.php

Lines 7185 to 7194 in a356d87

    
           	/** 
        
           	 * Return string containing the Jetpack logo. 
        
           	 * 
        
           	 * @since 3.9.0 
        
           	 * 
        
           	 * @return string 
        
           	 */ 
        
           	public static function get_jp_emblem() { 
        
           		return '<svg id="jetpack-logo__icon" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" viewBox="0 0 32 32"><path fill="#00BE28" d="M16,0C7.2,0,0,7.2,0,16s7.2,16,16,16c8.8,0,16-7.2,16-16S24.8,0,16,0z M15.2,18.7h-8l8-15.5V18.7z M16.8,28.8 V13.3h8L16.8,28.8z"/></svg>'; 
        
           	}

jeherve · 2018-12-13T21:13:22Z

We have an SVG with the logo here:

jetpack/_inc/client/components/masthead/index.jsx

Line 70 in dd27e52

    
           <svg className="jetpack-logo__masthead" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" height="32" viewBox="0 0 118 32">

We could make it its own little component actually, I think it could be useful!

keoshi · 2018-12-14T10:40:32Z

@jeherve The problem is not the SVG, but that the banner component requires a Gridicon and doesn't even allow to change its size.

jetpack/_inc/client/components/banner/index.jsx

Lines 62 to 86 in f8078c2

    
           getIcon() { 
        
           	const { 
        
           		icon, 
        
           		plan, 
        
           	} = this.props; 
        
           	if ( plan && ! icon ) { 
        
           		return ( 
        
           			<div className="dops-banner__icon-plan"> 
        
           				<PlanIcon plan={ plan } /> 
        
           			</div> 
        
           		); 
        
           	} 
        
           	return ( 
        
           		<div className="dops-banner__icons"> 
        
           			<div className="dops-banner__icon"> 
        
           				<Gridicon icon={ icon || 'info-outline' } size={ 18 } /> 
        
           			</div> 
        
           			<div className="dops-banner__icon-circle"> 
        
           				<Gridicon icon={ icon || 'info-outline' } size={ 18 } /> 
        
           			</div> 
        
           		</div> 
        
           	); 
        
           }

I mean, we could change that, but for the sake of keeping this PR short and sweet, I think I'll look for an alternative. Would appreciate any suggestions you may have.

jeherve

If you are happy with the current look, then this should be good to go!

keoshi · 2018-12-17T11:04:34Z

I'd say it's a reasonably acceptable compromise with what we currently have in the JetpackBanner component.

jeffgolenski · 2018-12-17T21:10:08Z

@jeherve Not directly related to this issue, but is that Performance JITM showing when you're not connected? We should change that. Its irrelevant unless the customer is connected. cc @withinboredom

jeherve · 2018-12-18T08:45:35Z

@jeffgolenski It's a very good point. Only folks who can activate modules should see that JITM. I committed D22452-code to fix this.

* 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

keoshi added 4 commits December 10, 2018 12:13

Add plans Gridicon

06d0cd1

Tweak styles

6beec20

Update notice to create Jetpack account

28b0b2c

Update card notices to create Jetpack account

faa2676

keoshi added [Status] Needs Review This PR is ready for review. [Status] Needs Design Review Design has been added. Needs a review! Admin Page React-powered dashboard under the Jetpack menu [Focus] NUX labels Dec 10, 2018

keoshi self-assigned this Dec 10, 2018

keoshi requested a review from a team December 10, 2018 12:18

keoshi mentioned this pull request Dec 10, 2018

User-only connection banner wording improvement #6293

Closed

keoshi requested a review from a team December 10, 2018 14:34

MichaelArestad added the [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. label Dec 10, 2018

MichaelArestad previously approved these changes Dec 10, 2018

View reviewed changes

keoshi removed the [Status] Needs Design Review Design has been added. Needs a review! label Dec 11, 2018

keoshi removed the [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. label Dec 11, 2018

Tweak copy

1fbd316

h/t @michelleweber

keoshi dismissed MichaelArestad’s stale review via 1fbd316 December 11, 2018 12:23

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 13, 2018

jeherve previously approved these changes Dec 13, 2018

View reviewed changes

keoshi removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Dec 14, 2018

keoshi added 2 commits December 14, 2018 11:06

Change to WordPress gridicon

6da166a

Fix Jetpack green color

e1c1a2d

keoshi dismissed jeherve’s stale review via e1c1a2d December 14, 2018 11:07

keoshi requested a review from jeherve December 14, 2018 11:08

oskosk added the [Status] Needs Review This PR is ready for review. label Dec 14, 2018

keoshi requested a review from a team December 17, 2018 10:08

jeherve 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 17, 2018

jeherve approved these changes Dec 17, 2018

View reviewed changes

keoshi merged commit 098843f into master Dec 17, 2018

matticbot added the [Status] Needs Changelog label Dec 17, 2018

ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 17, 2018

keoshi deleted the update/secondary-user-notices branch December 17, 2018 11:04

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Dec 19, 2018

jeherve added a commit that referenced this pull request Dec 19, 2018

changelog: add #10909

1780a02

jeherve added a commit that referenced this pull request Jan 3, 2019

changelog: add #10909

22c77e6

benlk mentioned this pull request Jan 9, 2019

"Connect your account" banner is confusing: does "you" refer to the site or the user? #11114

Open

Conversation

keoshi commented Dec 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Before

After

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

jetpackbot commented Dec 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brbrr commented Dec 10, 2018

Uh oh!

keoshi commented Dec 10, 2018

Uh oh!

brbrr commented Dec 10, 2018

Uh oh!

keoshi commented Dec 10, 2018

Uh oh!

MichaelArestad left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelArestad Dec 10, 2018

Choose a reason for hiding this comment

Uh oh!

keoshi Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

michelleweber commented Dec 11, 2018

Uh oh!

keoshi commented Dec 11, 2018

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

keoshi commented Dec 13, 2018

Uh oh!

keoshi commented Dec 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve commented Dec 13, 2018

Uh oh!

keoshi commented Dec 14, 2018

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

keoshi commented Dec 17, 2018

Uh oh!

jeffgolenski commented Dec 17, 2018

Uh oh!

jeherve commented Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

keoshi commented Dec 10, 2018 •

edited

Loading

jetpackbot commented Dec 10, 2018 •

edited

Loading

keoshi commented Dec 13, 2018 •

edited

Loading

jeherve commented Dec 18, 2018 •

edited

Loading