Skip to content

Conversation

zinigor
Copy link
Contributor

@zinigor zinigor commented Jan 27, 2017

Uses the new banner component introduced in the dops-component companion PR to display a POC notice that is only shown to users with a free plan.

Update:
I have rebased the PR, fixed lint errors in the current code. The banners now look like this:

VideoPress
media

Backups and Scan
security

Traffic
traffic

To test

After this line add the following:

  • to test the Premium plan: planClass = 'is-premium-plan';
  • to test the Business plan: planClass = 'is-business-plan';
  • to test the Free plan you can just be on a free plan or use planClass = 'is-personal-plan';

@zinigor zinigor added Admin Page React-powered dashboard under the Jetpack menu [Status] In Progress [Status] Needs Design Review Design has been added. Needs a review! [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jan 27, 2017
@zinigor zinigor added this to the 2/17 - February milestone Jan 27, 2017
@zinigor zinigor self-assigned this Jan 27, 2017
@rickybanister
Copy link

@MichaelArestad the checkbox style in the screenshot above don't match Calypso. Is that because @zinigor is not a Mac user? Checkboxes are styled appropriately on my system.

@MichaelArestad
Copy link
Contributor

@rickybanister I don't know. Do all checkboxes in JP look like that @zinigor?

@zinigor
Copy link
Contributor Author

zinigor commented Jan 30, 2017

Yes, I'm on Ubuntu, and that's how all checkboxes look on my system:
checkboxes

@eliorivero
Copy link
Contributor

eliorivero commented Jan 30, 2017

@rickybanister @MichaelArestad the checkboxes look like that in Firefox for iOS
captura de pantalla 2017-01-30 a las 12 06 25

@rickybanister
Copy link

I guess our Calypso form styles target webkit only?

@MichaelArestad
Copy link
Contributor

@rickybanister I don't think so. Some browsers don't let these things get styled too much. Looks like we are prefixed for moz, webkit, and modern browsers.

@eliorivero eliorivero modified the milestones: 2/17 - February, Settings UI Jan 30, 2017
@MichaelArestad MichaelArestad added [Status] Design Review Complete [Status] Needs Design Review Design has been added. Needs a review! and removed [Status] Needs Design Review Design has been added. Needs a review! [Status] Design Review Complete labels Feb 1, 2017
@MichaelArestad
Copy link
Contributor

MichaelArestad commented Feb 1, 2017

Okay. Let's workshop the copy and the number of banners.

List of things to promote by section:

Writing

  • Premium video (premium +)

Security

  • Antispam (personal +)
  • Backups (personal +)
  • Malware scans (premium +)

Traffic

  • SEO tools (professional)
  • Ads (premium +)
  • Google Analytics (professional)

Banner copy

Premium video

Premium video will be the easiest to deal with since it's the only upgrade available in the Writing section. I'm not thrilled with the subtitle, so suggestions super welcome.

This will be a banner related to the Premium plan. It will be attached to the Media group at the bottom.

Title: Add premium video
Subtitle: Upgrade to the Premium plan to easily upload videos to your website and display them using a fast, unbranded, customizable player

@MichaelArestad
Copy link
Contributor

CC @ashleighaxios for copy suggestions as we move forward with these.

@MichaelArestad
Copy link
Contributor

Security section

When on the free plan, something like this maybe:

image

When on the personal plan, remove the line about antispam.

When on Premium or above, don't show it at all. This is up for discussion since one could upgrade from premium to business, but I'm not sure we should show a banner at that level

@MichaelArestad
Copy link
Contributor

Traffic section

This section needs some serious copywriting love.

When on the free plan, something like this:

image

When on the Premium plan, remove the line about ads.

When on the Professional plan, remove the banner.

@MichaelArestad
Copy link
Contributor

Another thing to consider: we can make these dismissible. Should we?

@rickybanister
Copy link

I'm not sure dismissible helps in the same ways that the JITMs disappeared and we had no sensible way of bringing them back. I'm happy to open up the debate though. For reference, the banners on .com are not dismissible.

One other thought, we should make the banners color-coded to the plans they represent. Each plan has a color:

screen shot 2017-02-02 at 10 39 42 am

@MichaelArestad
Copy link
Contributor

MichaelArestad commented Feb 2, 2017

One other thought, we should make the banners color-coded to the plans they represent. Each plan has a color:

@rickybanister I don't have a great solution for that since in both combo banners, they aren't all really in one plan. For example, you can get antispam in the personal, but have to get premium to get the rest. I lean toward making it look like premium. I doubt the user will know or care since the only visual difference is the pen and a border.

@zinigor zinigor added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Feb 17, 2017
@eliorivero
Copy link
Contributor

eliorivero commented Feb 17, 2017

Fixed the GUI tests that were throwing an error

Invariant Violation: Could not find "store" in either the context or props of "Connect(SettingsCard)". Either wrap the root component in a <Provider>, or explicitly pass "store" as a prop to "Connect(SettingsCard)".

and then not running due to a missing property

TypeError: Cannot read property 'product_slug' of undefined

Opened a PR to fix issues found while gulp watch and yarn build.
Automattic/dops-components#95
Can you please review it?
Update: it's merged now.

@eliorivero
Copy link
Contributor

The banners included in this PR should not be visible when the site is in dev mode because they pass a not connected site as parameter and go to a URL like
https://wordpress.com/plans/localhost::somedevsite
which shows a blank page.

Even if they're visible, maybe they should have a different link.
cc @richardmuscat to confirm if we should show this in dev mode, maybe with a different link or text

@richardmuscat
Copy link
Contributor

richardmuscat commented Feb 20, 2017

@eliorivero: It wasn't intentional in the sense that I wasn't aware of it but we should keep the link with this modification:

Point the link towards the same URL but without the site parameter. So instead of:

https://jetpack.com/redirect/?source=plans-main-bottom&site=' + this.props.siteRawUrl

we just do

https://jetpack.com/redirect/?source=plans-main-bottom-dev-mode'

@richardmuscat
Copy link
Contributor

@MichaelArestad and @rickybanister -- these are looking cool but we need to add a call to action of some sort. A button or a link:

cta

@MichaelArestad
Copy link
Contributor

@richardmuscat The whole thing is a button/link and the first line in each one is a CTA

@MichaelArestad
Copy link
Contributor

Can you rebase this so we can test this without the build failing/warnings?

@dereksmart dereksmart added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2017
@dereksmart
Copy link
Contributor

@MichaelArestad I've rebased this

@dereksmart dereksmart added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 20, 2017
@richardmuscat
Copy link
Contributor

richardmuscat commented Feb 20, 2017 via email

@MichaelArestad
Copy link
Contributor

@richardmuscat that component has been working pretty well as is in Calypso. There is a CTA. It's already a flash of blue with a fancy image in a sea of white cards. We can add a button in the next iteration or a future PR. I want to get this thing in as is first (assuming I can get this to build properly).

@rickybanister
Copy link

For reference, here's how it would look with a button:

screen shot 2017-02-20 at 2 30 38 pm

@MichaelArestad MichaelArestad removed the [Status] Needs Design Review Design has been added. Needs a review! label Feb 20, 2017
@MichaelArestad
Copy link
Contributor

There are some display bugs on the Banner component, but that's unrelated to this PR. This looks good to go as a first pass.

@zinigor zinigor merged commit f57e648 into feature/settings-overhaul Feb 20, 2017
@zinigor zinigor removed the [Status] Needs Review This PR is ready for review. label Feb 20, 2017
@zinigor zinigor deleted the add/banner-component branch February 20, 2017 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants