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

Documentation: Add missing docs in editor package #60358

Closed
82 tasks done
ntsekouras opened this issue Apr 2, 2024 · 17 comments
Closed
82 tasks done

Documentation: Add missing docs in editor package #60358

ntsekouras opened this issue Apr 2, 2024 · 17 comments
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Developer Documentation Documentation for developers

Comments

 @ntsekouras
Copy link
Contributor

ntsekouras commented Apr 2, 2024

Recently, we added auto-generated docs for the editor package.

There are lots of undocumented APIs, so let's add what's missing.

We can do this iteratively, but it would be probably best to do it in chunks.

Instructions

  1. The documentation for editor package is auto generated, so after the addition of any JSDoc we need to npm run docs:build in order to update the README file. We shouldn't update manually the README file!
  2. There are some linting rules for JSDoc. You can check this doc's section about setting this up in your code editor.
  3. The generation script searches for JSDocs above the exported module. In some cases we declare the module, but export it in a different place (usually somewhere below in the same file). In that case we should add the JSDocs above the export.

Example:

 function EditorHistoryRedo() { ... } .... .... { Add JSDoc here!!! }  export default forwardRef( EditorHistoryRedo )
 @akasunil
Copy link
Member

Looks like there is issue with docs:build command, It generate the documentation only if component is exported as default. Otherwise pipeline get failed. How do we resolve this ?

 @ntsekouras
Copy link
Contributor Author

@sunil25393 can you give me an example of this? Because I just tested with usePostScheduleLabel which is not the default export and seems to work for me..

 @akasunil
Copy link
Member

Sure @ntsekouras , here is my draft PR #60932

 @colorful-tones
Copy link
Member

I'm just dropping an update here. I'm organizing an internal WP Engine Contributor Day on Thursday, May 16, 2024, and I plan on utilizing this issue to provide some folks with some quick wins and empower them to feel successful in their contribution efforts. Hopefully, we'll have all the items wrapped up that day. Thanks!

 @colorful-tones
Copy link
Member

@ntsekouras I'm happy to do a follow-up pull request to take a pass at refining the consistency of some of these docs. Two items I'm currently seeing:

Different use of @return (random sampling)

  • @return {JSX.Element} The rendered dropdown content.
  • @return {Component} The component to be rendered.
  • @return {JSX.Element|null} The rendered button component.
  • @return {Component|null} The component to be rendered. Return null if the post type doesn't...
  • @return {Element} The component to be rendered.

Description phrasing (random sampling)

  • Renders a control for enabling or disabling pingbacks and trackbacks in a WordPress post.
  • Renders the PostSlug component. It provide a control for editing the post slug.
  • This component renders a navigation bar at the top of the editor. It displays the title of the current...
  • Component handles the keyboard shortcuts for the editor.

 @akasunil
Copy link
Member

@colorful-tones I had the same thinking; I was waiting for the remaining documents to be completed so that we could refine them all at once.

 @ntsekouras
Copy link
Contributor Author

I'm happy to do a follow-up pull request to take a pass at refining the consistency of some of these docs. Two items I'm currently seeing:

That sounds good, thank you!

I was waiting for the remaining documents to be completed so that we could refine them all at once.

Yeah, it would be great to do them all at once, since the changes would be minimal.

 @colorful-tones
Copy link
Member

@ntsekouras it seems that ServerSideRender is deprecated and should not be on the list, correct? I just wanted to verify. Thanks.

export { default as ServerSideRender } from '@wordpress/server-side-render' ;

 @ntsekouras
Copy link
Contributor Author

@ntsekouras it seems that ServerSideRender is deprecated and should not be on the list, correct? I just wanted to verify

You're right. I'll remove it.

 @akasunil
Copy link
Member

akasunil commented Jun 10, 2024

I would go with these options for documentation uniformity

for return of component,
@return {Component|null} The component to be rendered or null if the post type doesn't

for description of component,
Renders a control for enabling or disabling pingbacks and trackbacks in a WordPress post.
Following simple structure of Render { render what? } for { whats the use } makes it easy to understand the purpose of component.

@colorful-tones @ntsekouras Let me know your thoughts.

 @colorful-tones
Copy link
Member

@akasunil I think your suggestions make sense.

I recommend we finalize the initial work in this issue, close it and then open a follow-up issue with notes/reference back to this one and submit a separate pull request towards the newer issue. Reasoning: clean break for work and intent, but I'm open to other ideas. Perhaps that is more work. 🤷

 @colorful-tones
Copy link
Member

@ntsekouras I'm not clear as to whether we need to add docs for transformStyles , as it is in packages/editor/src/index.js ? 🤔

https://github.com/WordPress/gutenberg/blob/trunk/packages/editor/src/index.js#L16

 @akasunil
Copy link
Member

recommend we finalize the initial work in this issue, close it and then open a follow-up issue with notes/reference back to this one and submit a separate pull request towards the newer issue.

Sounds good. And we will mention this issue in a new issue for future reference.

 @ntsekouras
Copy link
Contributor Author

Thank you all for the awesome work you did here! 🚀 Additional kudos to @colorful-tones for also organising a contributors day event!! transformStyles is just for back compat, so we can skip it and close this issue.

 @akasunil
Copy link
Member

@colorful-tones I have created issue #62744 as we discussed

Sign up for free to join this conversation on GitHub . Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

No branches or pull requests

3 participants