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

Publicize: point wp-admin users to WP.com calypso sharing page instead #3194

Merged
merged 3 commits into from
Jan 12, 2016

Conversation

jkudish
Copy link
Contributor

@jkudish jkudish commented Dec 18, 2015

This PR seeks to remove the publicize connection settings from WP.com's wp-admin and instead point users to the Calypso sharing page for their site.

screenshot from 2015-12-18 14-55-01

The code that powers the wp-admin sharing screen is considered legacy code, and is out of date with our latest Publicize connections codebase (powered by the WP.com REST API). We're taking aim at cleaning up old, less reliable code and reducing our technical debt, while offering the best user experience for users.

The XML-RPC methods will continue to be supported for the foreseeable future, even though the UI will be gone, but may be retired at some point once usage of older versions of Jetpack drops to low enough numbers.

With that in mind, it would be possible to re-create a wp-admin experience using the REST API. All of the endpoints are fully available and documented here: https://developer.wordpress.com/docs/api/#sharing

This PR is a merge of r128505-wpcom with additional removals for some JP-specific code.

To test:

  • Apply PR to your Jetpack site
  • Activate Publicize
  • Verify that you get the screenshot above in place of the publicize settings
  • Verify that the button and link take you to the expected Calypso Sharing page
  • Verify that you can still add and remove publicize connections
  • Verify that your added connections show up in wp-admin's Editor and Calypso Editor's as available connections
  • Verify that posting a post publicizes the post as expected
  • Verify that all of the above occurs without any php errors in your site's error log
  • Login with a secondary user on your JP site, verify that you're still presented with this:

screenshot from 2015-12-18 14-54-46

cc @timmyc @aduth @nylen for review

Joey Kudish added 2 commits December 18, 2015 21:30
Removes the publicize connection settings from WP.com's wp-admin and instead point users to the Calypso sharing page for their site.
@jkudish jkudish added [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Feature] AtD [Feature] Beautiful Math [Focus] Accessibility Improving usability for all users (a11y) labels Dec 18, 2015
@jkudish jkudish added this to the 3.9 milestone Dec 18, 2015
@jkudish jkudish added [Feature] Publicize Now Jetpack Social, auto-sharing [Type] Dotcom Merge and removed [Focus] Accessibility Improving usability for all users (a11y) [Feature] AtD [Feature] Beautiful Math [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. Test on WP.com labels Dec 18, 2015
@jkudish jkudish added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Dec 18, 2015
<h4><?php
printf(
wp_kses(
__( "We've recently made some updates to Publicize. Please visit the <a href='%s'>WordPress.com sharing page</a> to manage your publicize connections or use the button below.", 'jetpack' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid "recently" will become outdated pretty soon. Is there a better phrase or can we be sure to update it in, say, six months to something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually we can aim to remove this sentence all together, but yeah I see your point.

I don't want it to be too matter of fact without providing some context that this is an expected "new-ish" change.

Do you have a suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just drop "recently" ? Still conveys improvements, but without as much of a time implication.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to dropping "recently". Also, "publicize" should be capitalized in "to manage your publicize connections".

@samhotchkiss samhotchkiss 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 Jan 11, 2016
@dereksmart
Copy link
Member

this is pretty awesome 👍

@dereksmart
Copy link
Member

actually @jkudish I can't get posts to actually publicize with this patch.

edit: might be my local setup. Will look more tomorrow.

@dereksmart dereksmart added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 12, 2016
@jkudish
Copy link
Contributor Author

jkudish commented Jan 12, 2016

edit: might be my local setup. Will look more tomorrow.

That's likely. This PR doesn't affect the actual publicizing of posts.

@dereksmart
Copy link
Member

Yeah confirmed it's something broken with my site & publicize. Won't publicize from any branch. Others have confirmed this works.

:shipit:

dereksmart added a commit that referenced this pull request Jan 12, 2016
…nnection-settings

Publicize: point wp-admin users to WP.com calypso sharing page instead
@dereksmart dereksmart merged commit d6eee91 into master Jan 12, 2016
@dereksmart dereksmart deleted the remove/wp-admin-publicize-connection-settings branch January 12, 2016 15:53
dereksmart added a commit that referenced this pull request Jan 12, 2016
We need to figure out a way to handle resetting/adding/removing connections from wp-admin/post.php
@dereksmart dereksmart restored the remove/wp-admin-publicize-connection-settings branch January 12, 2016 19:15
dereksmart added a commit that referenced this pull request Jan 12, 2016
…-wp-admin

Publicize: Revert changes made in #3194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Publicize Now Jetpack Social, auto-sharing [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Status] Tested on WP.com Touches WP.com Files [Type] Dotcom Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants