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

Masterbar: Update the Reader link #14214

Merged
merged 2 commits into from
Dec 12, 2019
Merged

Masterbar: Update the Reader link #14214

merged 2 commits into from
Dec 12, 2019

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Dec 12, 2019

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:

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

Screen Shot 2019-12-12 at 2 50 05 pm

  • Now enable the WordPress.com toolbar

Screen Shot 2019-12-13 at 11 34 49 am

Screen Shot 2019-12-13 at 11 38 24 am

Proposed changelog entry for your changes:

…g A/B tests in Calypso on different default landing page destinations. Context: Automattic/wp-calypso#37547
@ramonjd ramonjd requested a review from a team as a code owner December 12, 2019 04:08
@ramonjd ramonjd self-assigned this Dec 12, 2019
@ramonjd ramonjd requested a review from a team December 12, 2019 04:09
@ramonjd ramonjd added [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations labels Dec 12, 2019
@jetpackbot
Copy link

jetpackbot commented Dec 12, 2019

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
andrewserong previously approved these changes Dec 12, 2019
Copy link
Member

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

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
kbrown9 previously approved these changes Dec 12, 2019
Copy link
Member

@kbrown9 kbrown9 left a comment

Choose a reason for hiding this comment

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

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

@kbrown9 kbrown9 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Dec 12, 2019
@jeherve jeherve added this to the 8.1 milestone Dec 12, 2019
@jeherve jeherve added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Dec 12, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

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:

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

@ramonjd
Copy link
Member Author

ramonjd commented Dec 12, 2019

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 jeherve dismissed stale reviews from kbrown9 and andrewserong via 018e43a December 12, 2019 10:21
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

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

@jeherve jeherve merged commit 3a6fdfa into master Dec 12, 2019
@jeherve jeherve deleted the update/masterbar-reader-url branch December 12, 2019 10:40
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 12, 2019
@ramonjd
Copy link
Member Author

ramonjd commented Dec 13, 2019

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 :)

jeherve added a commit that referenced this pull request Dec 13, 2019
zinigor added a commit that referenced this pull request Dec 30, 2019
* 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>
zinigor added a commit that referenced this pull request Dec 30, 2019
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [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.

6 participants