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.

Sign up for GitHub

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 basic pages block #28265

Merged
merged 16 commits into from
Feb 8, 2021
Merged

Add basic pages block #28265

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
70d0e6b
Add basic pages block
tellthemachines Jan 18, 2021
d6e8af6
Add dropdowns and indicators
tellthemachines Jan 19, 2021
2e4190e
Inherit settings from parent Nav block
tellthemachines Jan 21, 2021
66eb631
Add pages button adds the block and styling improvements
tellthemachines Jan 22, 2021
1335523
Change name to page list
tellthemachines Jan 27, 2021
16acfda
Add pages icon
tellthemachines Jan 27, 2021
109433a
Add page list fixture
tellthemachines Jan 28, 2021
a47670a
Update nav screen snapshot
tellthemachines Jan 28, 2021
f99a55e
Fix placeholder and dropdown background
tellthemachines Jan 29, 2021
1c59e8f
Use context and add keywords
tellthemachines Jan 29, 2021
b5bf865
Fix clickability and remove extra classname.
tellthemachines Feb 1, 2021
7544616
Test presence of pages block items
tellthemachines Feb 1, 2021
02ce0c0
Replace walker with manual iteration
tellthemachines Feb 3, 2021
15e8a89
Fix tests
tellthemachines Feb 3, 2021
a2ee259
Address review feedback
tellthemachines Feb 5, 2021
014b7c1
Order menu items and de-indent comment.
tellthemachines Feb 8, 2021
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
1 change: 1 addition & 0 deletions lib/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ function gutenberg_reregister_core_block_types() {
'shortcode.php' => 'core/shortcode',
'social-link.php' => 'core/social-link',
'tag-cloud.php' => 'core/tag-cloud',
'page-list.php' => 'core/page-list',
'post-author.php' => 'core/post-author',
'post-comment.php' => 'core/post-comment',
'post-comment-author.php' => 'core/post-comment-author',
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
@import "./navigation/editor.scss";
@import "./navigation-link/editor.scss";
@import "./nextpage/editor.scss";
@import "./page-list/editor.scss";
@import "./paragraph/editor.scss";
@import "./post-content/editor.scss";
@import "./post-excerpt/editor.scss";
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import * as list from './list';
import * as missing from './missing';
import * as more from './more';
import * as nextpage from './nextpage';
import * as pageList from './page-list';
import * as preformatted from './preformatted';
import * as pullquote from './pullquote';
import * as reusableBlock from './block';
Expand Down Expand Up @@ -148,6 +149,7 @@ export const __experimentalGetCoreBlocks = () => [
missing,
more,
nextpage,
pageList,
talldan marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@youknowriad youknowriad Feb 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

So this block is stable and independent of the navigation block, which means it's going to be included in the next WP major (5.8) regardless of the status of the Navigation block, I just want to make sure this is a conscious decision.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tellthemachines was the block meant to be tweaked with regards to syntax, or is it pretty much baked as is?

Copy link
Member

Choose a reason for hiding this comment

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

It seems we should keep this tied to the navigation block (and its child blocks) for a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In making this an independent block, I was thinking of the Widgets editor, as we don't currently have another block that can reproduce the Pages widget functionality. Assuming we want the Widgets editor to have feature parity with the legacy widgets screen, this block will be needed.

I'm happy with its current syntax, and the fact that it gets whatever styling properties from Navigation block context means that when used standalone, it works just like the Categories or the Archives block, that don't have any styling options.

Related question: should Categories also work as a child of Navigation? There's definitely a use case for categories in menus.

preformatted,
pullquote,
rss,
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/navigation/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ function Navigation( {
'core/navigation-link',
'core/search',
'core/social-links',
'core/page-list',
],
orientation: attributes.orientation || 'horizontal',
renderAppender:
Expand Down
27 changes: 2 additions & 25 deletions packages/block-library/src/navigation/placeholder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,29 +97,6 @@ function convertMenuItemsToBlocks( menuItems ) {
return mapMenuItemsToBlocks( menuTree );
}

/**
* Convert pages to blocks.
*
* @param {Object[]} pages An array of pages.
*
* @return {WPBlock[]} An array of blocks.
*/
function convertPagesToBlocks( pages ) {
if ( ! pages ) {
return null;
}

return pages.map( ( { title, type, link: url, id } ) =>
createBlock( 'core/navigation-link', {
type,
id,
url,
label: ! title.rendered ? __( '(no title)' ) : title.rendered,
opensInNewTab: false,
} )
);
}

function NavigationPlaceholder( { onCreate }, ref ) {
const [ selectedMenu, setSelectedMenu ] = useState();

Expand Down Expand Up @@ -220,9 +197,9 @@ function NavigationPlaceholder( { onCreate }, ref ) {
};

const onCreateAllPages = () => {
const blocks = convertPagesToBlocks( pages );
const block = [ createBlock( 'core/page-list' ) ];
const selectNavigationBlock = true;
onCreate( blocks, selectNavigationBlock );
onCreate( block, selectNavigationBlock );
};

useEffect( () => {
Expand Down
20 changes: 20 additions & 0 deletions packages/block-library/src/page-list/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"apiVersion": 2,
"name": "core/page-list",
"category": "widgets",
"usesContext": [
"textColor",
"customTextColor",
"backgroundColor",
"customBackgroundColor",
"fontSize",
"customFontSize",
"showSubmenuIcon"
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like "context" is being abused in the navigation block, what happens if I use this block independently and I want to customize these things?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm only four months late to this PR, but it sounds like we should try to let Global Styles absorb some of this (cc @nosolosw, @mtias).

Copy link
Member

Choose a reason for hiding this comment

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

Looked a bit at this. It relates to the block support mechanism and parent/child relationship.

Braindump of what we have now:

  • a block wants to serialize styles to its wrapper => default supports mechanism
  • a block wants to serialize styles to any other node => support mechanism with skip serialization (but only for its own markup, can't affect children)
  • a block wants to serialize styles to children blocks or or a children wants to take styles from the parent => block context

One thing we could explore would be to allow blocks to automatically expose some block supports via context without having to declare context support explicitely. Was that what you had in mind, Miguel?

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 added the context in to match the Navigation Link block, so that all the links in the Navigation could be styled equally. When Page List is used as an independent block, those styling options aren't available. This seemed ok as the standalone block reproduces the functionality of the Pages widget, and similar blocks like Latest Posts also have limited styling.
As for the styling mechanism used I think we should go with the same strategy for both Navigation Link and Page List so that they work consistently inside the Navigation block, but I'm not super knowledgeable about how Global Styles work so not sure what it might look like!

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing we could explore would be to allow blocks to automatically expose some block supports via context without having to declare context support explicitely. Was that what you had in mind, Miguel?

Actually, I'm much less concerned about explicitly including Global Styles / Block Supports keys under usesContext, than I am about manually replicating some of gutenberg_apply_colors_support as part of render_block_core_page_list (see block_core_page_list_build_css_colors). This is where I think it'd be best to see where, if anywhere, GS/BS could improve to accommodate scenarios like Page List.

Copy link
Member

Choose a reason for hiding this comment

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

In playing with this code I've noticed the block attributes and the block context have the same shape, so we can apply the same transformations on that data to create classes and styles ― it's a matter of finding how.

A small step in making both cases more similar could be #32807 (experimented only with colors to see how it feels).

],
"supports": {
"reusable": false,
"html": false
},
"editorStyle": "wp-block-page-list-editor",
"style": "wp-block-page-list"
}
31 changes: 31 additions & 0 deletions packages/block-library/src/page-list/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/

import { useBlockProps } from '@wordpress/block-editor';
import ServerSideRender from '@wordpress/server-side-render';

export default function PageListEdit( { context } ) {
const { textColor, backgroundColor, showSubmenuIcon } = context || {};

const blockProps = useBlockProps( {
className: classnames( {
'has-text-color': !! textColor,
[ `has-${ textColor }-color` ]: !! textColor,
'has-background': !! backgroundColor,
[ `has-${ backgroundColor }-background-color` ]: !! backgroundColor,
'show-submenu-icons': !! showSubmenuIcon,
} ),
} );

return (
<div { ...blockProps }>
<ServerSideRender block="core/page-list" />
</div>
);
}
29 changes: 29 additions & 0 deletions packages/block-library/src/page-list/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
.wp-block-navigation {
// Block wrapper gets the classes in the editor, and there's an extra div wrapper for now, so background styles need to be inherited.
.wp-block-page-list > div,
.wp-block-page-list {
background-color: inherit;
}
// Make the dropdown background white if there's no background color set.
&:not(.has-background) {
.submenu-container {
color: $gray-900;
background-color: $white;
}
}
}

// Make links unclickable in the editor
.wp-block-pages-list__item__link {
pointer-events: none;
}

.wp-block-page-list .components-placeholder {
min-height: 0;
padding: 0;
background-color: inherit;

.components-spinner {
margin-top: 0;
}
}
24 changes: 24 additions & 0 deletions packages/block-library/src/page-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* WordPress dependencies
*/
import { pages as icon } from '@wordpress/icons';
import { __, _x } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import metadata from './block.json';
import edit from './edit.js';

const { name } = metadata;

export { metadata, name };

export const settings = {
title: _x( 'Page List', 'block title' ),
description: __( 'Display a list of all pages.' ),
keywords: [ __( 'menu' ), __( 'navigation' ) ],
icon,
example: {},
talldan marked this conversation as resolved.
Show resolved Hide resolved
edit,
};
talldan marked this conversation as resolved.
Show resolved Hide resolved
Loading