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.

Sign up for GitHub

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

Jump to bottom

Sharing: Add X button #33134

Merged
merged 11 commits into from
Oct 3, 2023
Merged

Sharing: Add X button #33134

merged 11 commits into from
Oct 3, 2023

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Sep 18, 2023 •
edited
Loading

Fixes #32178
Fixes #33315

Proposed changes:

This PR does 2 things:

  1. It adds a new button for the X service.
  2. It changes the default sharing buttons added on sites that enable the module for the first time from Twitter and Facebook to Facebook and X.

Notes

  • It does not replace the existing Twitter button, for the reasons we discussed in Social Menu & Social Media Icons: Add X support #33118. This is less forceful on site owners.
  • The new Share_X class extends the existing Twitter class so we can rely on that for now.
  • At some point in the future, we may be able to merge the 2, once the Twitter brand is more deprecated than it is today.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

No discussion on this, but some previous related PRs:

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

  1. Go to Jetpack > Settings > Social and enable sharing buttons.
  2. Go to Settings > Sharing and add a Twitter and an X sharing button.
  3. They should both look good in wp-admin and on the frontend.
  4. Clicking the button should work too.

Note
Matching Calypso PR: Automattic/wp-calypso#81835

@jeherve
Social Menu & Social Media Icons: Add X support
696824a 
Update the icon font, the social icons widget, the social menu, the social-logos package to include the new X icon.

Noting that this purposefully does not replace the existing Twitter icon. This will allow site owners to decide whether the icon they'd like to use. It should be less forceful that way, we will not update the existing Twitter icon that may be in use on many sites; site owners will do that themselves if they'd like.

Related PR: Automattic/social-logos#126
@jeherve
Merge remote-tracking branch 'origin/trunk' into add/x-icon
9a991fc
@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Sharing Post sharing, sharing buttons [Status] In Progress [Pri] Normal labels Sep 18, 2023
@jeherve jeherve self-assigned this Sep 18, 2023
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Sep 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 18, 2023 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

Jetpack plugin:

  • Next scheduled release: October 10, 2023.
  • Scheduled code freeze: October 9, 2023.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 18, 2023 •
edited
Loading

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/twitter-sharing branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack update/twitter-sharing

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@jeherve jeherve changed the title Social Menu & Social Media Icons: Add X support Sharing: Add X button Sep 18, 2023
jeherve added a commit to Automattic/wp-calypso that referenced this pull request Sep 18, 2023
@jeherve
Social Logos: update to the latest version, with X logo.
5563385 
Related:

- Automattic/social-logos#126
- Automattic/jetpack#33134
@jeherve jeherve mentioned this pull request Sep 18, 2023
Merged
7 tasks
@jeherve jeherve requested a review from a team September 18, 2023 12:47
@jeherve jeherve force-pushed the update/twitter-sharing branch from 5439382 to 5ae2354 Compare September 21, 2023 16:28
@jeherve jeherve force-pushed the update/twitter-sharing branch from 5ae2354 to bb74dfb Compare September 22, 2023 07:26
@jeherve jeherve added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Sep 22, 2023
@jeherve jeherve requested a review from a team September 22, 2023 07:26
@anomiex anomiex mentioned this pull request Sep 27, 2023
Closed
@jeherve jeherve mentioned this pull request Sep 29, 2023
Merged
3 tasks
@jeherve
Sharing: add X sharing button
b21e9ae 
This adds a new button for the X service.

- It does not replace the existing Twitter button, for the reasons we discussed in #33118. This is less forceful on site owners.
- The new Share_X class extends the existing Twitter class so we can rely on that for now.
- At some point in the future, we may be able to merge the 2, once the Twitter brand is more deprecated than it is today.
@jeherve
Update default sharing buttons
37c9f87
@jeherve jeherve force-pushed the update/twitter-sharing branch from bb74dfb to 37c9f87 Compare September 29, 2023 13:07
Base automatically changed from add/x-icon to trunk September 29, 2023 13:45
zinigor
zinigor previously requested changes Sep 29, 2023
View reviewed changes
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

I don't know if I'm testing this wrong, but the actual sharing doesn't work for me. When I click the X sharing button, a new window opens that just takes me to the same page. Can it be due to the fact that the URL parameter still says twitter instead of X?

@zinigor zinigor 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] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 29, 2023
jeherve added 2 commits October 2, 2023 12:07
@jeherve
Merge remote-tracking branch 'origin/trunk' into update/twitter-sharing
90bcb22
@jeherve
Expand new class to include existing
4a8fc54 
We cannot rely on extending the Twitter class, since it is not always available.

See #33134 (review)
@jeherve
Copy link
Member Author

jeherve commented Oct 2, 2023

Ah, that's my bad. I had only tested on a site where the old Twitter button was also active. This obviously won't always be the case, so we cannot just extend the Twitter class. I've just pushed 4a8fc54
to take care of this issue. It does duplicate a few things, but I think this is fine for now; at some point in the future we will eventually get rid of the Twitter class.

@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Oct 2, 2023
sdixon194
sdixon194 approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@sdixon194 sdixon194 left a comment

Choose a reason for hiding this comment

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

Looks good to me and works! No official button is available yet right? Approving this PR so we can merge and test in the beta, and if there's an option for an official button later we can add it to a followup.

@sdixon194 sdixon194 merged commit 193e017 into trunk Oct 3, 2023
@sdixon194 sdixon194 deleted the update/twitter-sharing branch October 3, 2023 19:19
@github-actions github-actions bot removed [Status] Needs Team Review [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Oct 3, 2023
@github-actions github-actions bot added this to the jetpack/12.7 milestone Oct 3, 2023
@jeherve
Copy link
Member Author

jeherve commented Oct 4, 2023

No official button is available yet right?

Correct. Maybe one will be added at some point in the future.

jeherve added a commit that referenced this pull request Oct 4, 2023
@jeherve
Sharing: improve the display of the official X button
03f67a7 
Follow-up to #33134
@jeherve jeherve mentioned this pull request Oct 4, 2023
Merged
3 tasks
@jeherve
Copy link
Member Author

jeherve commented Oct 4, 2023

@sdixon194 Following-up to your remark about official buttons, I just opened #33474 to improve the button when using the Official style.

Thank you!

mikestottuk pushed a commit that referenced this pull request Oct 6, 2023
@jeherve @sdixon194
Sharing: Add X button (#33134)
cef9280 
Co-authored-by: sdixon194 <steve.dixon@automattic.com>
@laurelfulford laurelfulford mentioned this pull request Dec 2, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdixon194 sdixon194 sdixon194 approved these changes

@zinigor zinigor zinigor left review comments

Assignees

@jeherve jeherve

Labels
[Feature] Sharing Post sharing, sharing buttons [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
jetpack/12.7
Development

Successfully merging this pull request may close these issues.

Enhancement: Change Twitter share button to X. Sharing / Social: update Twitter logo
3 participants
@jeherve @zinigor @sdixon194