-
Notifications
You must be signed in to change notification settings - Fork 43
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
Query Block #176
Query Block #176
Conversation
…blocks. Implementation of Featured Image toggle to illustrate structure.
… save function. Default for Title block outside of post context.
@thomasguillot We have created a bunch of field blocks to lay out the query, would you be able to take a look at them and see how we can improve the design? |
…r the Title block and remove custom CSS
This works around some weird de-duplication issues, at the expense of making the async run in series instead of parallel.
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've made some comments to align this with Core better.
Great work overall! A lot of this is already ready to be merged into Core, and I would love to see some PRs soon.
import { withDispatch } from '@wordpress/data'; | ||
import { decodeEntities } from '@wordpress/html-entities'; | ||
|
||
class Edit extends 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.
The Post block should be a context provider only.
It should provide a post entity to its inner blocks and use their layout to decide what/how to render the post, similarly to how the Query block works in this PR.
Resolving which post to provide should go as follows:
- Check for fixed post ID attribute.
- Check for context, e.g., Query block parent. We can provide and read from a new query entity to implement this hierarchy.
- Render a post selector in a placeholder.
We already have the base for this in Core, and it would be nice if we continue this work there as both implementations have the same requirements: WordPress/gutenberg#19572.
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.
The Query block would then render a list of Post blocks.
src/blocks/query/edit.js
Outdated
return ( | ||
<article className={ post.id === editingPost ? 'is-editing' : '' } key={ post.id }> | ||
<EntityProvider kind="postType" type="post" id={ post.id }> | ||
<BlockEditorProvider |
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 should avoid breaking up the blocklist like this. It breaks a lot of assumptions the peripheral UI makes about the content area.
A lot of this file would be simplified with an approach like this:
function Component( { setAttributes } ) {
const [ blocks, _onInput, _onChange ] = useEntityBlockEditor(
'postType',
'post'
);
const onInput = useCallback( ( newBlocks ) => {
setAttributes( { blocks: newBlocks } );
_onInput( newBlocks );
}, [] );
const onChange = useCallback( ( newBlocks ) => {
setAttributes( { blocks: newBlocks } );
_onChange( newBlocks );
}, [] );
return (
<InnerBlocks
__experimentalBlocks={ blocks }
onInput={ onInput }
onChange={ onChange }
/>
);
}
</SVG> | ||
); | ||
|
||
const Edit = ( { 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.
We can already work this into Core: WordPress/gutenberg#19576.
</SVG> | ||
); | ||
|
||
const Edit = withSelect( select => { |
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 tags and categories be the same block with different options?
In any case, we should work this into Core: WordPress/gutenberg#19580
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 can see some generic blocks for taxonomy terms and categories, tags, custom taxonomies be configurations of the block that are registered as their own blocks in the inserter. It's more expressive to discover "tags" and "categories" in the block inserter but it's also convenient to have a shared underlying implementation. Not all of this is possible with the current block API.
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.
Like what you proposed for social links?
const [ newspack_author_info ] = useEntityProp( 'postType', 'post', 'categories' ); | ||
const categories = ( allCategories || [] ).filter( c => post.categories.includes( c.id ) ) | ||
return <ul> | ||
{ categories.map( c => <li key={ c.id }><a href={ c.link }>{ c.name }</a></li> ) } |
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.
What is this block for?
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.
This is vestigial and should have been deleted. But I'm working on a new version of this block that would allow folks to display custom fields, which is an important feature for Newspack users.
</SVG> | ||
); | ||
|
||
const Edit = withSelect( select => { |
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.
</SVG> | ||
); | ||
|
||
export const registerTitleBlock = () => registerBlockType( 'newspack-blocks/title', { |
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.
This too.
src/components/query-panel/index.js
Outdated
import { __ } from '@wordpress/i18n'; | ||
import { addQueryArgs } from '@wordpress/url'; | ||
|
||
const fetchAuthorSuggestions = async search => { |
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.
All of these API calls can become core-data
usage.
If an entity is not there already (like user), we can easily add it to the config or load it dynamically in this plugin.
'datetime' => true, | ||
); | ||
|
||
echo wp_kses( render_block( $block_data ), $allowed_html ); |
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.
This will rely heavily on rendering order. What if there is a nested block which sets a different post context. It will also break if rendering is not depth-first, right?
I think the child blocks should use gutenberg_get_post_from_context
, and the query block should use a new gutenberg_set_post_context
so that we can easily make changes to the internals of this in the future.
@epiqueras thank you so much for your review! I've been revising the Homepage Articles block in #289 to use a similar store to this block, and uncovered some of the things you mentioned in your review about API calls. I'll work through the issues you highlighted and then look at getting a core merge going. |
Looking forward to it 😄 |
Contributed to core in WordPress/gutenberg#20106. Closing. |
All Submissions:
Changes proposed in this Pull Request:
Adds a Query block and multiple field blocks
How to test the changes in this Pull Request:
Other information:
TODO