-
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
Add components examples in README.md files - Part 2 #8338
Conversation
I don't think we should expose |
@mmtr - whatever works in terms of the name. My only concern would be the need to document it somewhere so we could point contributors to a link which explains how examples should be structured to ensure we can consume them from docs. ServerSideRenderer probably should be moved to the editor or standalone package, @youknowriad any thoughts on that? This is similar to CodeEditor tied to WordPress instance. |
@gziolo this should be ready to review 🎉 |
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.
I had only one comment. I haven't tested code examples as it would be too time-consuming. Let's move forward and open a follow-up when we discover any issues. Those are only docs after all 😃
I had one question where I think we should skip providing props to the example component.
|
||
return <div>{ post.data.title.rendered }</div>; | ||
class Context extends React.Component { |
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.
FYI: We are deprecating this one: #8584.
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! I won't include it then in Calypso
@@ -7,15 +7,38 @@ | |||
Wrap your original component with `withContext`, defining a key of context to receive and an optional mapping function. | |||
|
|||
```jsx | |||
function OriginalComponent( { favoriteColor } ) { | |||
return <div>Your favorite color is: { favoriteColor }</div>; | |||
import PropTypes from 'prop-types'; |
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.
We are deprecating this one, too: #8189.
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.
one example less to include in Calypso
import { Button, Modal } from '@wordpress/components'; | ||
import { withState } from '@wordpress/compose'; | ||
|
||
const MyModal = withState( { |
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.
Much shorter 👍
@@ -14,10 +13,14 @@ New blocks should be built in conjunction with any necessary REST API endpoints, | |||
Render core/archives preview. | |||
|
|||
```jsx | |||
import { ServerSideRender } from '@wordpress/components'; | |||
|
|||
const MyServerSideRender = ( { attributes } ) => ( |
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.
Should it provide some dummy attributes
to make it work without any attributes provided?
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.
yep, it makes sense. I forgot this one because it's not being included in Calypso.
This continues #8267
The aim of this PR is to include working examples in the components documentation.
Components (Part 2)
ServerSideRender