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

Move Posts Per Page, Offset, and Pages controls from the block toolbar into Inspector Controls #58207

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/block-library/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Feature

- Query Loop Block: Moves per page, offset, and pages controls into Inspector Controls. ([#58207](https://github.com/WordPress/gutenberg/pull/58207))

## 9.3.0 (2024-07-10)

## 9.2.0 (2024-06-26)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import { TaxonomyControls } from './taxonomy-controls';
import StickyControl from './sticky-control';
import EnhancedPaginationControl from './enhanced-pagination-control';
import CreateNewPostLink from './create-new-post-link';
import PerPageControl from './per-page-control';
import OffsetControl from './offset-controls';
import PagesControl from './pages-control';
import { unlock } from '../../../lock-unlock';
import {
usePostTypes,
Expand All @@ -48,7 +51,10 @@ export default function QueryInspectorControls( props ) {
order,
orderBy,
author: authorIds,
pages,
postType,
perPage,
offset,
sticky,
inherit,
taxQuery,
Expand Down Expand Up @@ -136,6 +142,16 @@ export default function QueryInspectorControls( props ) {
showParentControl;
const dropdownMenuProps = useToolsPanelDropdownMenuProps();

const showPostCountControl = isControlAllowed(
allowedControls,
'postCount'
);
const showOffSetControl = isControlAllowed( allowedControls, 'offset' );
const showPagesControl = isControlAllowed( allowedControls, 'pages' );

const showDisplayPanel =
showPostCountControl || showOffSetControl || showPagesControl;

return (
<>
{ !! postType && (
Expand Down Expand Up @@ -237,6 +253,47 @@ export default function QueryInspectorControls( props ) {
/>
</PanelBody>
) }
{ ! inherit && showDisplayPanel && (
<ToolsPanel
className="block-library-query-toolspanel__display"
label={ __( 'Display' ) }
resetAll={ () => {
setQuery( {
offset: 0,
pages: 0,
} );
} }
dropdownMenuProps={ dropdownMenuProps }
>
<ToolsPanelItem
label={ __( 'Items' ) }
hasValue={ () => perPage > 0 }
>
<PerPageControl
perPage={ perPage }
offset={ offset }
onChange={ setQuery }
/>
Comment on lines +288 to +292
Copy link
Member

Choose a reason for hiding this comment

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

Nit (non-blocking): There is probably no need to create wrappers for simple components like - PerPageControl, OffsetControl and PagesControl.

This is mostly my personal preference. Feel free to ignore it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with wrappers to reduce the size of the index.js for this section. I guess its number of files vs lines of code. I'm happy either way, but if these components get more complicated, it would be nice to abstract that out of the main file.

</ToolsPanelItem>
<ToolsPanelItem
label={ __( 'Offset' ) }
hasValue={ () => offset > 0 }
onDeselect={ () => setQuery( { offset: 0 } ) }
>
<OffsetControl
offset={ offset }
onChange={ setQuery }
/>
</ToolsPanelItem>
<ToolsPanelItem
label={ __( 'Max Pages to Show' ) }
hasValue={ () => pages > 0 }
onDeselect={ () => setQuery( { pages: 0 } ) }
>
<PagesControl pages={ pages } onChange={ setQuery } />
</ToolsPanelItem>
</ToolsPanel>
) }
{ ! inherit && showFiltersPanel && (
<ToolsPanel
className="block-library-query-toolspanel__filters" // unused but kept for backward compatibility
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* WordPress dependencies
*/
import { __experimentalNumberControl as NumberControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

const MIN_OFFSET = 0;
const MAX_OFFSET = 100;

export const OffsetControl = ( { offset = 0, onChange } ) => {
return (
<NumberControl
label={ __( 'Offset' ) }
value={ offset }
min={ MIN_OFFSET }
onChange={ ( newOffset ) => {
if (
isNaN( newOffset ) ||
newOffset < MIN_OFFSET ||
newOffset > MAX_OFFSET
) {
return;
}
onChange( { offset: newOffset } );
} }
/>
);
};

export default OffsetControl;
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* WordPress dependencies
*/
import { __experimentalNumberControl as NumberControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

export const PagesControl = ( { pages, onChange } ) => {
return (
<NumberControl
label={ __( 'Max Pages to Show' ) }
value={ pages }
min={ 0 }
onChange={ ( newPages ) => {
if ( isNaN( newPages ) || newPages < 0 ) {
return;
}
onChange( { pages: newPages } );
} }
help={ __(
'Limit the pages you want to show, even if the query has more results. To show all pages use 0 (zero).'
) }
/>
);
};

export default PagesControl;
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* WordPress dependencies
*/
import { RangeControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

const MIN_POSTS_PER_PAGE = 1;
const MAX_POSTS_PER_PAGE = 100;

const PerPageControl = ( { perPage, offset = 0, onChange } ) => {
return (
<RangeControl
label={ __( 'Items Per Page' ) }
min={ MIN_POSTS_PER_PAGE }
max={ MAX_POSTS_PER_PAGE }
onChange={ ( newPerPage ) => {
if (
isNaN( newPerPage ) ||
newPerPage < MIN_POSTS_PER_PAGE ||
newPerPage > MAX_POSTS_PER_PAGE
) {
return;
}
onChange( { perPage: newPerPage, offset } );
} }
value={ parseInt( perPage, 10 ) }
Copy link
Member

Choose a reason for hiding this comment

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

I think the perPage value can be null, resulting in NaN.

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 the perPage value can be null, resulting in NaN.

Yes, I noticed in the current implementation on trunk these values do have checks for null and 'out of range' values. But this applies to all the three settings. Not sure why those checks have been removed in this PR but yes they should likely be added back, maybe by using an helper function.

More work for next week 🍹

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added these back now.

/>
);
};

export default PerPageControl;
91 changes: 1 addition & 90 deletions packages/block-library/src/query/edit/query-toolbar.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
/**
* WordPress dependencies
*/
import {
ToolbarGroup,
Dropdown,
ToolbarButton,
__experimentalNumberControl as NumberControl,
} from '@wordpress/components';
import { ToolbarGroup, ToolbarButton } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { settings } from '@wordpress/icons';

/**
* Internal dependencies
*/
import { usePatterns } from '../utils';

export default function QueryToolbar( {
attributes: { query },
setQuery,
openPatternSelectionModal,
name,
clientId,
Expand All @@ -26,87 +18,6 @@ export default function QueryToolbar( {

return (
<>
{ ! query.inherit && (
<ToolbarGroup>
<Dropdown
contentClassName="block-library-query-toolbar__popover"
renderToggle={ ( { onToggle } ) => (
<ToolbarButton
icon={ settings }
label={ __( 'Display settings' ) }
onClick={ onToggle }
/>
) }
renderContent={ () => (
<>
<NumberControl
__unstableInputWidth="60px"
className="block-library-query-toolbar__popover-number-control"
label={ __( 'Items per Page' ) }
labelPosition="edge"
min={ 1 }
max={ 100 }
onChange={ ( value ) => {
if (
isNaN( value ) ||
value < 1 ||
value > 100
) {
return;
}
setQuery( {
perPage: value,
} );
} }
step="1"
value={ query.perPage }
isDragEnabled={ false }
/>
<NumberControl
__unstableInputWidth="60px"
className="block-library-query-toolbar__popover-number-control"
label={ __( 'Offset' ) }
labelPosition="edge"
min={ 0 }
max={ 100 }
onChange={ ( value ) => {
if (
isNaN( value ) ||
value < 0 ||
value > 100
) {
return;
}
setQuery( { offset: value } );
} }
step="1"
value={ query.offset }
isDragEnabled={ false }
/>
<NumberControl
__unstableInputWidth="60px"
className="block-library-query-toolbar__popover-number-control"
label={ __( 'Max pages to show' ) }
labelPosition="edge"
min={ 0 }
onChange={ ( value ) => {
if ( isNaN( value ) || value < 0 ) {
return;
}
setQuery( { pages: value } );
} }
step="1"
value={ query.pages }
isDragEnabled={ false }
help={ __(
'Limit the pages you want to show, even if the query has more results. To show all pages use 0 (zero).'
) }
/>
</>
) }
/>
</ToolbarGroup>
) }
{ hasPatterns && (
<ToolbarGroup className="wp-block-template-part__block-control-group">
<ToolbarButton onClick={ openPatternSelectionModal }>
Expand Down
28 changes: 28 additions & 0 deletions packages/e2e-tests/plugins/observe-typing.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* Plugin Name: Gutenberg Test Observe Typing
* Plugin URI: https://github.com/WordPress/gutenberg
* Author: Gutenberg Team
*
* @package gutenberg-test-block-api
*/

/**
* Registers a custom script for the plugin.
*/
function enqueue_observe_typing_plugin_script() {
wp_enqueue_script(
'gutenberg-test-observe-typing',
plugins_url( 'observe-typing/index.js', __FILE__ ),
array(
'wp-blocks',
'wp-block-editor',
'wp-components',
'wp-element'

Check failure on line 21 in packages/e2e-tests/plugins/observe-typing.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

There should be a comma after the last array item in a multi-line array.
),
filemtime( plugin_dir_path( __FILE__ ) . 'observe-typing/index.js' ),
true
);
}

add_action( 'init', 'enqueue_observe_typing_plugin_script' );
45 changes: 45 additions & 0 deletions packages/e2e-tests/plugins/observe-typing/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
( function () {
const { registerBlockType } = wp.blocks;
const { useBlockProps, BlockControls } = wp.blockEditor;
const { Dropdown, ToolbarButton, TextControl } = wp.components;
const { createElement: el, useState } = wp.element;

registerBlockType( 'e2e-tests/observe-typing', {
apiVersion: 3,
title: 'Observe Typing',
description: 'Observe Typing test block.',
category: 'widgets',
edit: function Edit() {
const [ value, setValue ] = useState( '' );
const blockProps = useBlockProps();

return el(
'div',
blockProps,
el(
BlockControls,
{ group: 'block' },
el( Dropdown, {
renderToggle: ( { onToggle } ) =>
el(
ToolbarButton,
{
onClick: onToggle,
},
'Open Dropdown'
),
renderContent: () =>
el( TextControl, {
label: 'Dropdown field',
value,
onChange: setValue,
__next40pxDefaultSize: true,
} ),
} )
),
el( 'p', {}, 'Hello Editor!' )
);
},
save: () => null,
} );
} )();
Loading
Loading