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

Use 03-editable-esnext to introduce people to esnext #18

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

aaronjorbin
Copy link
Member

If you have not been exposed to esnext, it's easy to get lost. Concepts
like Destructuring assignment and arrow functions can be foriegn. This
adds inline documentation with esnext terminoly and links to mozilla
documentation. Additionally, Gutenberg specific concepts are linked to
in order to assist with understanding how to build your own blocks.

If you have not been exposed to esnext, it's easy to get lost. Concepts
like Destructuring assignment and arrow functions can be foriegn. This
adds inline documentation with esnext terminoly and links to mozilla
documentation. Additionally, Gutenberg specific concepts are linked to
in order to assist with understanding how to build your own blocks.
category: 'layout',
// this blocks structrued data
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "structrued" -> "structured"

icon: 'universal-access-alt',
// The category where the block will live in the block selector
Copy link
Member

Choose a reason for hiding this comment

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

Terminology-wise, I think it'd be good to be consistent with use of "Inserter" for block, over selector.

Copy link
Member

Choose a reason for hiding this comment

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

// see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const
//
// These are using Destructing Assignment which can be used to unpack properties from an object
// see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment
const { __ } = wp.i18n;
const { registerBlockType, Editable, source: { children } } = wp.blocks;
Copy link
Member

Choose a reason for hiding this comment

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

This is also out-of-date. There is no wp.blocks.source, and it's not used in this code anymore.

If we're wanting to introduce ESNext, I'd also recommend we stay away from nested destructuring, as I find it can be quite difficult to read, even when becoming familiar with the syntax.

attributes: {
content: {
// the data type we are storing
Copy link
Member

Choose a reason for hiding this comment

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

type: 'array',
// source and selector combine to dictate how to get this content
Copy link
Member

Choose a reason for hiding this comment

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

Note that this can be specific to the type of source. The HTML-based sources will have selector, but others (like meta, and perhaps eventually post, TBD) will have their own property configuration.

source: 'children',
selector: 'p',
},
},
// This uses arrow functions which inherit `this` from it's parent.
Copy link
Member

Choose a reason for hiding this comment

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

And in this instance, the this inheritance isn't really impactful, and personally, the use of arrow function here isn't really giving us any wins over the object function value shorthand: edit( props ) {

return (
// see: https://wordpress.org/gutenberg/handbook/block-api/editable-api/
Copy link
Member

Choose a reason for hiding this comment

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

This component has since been renamed RichText, but the documentation in the handbook is languishing and continuing to show the old component:

https://wordpress.org/gutenberg/handbook/block-api/rich-text-api/

@gziolo
Copy link
Member

gziolo commented Feb 20, 2018

You can copy inline docs also from https://wordpress.org/gutenberg/handbook/blocks/generate-blocks-with-wp-cli/#create-a-block-inside-the-plugin. They are included as part of the wp scaffold block command.

@aaronjorbin
Copy link
Member Author

@gziolo @aduth thank you both for your feedback. I've pushed up some updates.

@ryanwelcher
Copy link
Contributor

@aaronjorbin this PR has some merge conflicts. Do you mind addressing them or do you think it's better at this point to close and re-approach this given the amount of changes in the repo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants