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 Editor: Add documentation for SpacingSizesControl component #68581

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

Conversation

Infinite-Null
Copy link
Contributor

Related to: #22891
Closes: #52070

What?

Adds documentation for the SpacingSizesControl component by:

  • Adding a comprehensive README.md file
  • Adding JSDoc comments with proper syntax, examples, and prop descriptions

Why?

The SpacingSizesControl component currently lacks documentation, making it difficult for developers to understand and implement the component correctly:

  • Clear documentation of all available props and their usage
  • Examples showing how to implement the component properly
  • Documentation of component views and internal structure

The documentation follows established patterns from other components while providing specific details relevant to SpacingSizesControl, making it easier for developers to use this component effectively in their blocks.

@Infinite-Null Infinite-Null marked this pull request as ready for review January 10, 2025 05:56
Copy link

github-actions bot commented Jan 10, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Infinite-Null <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: glendaviesnz <[email protected]>
Co-authored-by: mburridge <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added [Type] Developer Documentation Documentation for developers [Package] Block editor /packages/block-editor labels Jan 11, 2025
return (
<View className="tools-panel-item-spacing">
<SpacingSizesControl
values={ { all: value } }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t see where this all key is supported. I got here from testing #68339 so I even tried this with that story and it didn’t cause errors but also didn’t seem to do anything. It looks like this ought to be one of top, right, bottom, left. It’s probably best to use them all just for a complete example even though its seems they are all optional.

Comment on lines 32 to 34
const handleOnChange = ( unprocessedValue ) => {
onChange( unprocessedValue.all );
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it’d be fine to not have the wrapper and simply pass onChange onto SpacingSizesControl. At the least, the unprocessedValue.all seems like it shouldn’t be there (per my other comment about all not being a supported part of the API).

On the other hand, if it seems nice to demonstrate what the received value will consist of it could be something like this:

Suggested change
const handleOnChange = ( unprocessedValue ) => {
onChange( unprocessedValue.all );
};
const handleOnChange = ( { top, right = '0px', bottom, left = '0px' } ) => {
onChange( { top, right, bottom, left } );
};

@Infinite-Null
Copy link
Contributor Author

Hi @stokesman,
Thank you for the feedback!
I have implemented the requested changes. Please review them at your convenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add README.md file for spacing-sizes-control component
3 participants