Masterbar: Update the Reader link by ramonjd · Pull Request #14214 · Automattic/jetpack

ramonjd · 2019-12-12T04:08:18Z

Fixes N/A

Changes proposed in this Pull Request:

We're updating the masterbar Reader link to explicitly go to wordpress.com/read

For background context, we'd like to set up an A/B test to redirect a (logged-in) test group to Customer Home instead of the Reader upon visiting wordpress.com

In order to achieve that we need to ensure we can manipulate the default landing page for /. Currently it is /read.

By updating instances of / to /read, we'll ensure that links to the Reader work regardless of which A/B test cohort the clicker is in.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

For context see:

p58i-89H-p2
EPIC Issue: https://github.com/Automattic/zelda-private/issues/173
Calypso PR: Move the Reader from / to /read wp-calypso#37547

Testing instructions

Connect your local ngroked Jetpack application to a wordpress.com site
Visit http://{yoursiteurl}/wp-admin/plugins.php?calypsoify=1
The Reader link in the masterbar should link to https://wordpress.com/read

Now enable the WordPress.com toolbar

The Followed Sites link in the sidebar should link to https://wordpress.com/read

Proposed changelog entry for your changes:

Updated the link to the Reader in the masterbar to point to https://wordpress.com/read

…g A/B tests in Calypso on different default landing page destinations. Context: Automattic/wp-calypso#37547

jetpackbot · 2019-12-12T04:10:41Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 14, 2020.
Scheduled code freeze: January 7, 2020

Generated by 🚫 dangerJS against 018e43a

andrewserong

Thanks for working on this one @ramonjd! Tested in my local Jetpack dev environment, and the masterbar Reader button now links through to https://wordpress.com/read.

Still might just want a review from @Automattic/jetpack-crew too, to see about getting this in the next release :)

kbrown9

This looks good to me! I tested the change, and the Reader link in the masterbar links to https://wordpress.com/read.

jeherve

This works for me, but I wonder if we shouldn't also change the Reader link in the regular Masterbar, displayed to everyone even when Calypsoify is off:

jetpack/modules/masterbar/masterbar.php

Line 407 in cd17208

'href' => '#',

Or do we want those folks to keep using the old link?

ramonjd · 2019-12-12T08:22:45Z

This works for me, but I wonder if we shouldn't also change the Reader link in the regular Masterbar, displayed to everyone even when Calypsoify is off:

Thanks @jeherve Totally missed that. All links to the Reader, whose underlying URL is wordpress.com should be switched to wordpress.com/read.

I'll take a look and change appropriately tomorrow.

jeherve

Thanks for the confirmation! I updated the link for you, this should now be good to merge!

ramonjd · 2019-12-13T00:44:42Z

Thanks for the confirmation! I updated the link for you, this should now be good to merge!

Thanks @jeherve ! I updated the test instructions anyway since it took me a while to work out where I should be seeing this link :)

* Changelog: 8.1 additions * Changelog: add #13858 * Changelog: add #13963 * Changelog: add #14174 * Changelog: add #14178 * Changelog: add #14175 * Changelog: add #14192 * Changelog: add #14196 * Changelog: add #14182 * Changelog: add #14218 * Changelog: add #14214 * Changelog: add #13757 * Changelog: add #14190 * Changelog: add #14131 * Changelog: add #14101 * Changelog: add #14203 * Changelog: add #14211 * Changelog: add #14224 * Changelog: add #14230 * Changelog: add #14241 * Changelog: add #14249 * Changelog: add #14264 * Changelog: add #14263 * Changelog: add #14256 * Changelog: add #10189 * Changelog: add #14240 * Changelog: add #14239 Also added some new entries to the testing file. Co-authored-by: Igor Zinovyev <zinigor@gmail.com>

Update the Reader link to go to wordpress.com/read as we're performin…

0dd3568

…g A/B tests in Calypso on different default landing page destinations. Context: Automattic/wp-calypso#37547

ramonjd requested a review from a team December 12, 2019 04:08

ramonjd self-assigned this Dec 12, 2019

ramonjd requested a review from a team December 12, 2019 04:09

ramonjd added [Status] Needs Review This PR is ready for review. [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations labels Dec 12, 2019

andrewserong previously approved these changes Dec 12, 2019

View reviewed changes

kbrown9 previously approved these changes Dec 12, 2019

View reviewed changes

kbrown9 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 12, 2019

jeherve added this to the 8.1 milestone Dec 12, 2019

jeherve added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Dec 12, 2019

jeherve reviewed Dec 12, 2019

View reviewed changes

Update Reader link in Masterbar as well

018e43a

jeherve dismissed stale reviews from kbrown9 and andrewserong via 018e43a December 12, 2019 10:21

jeherve approved these changes Dec 12, 2019

View reviewed changes

jeherve merged commit 3a6fdfa into master Dec 12, 2019

jeherve deleted the update/masterbar-reader-url branch December 12, 2019 10:40

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 12, 2019

jeherve added a commit that referenced this pull request Dec 13, 2019

Changelog: add #14214

b4f4195

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Dec 13, 2019

ramonjd mentioned this pull request Dec 17, 2019

[SIGNUP]: A/B test an alternative default logged-in landing page Automattic/wp-calypso#38463

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Masterbar: Update the Reader link#14214

Masterbar: Update the Reader link#14214
jeherve merged 2 commits intomasterfrom
update/masterbar-reader-url

ramonjd commented Dec 12, 2019 •

edited

Loading

Uh oh!

jetpackbot commented Dec 12, 2019 •

edited

Loading

Uh oh!

andrewserong left a comment

Uh oh!

kbrown9 left a comment

Uh oh!

jeherve left a comment

Uh oh!

ramonjd commented Dec 12, 2019 •

edited

Loading

Uh oh!

jeherve left a comment

Uh oh!

ramonjd commented Dec 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ramonjd commented Dec 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions

Proposed changelog entry for your changes:

Uh oh!

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

Uh oh!

Uh oh!

andrewserong left a comment

Choose a reason for hiding this comment

Uh oh!

kbrown9 left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

ramonjd commented Dec 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

ramonjd commented Dec 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ramonjd commented Dec 12, 2019 •

edited

Loading

jetpackbot commented Dec 12, 2019 •

edited

Loading

ramonjd commented Dec 12, 2019 •

edited

Loading