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

Docs: Fix sample code of Block Preview in block registration docs #23899

Merged
merged 4 commits into from
Jul 24, 2020

Conversation

naogify
Copy link
Contributor

@naogify naogify commented Jul 13, 2020

Description

Fix example code of adding innerBlocks to Block Preview in block registration document.
I described the detail in #23898

How has this been tested?

I checked the preview of the document on my GitHub.
https://github.com/naogify/gutenberg/blob/docs/update-block-registration/docs/designers-developers/developers/block-api/block-registration.md

Screenshots

screenshot

Types of changes

Documentation

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@naogify naogify changed the title Fix sample code of Block Preview in block registration docs Docs: Fix sample code of Block Preview in block registration docs Jul 13, 2020
Copy link
Member

@ajitbohra ajitbohra left a comment

Choose a reason for hiding this comment

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

inner blocks array should contain block object

…ion.md


inner blocks array should contain block object

Co-authored-by: Ajit Bohra <[email protected]>
@naogify
Copy link
Contributor Author

naogify commented Jul 14, 2020

@ajitbohra Thank you :) I commited your change.

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Sorry to request more changes.

Just a couple of things to make this match the coding standards.

Comment on lines 199 to 207
{
name: 'core/paragraph',
attributes: {
/* translators: example text. */
content: __(
'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Praesent et eros eu felis.'
),
},
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This object (lines 199-207) should be indented by four spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @talldan.

I just have a question. According to the .editorconfig of Gutenberg, it says indent should be tab.
https://github.com/naogify/gutenberg/blob/docs/update-block-registration/.editorconfig#L14

The codingstandards of of documentaiotn is different from the one?

Copy link
Contributor

Choose a reason for hiding this comment

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

@naogify That's true, the coding standards say tabs - https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#spacing.

In my comment above I mostly opted for consistency with the surrounding code which seemed to be indented using four spaces, but checking the rest of the file it seems inconsistent, sometimes tabs, sometimes spaces.

I think it probably doesn't matter so much in documentation, as long as it looks correct. The trouble is that it's harder to enforce without code linting.

Copy link
Member

Choose a reason for hiding this comment

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

@talldan We have documentation linting and prettier for docs, not enforced yet as the config and definitions needs some refining, but the basics are in place.

Editor config for setting up Prettier:
https://developer.wordpress.org/block-editor/contributors/document/#editor-config

And the linting uses markdownlint that also can be setup in editor or run using: npm run lint:md-docs

Copy link
Contributor

Choose a reason for hiding this comment

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

@mkaz Thanks for the info! I managed to get that working in my editor. Looking forward to being able to write everything on one line, and then magically have it formatted when saving 😄 .

Copy link
Contributor Author

@naogify naogify Jul 25, 2020

Choose a reason for hiding this comment

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

@mkaz @talldan
Thank you for the information,and reply. I understand and will try the markdown linting. :)

name: 'core/paragraph',
attributes: {
/* translators: example text. */
content: __(
'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Praesent et eros eu felis.'
),
},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just missing a trailing comma 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.

Thank you. I fixed it.

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@mkaz
Copy link
Member

mkaz commented Jul 24, 2020

@ajitbohra ping, I think this one is all set to merge, can you clear your "request change"? Thanks!

@ajitbohra ajitbohra merged commit 6b6aa93 into WordPress:master Jul 24, 2020
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 24, 2020
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