-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Storybook: Add stories for BlockAlignmentMatrixControl Component #67292
Storybook: Add stories for BlockAlignmentMatrixControl Component #67292
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Thank you for the ping. We @WordPress/gutenberg-components will try to get to reviewing this soon, but in the meantime I'll add a few more folks in case they have capacity for a first round of review before us. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Thanks for noticing that the props in the README weren't complete 😅
| `label` | `string` | `Change matrix alignment` | concise description of tool's functionality. | | ||
| `onChange` | `function` | `noop` | the function to execute upon a user's change of the matrix state | | ||
| `value` | `string` | `center` | describes the content alignment location and can be `top`, `right`, `bottom`, `left`, `topRight`, `bottomRight`, `bottomLeft`, `topLeft` | | ||
| `label` | `string` | `Change matrix alignment` | Label for the control. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the block-editor
package, there are no strict rules for the format of props, but the table format is not used outside of this component.
The following format, which is the most widely used, may be a good choice.
### Props
### `propName`
- **Type:** `String`
- **Default:** `none`
- **Options:** `one`, `two`, `three`
Prop description goes here.
### `onChange`
- **Type:** `Function`
Prop description goes here.
control: 'text', | ||
table: { | ||
type: { summary: 'string' }, | ||
defaultValue: { summary: 'center center' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultValue: { summary: 'center center' }, | |
defaultValue: { summary: "'center'" }, |
I think the default value is "center"
.
| `value` | `string` | `center` | describes the content alignment location and can be `top`, `right`, `bottom`, `left`, `topRight`, `bottomRight`, `bottomLeft`, `topLeft` | | ||
| `label` | `string` | `Change matrix alignment` | Label for the control. | | ||
| `onChange` | `function` | `noop` | Function to execute upon a user's change of the matrix state. | | ||
| `value` | `string` | `center` | Describes the content alignment location and can be `center center`, `center left`, `center right`, `top center`, `top left`, `top right`, `bottom center`, `bottom left`, `bottom right`. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"center"
should also be acceptable:
export type AlignmentMatrixControlValue = | |
| 'top left' | |
| 'top center' | |
| 'top right' | |
| 'center left' | |
| 'center' | |
| 'center center' | |
| 'center right' | |
| 'bottom left' | |
| 'bottom center' | |
| 'bottom right'; |
description: 'Disables the control.', | ||
}, | ||
value: { | ||
control: 'text', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
control: 'text', | |
control: { type: null }, |
Since the value
prop is controlled by useState, I don't think a text field is necessary.
control: 'text', | ||
table: { | ||
type: { summary: 'string' }, | ||
defaultValue: { summary: 'Change matrix alignment' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultValue: { summary: 'Change matrix alignment' }, | |
defaultValue: { summary: "'Change matrix alignment'" }, |
It might be better to explicitly state that it is a string
type.
5efe581
to
613d0b0
Compare
Part of #67165
What?
This PR will add stories for BlockAlignmentMatrixControl component in the Storybook.
Testing Instructions
Screenshots or screencast