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: add linkedin preview #14198

Merged
merged 5 commits into from
May 23, 2017
Merged

Publicize: add linkedin preview #14198

merged 5 commits into from
May 23, 2017

Conversation

jhnstn
Copy link
Member

@jhnstn jhnstn commented May 17, 2017

This adds Linkedin to publicize preview.
Testing

  • Enable publicize sharing on calypso.localhost:3000 : ENABLE_FEATURES=publicize-scheduling make run
  • Authorize Linkedin on a premium site
  • Open the sharing tab on a published post
  • Click 'Preview'
  • Linkedin should be available for previewing
  • Clicking "Linkedin share" on the left should show the correct linkedin share preview

breaks up #12644

@matticbot
Copy link
Contributor

@matticbot matticbot added the [Size] L Large sized issue label May 17, 2017
@jhnstn
Copy link
Member Author

jhnstn commented May 17, 2017

Copied comment from @iamtakashi :


LinkedIn

  • I don't see the account name.
  • Can we show the company name of the account that is required info of an account?
  • We need to show the site URL.

image

@jhnstn jhnstn force-pushed the add/publicize-linkedin-preview branch from d1f0f5d to dc3d4f1 Compare May 20, 2017 01:04
@matticbot matticbot added [Size] XL Probably needs to be broken down into multiple smaller issues and removed [Size] L Large sized issue labels May 20, 2017
@@ -13,6 +13,7 @@ import SocialLogo from 'social-logos';
const services = translate => ( {
facebook: { icon: 'facebook', label: translate( 'Facebook feed' ) },
google: { icon: 'google', label: translate( 'Google search' ) },
google_plus: { icon: 'google-plus', label: translate( 'Google+ ' ) },
Copy link

Choose a reason for hiding this comment

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

Hi! I've found a possible matching string that has already been translated 55 times:
translate( 'Google' ) ES Score: 11
See 2 additional suggestions in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

@jhnstn jhnstn force-pushed the add/publicize-linkedin-preview branch from dc3d4f1 to 5852ad2 Compare May 22, 2017 22:51
@matticbot matticbot added [Size] L Large sized issue and removed [Size] XL Probably needs to be broken down into multiple smaller issues labels May 22, 2017
@jhnstn jhnstn force-pushed the add/publicize-linkedin-preview branch from 5852ad2 to 9953ccc Compare May 22, 2017 22:52
@jhnstn jhnstn added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label May 22, 2017
@jhnstn
Copy link
Member Author

jhnstn commented May 22, 2017

@iamtakashi Adding the company name is bit tricky given the way access linkedin account information. Here is what we have now in this PR:

screen shot 2017-05-22 at 3 51 09 pm

@jhnstn jhnstn added [Feature] Sharing Features and settings for sharing posts across different platforms, including sharing buttons. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels May 23, 2017
@jhnstn jhnstn force-pushed the add/publicize-linkedin-preview branch from 9953ccc to 8b734d3 Compare May 23, 2017 19:15
@rralian
Copy link
Contributor

rralian commented May 23, 2017

FWIW, the comment here (#14198 (comment)) is not how I see shares show up in Linkedin. Next to the avatar I see, three lines:

[name]
[company]
[age of post]

In any case, let's merge this for now and we can tweak the design in a followup. I've tested and it works as designed. :shipit:

@rralian rralian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 23, 2017
@jhnstn
Copy link
Member Author

jhnstn commented May 23, 2017

Thanks @rralian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Sharing Features and settings for sharing posts across different platforms, including sharing buttons. [Size] L Large sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants