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

Block: Improve in file documentation to make it easier to edit content #107

Merged
merged 3 commits into from
Dec 29, 2017
Merged

Block: Improve in file documentation to make it easier to edit content #107

merged 3 commits into from
Dec 29, 2017

Conversation

gziolo
Copy link
Contributor

@gziolo gziolo commented Dec 20, 2017

This PR tries to improve the experience for the future block implementers by adding inline documentation based on the Gutenberg Handbook.

Initial suggestion came from @danielbachhuber in #88 (comment):

Another idea to throw into the mix: Add jsDoc to the scaffolded edit and save functions that provides an introductory overview to the functions and then links to the corresponding handbook docs.

In addition, I added the following changes:

  • Added CSS styles loaded for both editor and fronted.
  • Added a note that a recommended way is to generate a block inside a plugin.
  • Fixed a few linting errors for generated PHP and JS files.
  • Update API to use a new way of setting supportsHtml.

Screenshots

In the editor:

screen shot 2017-12-21 at 12 22 18

In the post:

screen shot 2017-12-21 at 12 22 25

@@ -126,6 +126,7 @@ Feature: WordPress block code scaffolding
Then the {THEME_DIR}/blocks/fight-club.php file should exist
Copy link
Contributor Author

Choose a reason for hiding this comment

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

2 tests are failing, need to be updated.

2 more tests need to verify if style.css file was created.

* This represents what the editor will render when the block is used.
* @see https://wordpress.org/gutenberg/handbook/block-edit-save/#edit
*
* @param {Object} props Properties passed to the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@param and @return need more love.

* which is then serialized by Gutenberg into `post_content`.
* @see https://wordpress.org/gutenberg/handbook/block-edit-save/#save
*
* @return {Object}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@return needs more love.

@gziolo
Copy link
Contributor Author

gziolo commented Dec 20, 2017

@danielbachhuber - how do you regenerate README file?

I tried wp scaffold package-readme but apparently failed with the following:

Error: 'package-readme' is not a registered subcommand of 'scaffold'. See 'wp help scaffold' for available subcommands.

@danielbachhuber
Copy link
Member

@gziolo Don't worry about it if you can't figure it out. We can do so at the very end.

@gziolo gziolo changed the title [WIP] Block: Improve in file documentation to make it easier to edit content Block: Improve in file documentation to make it easier to edit content Dec 21, 2017
@gziolo
Copy link
Contributor Author

gziolo commented Dec 21, 2017

Travis is finally happy, I added a few improvements and addressed all my comments. It needs to have README file regenerated before it gets merged. It's ready for a review.

I want to add support for another template in the follow-up PR.

Another thing I would like to explore is to add a flag for a plugin scaffolding command to optionally generate a block. Does it even make sense?

I want to have all that included in v1.5, so expect my PRs in the middle of January :)

/**
* This is the display title for your block, which can be translated with `i18n` functions.
* The block inserter will show this name.
*/
title: __( '{{title_ucfirst}}', '{{textdomain}}' ),

Choose a reason for hiding this comment

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

IIRC the domain is not handled yet and I wonder if the i18n module is ready for third-party usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove domain part or leave it as it is? I think it doesn't harm to have 2nd param :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I should remove the domain part from JS file. Related documentation in Gutenberg: https://github.com/WordPress/gutenberg/tree/master/i18n#usage.

Note that you will not need to specify domain for the strings.

Copy link

@youknowriad youknowriad 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 solid to me. I worry a bit about the links to the handbooks in case we change the urls and forget to update here.

Great Work @gziolo

@gziolo
Copy link
Contributor Author

gziolo commented Dec 21, 2017

This looks solid to me. I worry a bit about the links to the handbooks in case we change the urls and forget to update here.

The same issue applies to the API. I fix supportHTML changes in this PR.

@schlessera
Copy link
Member

schlessera commented Dec 21, 2017

@gziolo To refresh the README file:

  1. Install the wp-cli/scaffold-package-command package:
wp package install wp-cli/scaffold-package-command
  1. Run the wp scaffold package-readme command in the command's root folder:
wp scaffold package-readme . --force

@gziolo
Copy link
Contributor Author

gziolo commented Dec 22, 2017

I removed textdomain option as it turned out to be obsolete.

README is regenerated to use the updated PHPDoc. @schlessera thanks for sharing steps to do it 👍

It should be good to merge. I retested with the recent changes, everything works as expected.

icon: '{{dashicon}}',

{{/dashicon}}
/**
* Blocks are grouped into categories to help users browse and discover them.
* The core provided categories are `common`, `embed`, `formatting`, `layout` and `widgets`.
Copy link
Member

Choose a reason for hiding this comment

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

This seems grammatically off to me. Depending on the intended meaning, I'd suggest either of these two:

  • The provided core categories
  • The categories provided by core

*
* @param {Object} [props] Properties passed from the editor.
* @return {Element} Element to render.
*/
edit: function( props ) {
return el(
Copy link
Member

Choose a reason for hiding this comment

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

As this scaffolding serves as an onboarding tool as well, it would be useful to also document what the el() does here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@gziolo
Copy link
Contributor Author

gziolo commented Dec 27, 2017

I added more inline comments to make the generated code better documented.

@schlessera
Copy link
Member

There's still no documentation for the el() part, which is rather obscure for newcomers. Would you mind adding a short comment for that as well?

@schlessera
Copy link
Member

Ah, sorry, nvm my previous comment, I just failed to look properly.
This looks good to me now, merging.

@schlessera schlessera added this to the 1.1.2 milestone Dec 29, 2017
@schlessera schlessera merged commit 13b3ccb into wp-cli:master Dec 29, 2017
@gziolo gziolo deleted the update/scaffold-block branch December 29, 2017 16:57
danielbachhuber pushed a commit that referenced this pull request Nov 18, 2022
Block: Improve in file documentation to make it easier to edit content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants