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

Shortlinks: Register as REST API post field and Gutenberg plugin #10981

Merged
merged 3 commits into from
Dec 17, 2018

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Dec 15, 2018

In the WP.com old editor, we used to have the shortlinks handy in the sidebar. We don't have them in Gutenberg, and they can sometimes be a very handy feature. This PR registers the post shortlink as a REST API field, and as a Gutenberg plugin, which are necessary to be able to build the UI for shortlinks in Gutenberg.

Changes proposed in this Pull Request:

  • Add shortlinks to the REST API post response.
  • Register shortlinks as a Gutenberg plugin, so they're available in our block availability endpoint.

Testing instructions:

  • Start a JN site with this branch.
  • Connect the site, and activate all recommended features.
  • Write a post and save it.
  • Type wp.data.select( 'core/editor' ).getCurrentPost().jetpack_shortlink in your browser console.
  • Verify it returns a correct shortlink that leads to the post you just created.
  • Type window.Jetpack_Editor_Initial_State.available_blocks.shortlinks in your browser console.
  • Verify it returns {available: true}.
  • Want to test the actual Gutenberg extension that uses this? Go to Gutenberg: Add WP.com shortlinks extension wp-calypso#29475 and follow the test instructions there

Proposed changelog entry for your changes:

  • Shortlinks: Register as REST API post field and Gutenberg plugin

Notes

I'm marking this as low priority, as it isn't super important to have. In the same time, I think it's handy to have it, and it's straightforward enough so we could have it ready for the next Jetpack version.

@tyxla tyxla added [Feature] Shortlinks [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Feature Request [Pri] Low [Feature] WP REST API [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Dec 15, 2018
@tyxla tyxla added this to the 6.9 milestone Dec 15, 2018
@tyxla tyxla self-assigned this Dec 15, 2018
@tyxla tyxla requested review from a team December 15, 2018 13:20
@jetpackbot
Copy link

jetpackbot commented Dec 15, 2018

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 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against 457833f

@jeherve jeherve 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 Dec 17, 2018
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 well for me, except for this:

Type window.Jetpack_Editor_Initial_State.available_blocks.shortlinks in your browser console.
Verify it returns {available: true}.

For this to work, you would have to add shortlinks to the list of default plugins in class.jetpack-gutenberg.php.

I personally find this confusing to be honest, so I'll just link to my other comment about this :)
#10844 (comment)

@matticbot
Copy link
Contributor

D22402-code. (newly created revision)

@tyxla tyxla dismissed jeherve’s stale review December 17, 2018 10:14

Addressed feedback

@tyxla
Copy link
Member Author

tyxla commented Dec 17, 2018

I agree this could be confusing @jeherve! It was even more confusing because it worked for me without adding it specifically.

I've added it in 457833f, and this PR could use another review.

@tyxla tyxla 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 Dec 17, 2018
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.

Looking good now. 👍

Copy link
Member

@roccotripaldi roccotripaldi left a comment

Choose a reason for hiding this comment

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

works well!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 17, 2018
@jeherve jeherve merged commit 285e775 into master Dec 17, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 17, 2018
@jeherve jeherve deleted the update/shortlinks-rest-api-gutenberg branch December 17, 2018 15:55
@tyxla
Copy link
Member Author

tyxla commented Dec 18, 2018

Thank you folks!

@tyxla
Copy link
Member Author

tyxla commented Dec 18, 2018

Diff to manually sync this to WP.com: D22459-code

jeherve added a commit that referenced this pull request Dec 19, 2018
jeherve added a commit that referenced this pull request Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Shortlinks [Feature] WP REST API [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Low Touches WP.com Files [Type] Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants