Fix/update docs alignment matrix control #34624
Conversation
ryanwelcher
added
[Type] Developer Documentation
ryanwelcher
requested review from
ajitbohra and
chrisvanpatten
as code owners
There was a problem hiding this comment.
Hey @ryanwelcher, thank you so much for working on this! It's very appreciated :)
Despite this PR already being reviewed and merged, I took a look at it and left a few comments — would you mind addressing them by opening a follow-up PR?
| The class that will be added with "component-alignment-matrix-control" to the classes of the wrapper <Composite/> component. | ||
| If no className is passed only "component-alignment-matrix-control" is used. |
There was a problem hiding this comment.
I know that some older READMEs have a similar paragraph, but from now on let's not mention the "component-alignment-matrix-control" className in public docs
| Unique ID for the component. | ||
| - Type: `String` | ||
| - Required: No | ||
| ### label |
There was a problem hiding this comment.
Tiny nitpick: most headers used for props are missing an empty line above them
| ### label | |
| ### label |
|
|
||
| - Type: `function` | ||
| - Required: No | ||
| - Default: `noop` |
There was a problem hiding this comment.
I know that the default value for this prop is a noop function, but I'd remove this line from the README, as it may confuse a reader instead of adding value
|
|
||
| A function that receives the updated alignment value. | ||
|
|
||
| - Type: `function` |
There was a problem hiding this comment.
Could we replace types to be closer to how TypeScript types are described?
In this case, it could be something like ( nextValue: string ) => void
Other changes would be:
- from
Stringtostring - from
Numbertonumber
| - Required: No | ||
| ### label | ||
|
|
||
| If provided, sets the aria-label attribute of the wrapper <Composite/> component. |
There was a problem hiding this comment.
Nit:
| If provided, sets the aria-label attribute of the wrapper <Composite/> component. | |
| Accessible label. If provided, sets the `aria-label` attribute of the underlying <Composite/> component. |
| import { __experimentalAlignmentMatrixControl as AlignmentMatrixControl } from '@wordpress/components'; | ||
| import { useState } from '@wordpress/element'; | ||
|
|
||
| const Example = () => { | ||
| const [ alignment, setAlignment ] = useState( 'center center' ); | ||
| const [alignment, setAlignment] = useState('center center'); | ||
|
|
||
| return ( | ||
| <AlignmentMatrixControl value={ alignment } onChange={ setAlignment } /> | ||
| <AlignmentMatrixControl | ||
| value={alignment} | ||
| onChange={(newAlignment) => setAlignment(newAlignment)} | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Very very minor, but — it'd be great if the example snippet followed the repo's prettier formatting rules
| import { __experimentalAlignmentMatrixControl as AlignmentMatrixControl } from '@wordpress/components'; | |
| import { useState } from '@wordpress/element'; | |
| const Example = () => { | |
| const [ alignment, setAlignment ] = useState( 'center center' ); | |
| const [alignment, setAlignment] = useState('center center'); | |
| return ( | |
| <AlignmentMatrixControl value={ alignment } onChange={ setAlignment } /> | |
| <AlignmentMatrixControl | |
| value={alignment} | |
| onChange={(newAlignment) => setAlignment(newAlignment)} | |
| /> | |
| ); | |
| }; | |
| import { __experimentalAlignmentMatrixControl as AlignmentMatrixControl } from '@wordpress/components'; | |
| import { useState } from '@wordpress/element'; | |
| const Example = () => { | |
| const [ alignment, setAlignment ] = useState( 'center center' ); | |
| return ( | |
| <AlignmentMatrixControl | |
| value={ alignment } | |
| onChange={ ( newAlignment ) => setAlignment( newAlignment ) } | |
| /> | |
| ); | |
| }; |
| - Type: `String` | ||
| - Required: No | ||
| - Default: `Alignment Matrix Control` | ||
| ### defaultValue |
There was a problem hiding this comment.
Can we also add the value prop to the docs? It looks like it's currently missing
|
Thanks for the review, @ciampo, and sorry that I didn't catch those issues while reviewing. |
No worries! Most of my comments are minor, but they're mostly there to ensure consistency across different components' READMEs :) |
|
Do we have a style guide for component documentation? It could be useful as a reference. |
|
@ciampo thanks for the feedback! I'll get that addressed today.
This would be extremely useful and take any guesswork out for contributors. |
No, we don't yet — but we're working on updating the contributing guidelines. A documentation style guide may be a nice addition to that document, once the critical sections are addressed. |
* trunk: (214 commits) Fix snackbar overflow on nav editor (#34661) Mobile - Allow disabling text and background color via theme.json (#34633) Fix disabled blocks logical error on Widgets screen (#34634) [Mobile] - Global styles - Add support to render font sizes and line height (#34144) ESLint Plugin: Update eslint jsdoc dependency (#34338) Scripts: Add CHANGELOG entry for `jest-dev-server` upgrade (#34657) Bump jest-dev-server to v5 (#34560) Refactor the `core-data` store to thunks (#28389) Only capture toolbars on parent Nav block when not in vertical mode (#34615) Update Changelog for 11.5.0-rc.1 Bump plugin version to 11.5.0-rc.1 Gallery block: Fix media placeholder height in site editor (#34629) Border Controls: Display color indicator and check selected color (#34467) Remove horizontal and vertical navigation block variations from inserter (#34614) AlignmentMatrixControl : Fix/update docs (#34624) Gap block support: force gap change to cause the block to re-render (fix Safari issue) (#34567) [Block Library - Social Links]: Use the new `flex` layout (#34493) [Mobile] Update the bottom sheet header (#34309) Group block: Add a row variation (#34535) Migrate entities.js to thunks (#34582) ...
Description
This PR updates the documentation for the
AlignmentMatrixControlto include a working example and introduce props definitions as part of the larger effort in #33263How has this been tested?
Docs updates only. I have tested the example in a local env.
Types of changes
Documentation only
Checklist:
*.native.jsfiles for terms that need renaming or removal).