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

Add upsell for Local SEO to search appearance #12646

Merged
merged 9 commits into from
Apr 15, 2019

Conversation

igorschoester
Copy link
Member

@igorschoester igorschoester commented Apr 10, 2019

Summary

This PR can be summarized in the following changelog entry:

  • [not-for-changelog] Adds an upsell for Local SEO to search appearance page when selecting an organization in the knowledge graph settings.

Relevant technical choices:

  • Using PHP to get the background image URL because:
    • When using the import it goes through react-svg-loader, which turns it into a component.
    • Now it has the added advantage of not being bundled in the JS too. Thanks Alexander!
  • Using just one image as background, not a fancier 9 sliced version, as this does not really change size too much. Thanks Sjardo!
  • The thing I'm not 100% on right now is the empty parent div that will always be put on the page (id: wpseo-local-seo-upsell). Then the component is mounted on that depending on whether we want the upsell there or not. I left this because adding more logic in the PHP view is not really nice either. If you have a strong opinion about this, please share.

Test instructions

This PR can be tested by following these steps:

Viewing the upsell.

  • Go to Search Appearance → General and select Organization as representation under the Knowledge Graph setting.
  • See the screenshot as to how it should look. See the issue as to how it was designed.

Install the helper plugin to manipulate the license

The upsell should only show when you do not have an active Local SEO license.

  • This is a bit tricky as it depends on which licenses are active on your site.
    • You can create another vagrant box to test a site without a license.
    • Or you can edit the PHP code. In admin/class-config.php on line 143 change the value of showLocalSEOUpsell into true or false.
  • Set the expiryDate in the helper plugin to a date in the past (2019-01-27T01:00:00.000Z).
  • Refresh the SEO -> Premium page to see if the status of Local SEO is indeed NOT ACTIVATED.
  • Ensure the upsell is shown.
  • Set the expiryDate in the helper plugin to a date in the future (2019-12-27T01:00:00.000Z).
  • Refresh the SEO -> Premium page to see if the status of Local SEO is indeed ACTIVATED.
  • Ensure the upsell is NOT shown.

The upsell should only show when you are in Free, never in Premium.

There is RTL support.

  • Change to a right-to-left (RTL) language in WordPress.
  • Ensure the upsell image is on the right and the background image is flipped horizontally.

The link should contain UTM tags and other info

  • Ensure the link goes to https://yoast.com/wordpress/plugins/local-seo/#utm_source=yoast-seo&utm_medium=software&utm_term=local-seo-kb-notification&utm_content=settings&utm_campaign=wordpress-ad
  • Ensure there is extra information passed like php_version and platform_version.

Screenshot

local_seo_upsell_screenshot

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Fixes https://github.com/Yoast/wordpress-seo-premium/issues/2127

This outputs the difference in style of styled-components to the snapshots.
Making the right-to-left style difference testable with snapshots.

Update existing snapshots to match.
@igorschoester igorschoester added the UI change PRs that result in a change in the UI label Apr 10, 2019
@igorschoester
Copy link
Member Author

While looking at Yoast/javascript#220 I see that the adding of jest-styled-components does not work nicely with styled-components v4.
Should discuss this later... @xyfi

@abotteram
Copy link
Contributor

CR 👍

@IreneStr
Copy link
Contributor

Acceptance 🚧
The upsell is showing, even though I have an active Local SEO plugin:
Schermafbeelding_2019-04-15_om_09_18_53
Schermafbeelding 2019-04-15 om 09 21 22

@igorschoester
Copy link
Member Author

igorschoester commented Apr 15, 2019

I'm not getting this reproduced, but I did get a nice plugin from Alexander to be able to test this easier!

Install test plugin

See https://github.com/Yoast/Wiki/wiki/Filter-the-subscriptions-response-in-the-addon-manager-request

@johannadevos
Copy link
Contributor

@IreneStr I cannot reproduce your situation either. But should we look into it again before merging this PR?

@IreneStr
Copy link
Contributor

It looks like it was cache-related, so it can be merged after your thumbs up @johannadevos

@johannadevos
Copy link
Contributor

Acceptance 👍

@johannadevos johannadevos added this to the 11.1 milestone Apr 15, 2019
@johannadevos johannadevos merged commit c174cc8 into trunk Apr 15, 2019
@johannadevos johannadevos deleted the 2127-ad-upsell-for-local-seo-to-search-appearance branch April 15, 2019 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI change PRs that result in a change in the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants