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

Traffic Settings: add Jetpack Search upgrade message #27059

Merged
merged 6 commits into from
Sep 18, 2018

Conversation

gibrown
Copy link
Member

@gibrown gibrown commented Sep 6, 2018

This adds an upgrade message in traffic settings where the Jetpack Search settings are displayed once a plan has been purchased.

The messaging is based on Automattic/jetpack#9747

I have also reordered the traffic settings page a bit to adjust which items are on top. Some thoughts behind this:

  • Put the things the user is more likely to toggle higher up - things that are not on by default for instance
  • Put stuff like disabling stats at the bottom since very few folks disable stats

One question I have here is that I am seeing "Warning: Failed prop type: Invalid prop description supplied to Banner." in my console (Chrome). I suspect this has to do with the gnarly html description I have used. Some way around this?

- add an upgrade message for Jetpack Search - also for wp.com sites
- reorder the traffic settings to put the stuff you may be more likely to toggle higher up
@gibrown gibrown added [Type] Enhancement [Feature] Site Settings All other general site settings. [Feature Group] A8C Marketing & Sales Automattic's marketing and sales operations for WordPress.com. labels Sep 6, 2018
@gibrown gibrown self-assigned this Sep 6, 2018
@matticbot
Copy link
Contributor

@alisterscott alisterscott added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 7, 2018
@ebinnion
Copy link
Contributor

ebinnion commented Sep 7, 2018

Top of traffic tab for Jetpack site:

screen shot 2018-09-07 at 2 25 02 pm

Top of traffic tab for WP.com site:

screen shot 2018-09-07 at 2 24 26 pm

@ebinnion
Copy link
Contributor

ebinnion commented Sep 7, 2018

I pushed a commit to address the prop warning. Besides that, the code looks OK to me. We should probably get a design review for the content in the banner since it seems like more than usual.

@ebinnion ebinnion added [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Sep 7, 2018
Copy link
Contributor

@ebinnion ebinnion left a comment

Choose a reason for hiding this comment

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

I'll approve for now. But, I'd suggest waiting until we get the design review, or making changes after the design review.

Copy link
Contributor

@eliorivero eliorivero left a comment

Choose a reason for hiding this comment

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

Left a comment and also tagged for copy review.

}

return (
<div>
<div style={ { marginBottom: '16px' } }>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be moved to a stylesheet.

@eliorivero eliorivero added the [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR label Sep 11, 2018
@michelleweber
Copy link

I'd tweak the headline: Add faster, more advanced searching to your site with Jetpack Professional

In the body copy:

  • I'd delete the first sentence entirely ("WP build in search is fine for small sites..."). Noneed to slag WP in our own UI.
  • "High-relevant" needs a hyphen here, but I'd reconsider it As written, it sounds like the search engine is highly relevant, rather than that it delivers highly relevant results. Since you make the point about relevancy in the next sentence, I'd delete the "highly-relevant' bit.
  • Delete "that is"

@gibrown gibrown added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels Sep 13, 2018
@gibrown
Copy link
Member Author

gibrown commented Sep 13, 2018

@ebinnion or @eliorivero can I get one last look. Only changes I made were to move the style into into the css sheet and some updates to the wording.

@jeffgolenski fyi, @michelleweber helped a bit with some feedback on the wording and I'm going with that. The latest looks like:

screen shot 2018-09-13 at 2 53 02 pm

I'll use the same wording in the Jetpack PR for an upgrade message.

@gibrown gibrown removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 18, 2018
@gibrown gibrown merged commit ed867f6 into master Sep 18, 2018
@jeherve jeherve deleted the add/jp-search-upgrade-msg branch September 26, 2018 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] A8C Marketing & Sales Automattic's marketing and sales operations for WordPress.com. [Feature] Site Settings All other general site settings. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants