Adding core search widget block by bamadesigner · Pull Request #4501 · WordPress/gutenberg

bamadesigner · 2018-01-16T07:23:33Z

Description

Adding new search widget block. For #1800

Some thoughts/questions for discussion:

How much do we want this to work with get_search_form()?
I copy/pasted form markup from get_search_form() because I added 3 fields so users can customize search label, placeholder, and submit button value but this markup isn't editable in get_search_form() at the moment.
These fields could be an issue with current theme architecture searchform.php, e.g. need to get the search block attributes to the template files.
get_search_form() and its markup used here needs an accessibility check.
I don't think the form label (taken from get_search_form()) should be instantly classified as "screen-reader-text". That should be up to the theme on whether or not to hide it from display. I changed class to "search-label" in the admin so it would display.
@melchoyce mentioned in the issue to have the search label be set to "Search" but that doesnt match get_search_form() that uses "Search for:".
The screenshot in the issue lists the default placeholder text as "Enter a search keyword or phrase" but the placeholder in get_search_form() is "Search …".
I added a "get_search_form_args" filter. Really just trying to get in the mindset of updating get_search_form() to be used with the block.
I just put in some basic styles for the form in the editor to get started

How Has This Been Tested?

Tested in local environment with Twenty Seventeen theme. Had to remove/rename searchform.php template to test front-end search for markup.

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows has proper inline documentation.

melchoyce · 2018-01-16T15:20:29Z

@melchoyce mentioned in the issue to have the search label be set to "Search" but that doesnt match get_search_form() that uses "Search for:".
The screenshot in the issue lists the default placeholder text as "Enter a search keyword or phrase" but the placeholder in get_search_form() is "Search …".

Ah, then let's go with the default values for these, seems easier 👍

ghost · 2018-01-16T16:08:20Z

How much do we want this to work with get_search_form()?

Just make sure the code can be overridden by or handle integration to

a dedicated search engine, like Elasticsearch

as advised in WordPress search, unexpected results due to Gutenberg serialization markup #3739 as solution for a working search.

westonruter

It's a difficult question on whether to try to re-use get_search_form(). I think the most important thing is that the preview be faithful to the frontend. This is currently a challenge in Gutenberg for widgets because they have separate implementations between editing (JS) and viewing (PHP). A theme can override the PHP rendering to be something completely different already, which would break the WYSIWYG block editing preview. But this would be less likely than the theme defining a searchform.php. Since the block here is not going to be used in normal theme template contexts but rather in new Gutenberg contexts, I'm inclined to think that the search block you're defining here should break away from trying to re-use searchform.php to instead use a new Gutenberg block template, one that leverages all of the new customizability you are introducing. After all, the Latest Posts widget block is not re-using the Recent Posts widget template.

In the Gutenberg world, theme templates are going to need to be rethought anyway in favor of a block templating system, I think. (Especially to eliminate duplicating templates across PHP and JS.) So I don't think we should try too hard here to re-use existing WP theme templates.

westonruter · 2018-02-20T06:24:08Z

blocks/library/search/index.php

+		$defaults = array(
+			'label'       => _x( 'Search for:', 'label' ),
+			'placeholder' => esc_attr_x( 'Search &hellip;', 'placeholder' ),
+			'submitValue' => esc_attr_x( 'Search', 'submit button' ),


These strings should have the gutenberg text domain.

westonruter · 2018-02-20T06:28:14Z

blocks/library/search/index.php

+	 * @since 2.7.0
+	 *
+	 * @param string $form The search form HTML output.
+	 */


Since this is copied from core, this phpdoc can be reduced to:

/** This filter is documented in wp-includes/general-template.php */

westonruter · 2018-02-20T06:30:50Z

blocks/library/search/index.js

+	submitValue: {
+		type: 'string',
+		default: _x( 'Search', 'submit button' ),
+	},


The strings here are duplicated here in JS and in PHP. It would be ideal if they were all consolidated in the one register_block_type() PHP call and then exported to JS.

Yes, attributes should be defined on the server using PHP. It will solve the issues with code duplication. See Latest Posts block for reference:

gutenberg/packages/block-library/src/latest-posts/index.php

Lines 88 to 130 in e8f0f89

function register_block_core_latest_posts() {

register_block_type(

'core/latest-posts',

array(

'attributes' => array(

'categories' => array(

'type' => 'string',

),

'className' => array(

'type' => 'string',

),

'postsToShow' => array(

'type' => 'number',

'default' => 5,

),

'displayPostDate' => array(

'type' => 'boolean',

'default' => false,

),

'postLayout' => array(

'type' => 'string',

'default' => 'list',

),

'columns' => array(

'type' => 'number',

'default' => 3,

),

'align' => array(

'type' => 'string',

),

'order' => array(

'type' => 'string',

'default' => 'desc',

),

'orderBy' => array(

'type' => 'string',

'default' => 'date',

),

),

'render_callback' => 'render_block_core_latest_posts',

)

);

}

westonruter · 2018-02-20T06:31:40Z

blocks/library/search/index.php

+		$form = ob_get_clean();
+	} else {
+
+		$defaults = array(


These defaults are duplicated. It would be better of the defaults could be pulled from the PHP-registered block's attributes, per my note above.

westonruter · 2018-02-20T06:32:04Z

blocks/library/search/index.php

+		$form = '<form role="search" method="get" class="search-form" action="' . esc_url( home_url( '/' ) ) . '">
+			<label>
+				<span class="screen-reader-text">' . $args['label'] . '</span>
+				<input type="search" class="search-field" placeholder="' . $args['placeholder'] . '" value="' . get_search_query() . '" name="s" />


The attribute values here and below should be wrapped in esc_attr()

Updating from master

danielbachhuber · 2018-06-15T12:17:33Z

Closing this for now, per #1800 (comment)

This seems like Customization territory, not 5.0.

gziolo · 2019-01-21T10:18:29Z

@bamadesigner Can you rebase it again?
The Gutenberg phase 2 with Widgets to blocks has started and there are volunteers to help with reviews of this PR: https://make.wordpress.org/core/2019/01/17/editor-chat-summary-january-16/.

noisysocks · 2019-01-24T05:46:16Z

Spoke to @bamadesigner who says she's too swamped right now to continue with this. I'll be taking over and will try and get it in for Gutenberg 5.1 🙂

gziolo

This PR needs to be rebased. Blocks were moved to packages/block-library/src/ so it might require some manual work.

I left my feedback which I hope will help to get this PR up to date and some notes how the comment from @westonruter can be addressed.

gziolo · 2019-01-24T10:28:10Z

blocks/library/search/index.js

+/**
+ * Internal dependencies
+ */
+ import './editor.scss';


We no longer import SCSS files in JavaScript. There is editor.scss file where this import should be added instead:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/editor.scss

gziolo · 2019-01-24T10:28:46Z

blocks/library/search/index.js

+ import './editor.scss';
+import { registerBlockType } from '../../api';
+import InspectorControls from '../../inspector-controls';
+import TextControl from '../../inspector-controls/text-control';


All three imports should be converted to WordPress dependencies.

gziolo · 2019-01-24T10:30:27Z

blocks/library/search/index.js

+				</label>
+				<input type="submit" className="search-submit" value={ submitValue } />
+			</form>,
+			focus && (


focus was removed and we no longer need it.

render method might return Fragment component rather than array which needs to assign key to every child element. form doesn't have key provided so it will print warnings on the console.

gziolo · 2019-01-24T10:32:41Z

blocks/library/search/index.js

+import InspectorControls from '../../inspector-controls';
+import TextControl from '../../inspector-controls/text-control';
+
+class SearchBlock extends Component {


SearchBlock component should be moved to its own edit.js file so it could be overridden by the mobile app when necessary.

gziolo · 2019-01-24T10:36:11Z

blocks/library/search/index.js

+	},
+};
+
+registerBlockType( 'core/search', {


We no longer register block types. Instead, we export name and settings. See RSS block implementation for reference:
https://github.com/WordPress/gutenberg/blob/e8f0f89c5a954d4d67c3bfac1cfa9ef0edd082ed/packages/block-library/src/rss/index.js
and then it's imported and added in the file which contain registerCoreBlocks method:

gutenberg/packages/block-library/src/index.js

Line 89 in e8f0f89

rss,

gziolo · 2019-01-24T10:37:18Z

blocks/library/search/index.php

+/**
+ * Server-side rendering of the `core/search` block.
+ *
+ * @package gutenberg


This file is going to be copied to WordPress so it should be WordPress. See another block for reference:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/categories/index.php#L5

gziolo · 2019-01-24T10:39:14Z

blocks/library/search/index.js

+	submitValue: {
+		type: 'string',
+		default: _x( 'Search', 'submit button' ),
+	},


Yes, attributes should be defined on the server using PHP. It will solve the issues with code duplication. See Latest Posts block for reference:

gutenberg/packages/block-library/src/latest-posts/index.php

Lines 88 to 130 in e8f0f89

function register_block_core_latest_posts() {

register_block_type(

'core/latest-posts',

array(

'attributes' => array(

'categories' => array(

'type' => 'string',

),

'className' => array(

'type' => 'string',

),

'postsToShow' => array(

'type' => 'number',

'default' => 5,

),

'displayPostDate' => array(

'type' => 'boolean',

'default' => false,

),

'postLayout' => array(

'type' => 'string',

'default' => 'list',

),

'columns' => array(

'type' => 'number',

'default' => 3,

),

'align' => array(

'type' => 'string',

),

'order' => array(

'type' => 'string',

'default' => 'desc',

),

'orderBy' => array(

'type' => 'string',

'default' => 'date',

),

),

'render_callback' => 'render_block_core_latest_posts',

)

);

}

gziolo · 2019-01-24T10:41:15Z

blocks/library/search/index.php

+
+		$defaults = array(
+			'label'       => _x( 'Search for:', 'label', 'gutenberg' ),
+			'placeholder' => esc_attr_x( 'Search &hellip;', 'placeholder', 'gutenberg' ),


Translations shouldn't use the domain param as this file is going to be copied to core.

noisysocks · 2019-01-30T05:47:00Z

I've picked this up over in #13583. Thanks again @bamadesigner for all your fantastic work on this!

Adding search widget block

a4b1b7e

melchoyce requested review from a team and westonruter February 10, 2018 19:01

westonruter requested changes Feb 20, 2018

View reviewed changes

bamadesigner added 6 commits March 1, 2018 16:59

Merge pull request #1 from WordPress/master

8179bc8

Updating from master

Merge branch 'master' into add/search-block

d8c503a

Adding gutenberg text domain

4a75f21

Adding context to _x()

fe21250

Adding esc_attr() where needed

02a17c5

Optimizing a filter phpdoc

f0e10bf

danielbachhuber closed this Jun 15, 2018

danielbachhuber mentioned this pull request Jun 15, 2018

Add Block: Search #1800

Closed

melchoyce reopened this Dec 14, 2018

gziolo added Needs Accessibility Feedback Need input from accessibility New Block Suggestion for a new block [Feature] Blocks Overall functionality of blocks labels Jan 21, 2019

gziolo requested review from Soean, ajitbohra and gziolo January 21, 2019 09:56

gziolo assigned bamadesigner Jan 21, 2019

gziolo added this to the 5.1 (Gutenberg) milestone Jan 22, 2019

youknowriad mentioned this pull request Jan 22, 2019

Phase 2 Scope & Features #13113

Closed

22 tasks

noisysocks self-requested a review January 23, 2019 03:05

gziolo requested changes Jan 24, 2019

View reviewed changes

melchoyce mentioned this pull request Jan 28, 2019

Add Block: Calendar #1462

Closed

noisysocks mentioned this pull request Jan 30, 2019

Add search block #13583

Merged

noisysocks closed this Jan 30, 2019

afercia added the [Feature] Widgets Screen The block-based screen that replaced widgets.php. label Jan 30, 2019

	function register_block_core_latest_posts() {
	register_block_type(
	'core/latest-posts',
	array(
	'attributes' => array(
	'categories' => array(
	'type' => 'string',
	),
	'className' => array(
	'type' => 'string',
	),
	'postsToShow' => array(
	'type' => 'number',
	'default' => 5,
	),
	'displayPostDate' => array(
	'type' => 'boolean',
	'default' => false,
	),
	'postLayout' => array(
	'type' => 'string',
	'default' => 'list',
	),
	'columns' => array(
	'type' => 'number',
	'default' => 3,
	),
	'align' => array(
	'type' => 'string',
	),
	'order' => array(
	'type' => 'string',
	'default' => 'desc',
	),
	'orderBy' => array(
	'type' => 'string',
	'default' => 'date',
	),
	),
	'render_callback' => 'render_block_core_latest_posts',
	)
	);
	}

Conversation

bamadesigner commented Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

Uh oh!

melchoyce commented Jan 16, 2018

Uh oh!

ghost commented Jan 16, 2018

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielbachhuber commented Jun 15, 2018

Uh oh!

gziolo commented Jan 21, 2019

Uh oh!

noisysocks commented Jan 24, 2019

Uh oh!

gziolo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisysocks commented Jan 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bamadesigner commented Jan 16, 2018 •

edited

Loading