Query Title: add Search and 404 page titles #33515

mikachan · 2021-07-16T16:32:07Z

Description

This PR addresses #33476 and is predominantly based on the work in #27989.

The change re-introduces a search title to the Query Title block from #27989 and adds a new 404 title to the same block. Both use the _x() function to provide translatable titles: "Search results for %search%" is used for the search title and "Nothing found" is used for the 404 title.

I've tried to be as mindful as possible of the previous discussion in #27989 and cherry-picked the search title changes in line with the current archive title, but I'd love to get some ideas on any other approaches or thoughts.

How has this been tested?

In emptytheme and Blockbase by using the editor to add Query Title blocks to the search, 404, archive, and index templates.

Screenshots

Types of changes

Non-breaking change which adds functionality.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

# Conflicts: # lib/blocks.php # packages/block-library/src/index.js # packages/block-library/src/query-title/block.json # packages/block-library/src/query-title/edit.js # packages/block-library/src/query-title/index.js # packages/block-library/src/query-title/index.php # packages/block-library/src/query-title/variations.js

Based on original work in WordPress#27989

Merged in by mistake

github-actions · 2021-07-16T16:32:30Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @mikachan! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

mtias · 2021-07-19T11:18:14Z

packages/block-library/src/query-title/variations.js

+	},
+	{
+		name: 'search-title',
+		title: __( 'Search Title' ),


Do these need to be variations or could we just infer from context what to display? As a template author, what happens if I only have index.html with archive title added?

If we're able to infer what to display from the page context that would work better I think. This would be much quicker for creating and editing templates. I'll look into this, thanks!

scruffian · 2021-07-19T11:30:17Z

packages/block-library/src/query-title/edit.js

+	} else {
+		titleElement = (
+			<div { ...blockProps }>
+				<RichText


Is there a reason to use RichText here rather than an H* element like we do for the Archive title?

I'm not sure actually, I pulled this from the previous work on the search title in #27989. I'm guessing the intent was to use the additional functionality you get from RichText, but using the H* / TagName would be easier to read and maintain because there are fewer props. I'll try it out with just the H* tag.

RichText should be used if text is meant to be editable

ntsekouras · 2021-07-19T11:39:17Z

👋 - thanks for working on this!

Why would we need a 404 title? This should be handled in 404 template where we can add whatever blocks we want.

scruffian · 2021-07-19T11:54:03Z

Why would we need a 404 title? This should be handled in 404 template where we can add whatever blocks we want.

The main benefit is for i18n. If we just add a title that says "Nothing found" then it won't be translated, whereas a block like this will be.

ntsekouras

Thanks for working on this @mikachan !

I've left some comments and in general I'm not sure what we gain by not using block variations. Is it mostly because we want users to insert just one block (Query Title) in archive, search, and 404 ? What are other benefits and how they are compared to block variations benefits/restrictions.

In addition if we go this way, we should add a deprecation for this block (removal of type attribute).

ntsekouras · 2021-07-20T11:56:34Z

packages/block-library/src/query-title/index.php

 	if ( $is_archive ) {
 		$title = get_the_archive_title();
 	}
+	if ( $is_search ) {


Nitpick: we could make this elseif.

ntsekouras · 2021-07-20T12:18:22Z

packages/block-library/src/query-title/edit.js

+			);
+
+			// Use placeholder content if new Query Title block is added
+			if ( content === '' ) {


We shouldn't add the placeholder to content here as the user will not be able to clear the input and write something new.

Ah yeah I can see this problem when I try to clear the input. If I use a non-empty default placeholder such as 'Query title' rather than checking for an empty string, it looks like I can clear the input safely from the editor.

I'm not sure this is the correct fix though, as this logic was mainly because my use of useEffect was overwriting the custom content when a new query title block was added. I'll try and fix this as part of a useEffect refactor.

ntsekouras · 2021-07-20T12:19:39Z

packages/block-library/src/query-title/edit.js

+		case 'archive':
+			// translators: Title for archive template.
+			content = _x( 'Archive title', 'archive template title' );
+			titleElement = <TagName { ...blockProps }>{ content }</TagName>;


content is not used in archive so there is no reason to assign it to content.
It should be <TagName { ...blockProps }>{ _x( 'Archive title', 'archive template title' ) }</TagName>;

ntsekouras · 2021-07-20T12:22:38Z

packages/block-library/src/query-title/edit.js

+		case '404':
+			// translators: Title for 404 template.
+			content = _x( 'Nothing found', '404 template title' );
+			titleElement = <TagName { ...blockProps }>{ content }</TagName>;


IMO this should be editable. I don't think every user should want Nothing found as a title without the ability to change it.

If they want to edit it can't they just add their own heading block?

ntsekouras · 2021-07-20T12:34:48Z

packages/block-library/src/query-title/edit.js

+	}
+
+	// Update content based on current template
+	useEffect( () => {


This seems hacky to me 😄 . We use React's implementation details to update the content after the first render and by updating the content attribute, we also create an undo level in history. That's one of the reasons I had block variations in mind back then. The php handling remains the same, the above content attribute handling is done by the variations and we can provide a bit more specific info for the block (BlockCard etc..).

An alternative, if this approach is preferred, is to use __unstableMarkNextChangeAsNotPersistent here and a new variable for the first content (to avoid changing the existing content var).

Haha I wasn't sure where else to turn so I jumped to useEffect! Thanks so much for the detailed explanation.

As soon as I moved this logic from the variations file to edit.js, I could see the advantage of using variations. However, as @scruffian has just mentioned above, I think the advantage to not using variations is that the user (or a theme) only needs to insert one block, and the block works the content out itself based on context (i.e. the template).

With that in mind, I'm going to attempt your suggested alternative. Thanks for the suggestion and input, it's really helped me understand this more.

Content is not used in archive

This allows the user to delete the block content and write something new

scruffian · 2021-07-20T13:45:00Z

@ntsekouras

Is it mostly because we want users to insert just one block (Query Title) in archive, search, and 404

The benefit I see is it means a simple theme can just have one index.html and the block will handle all scenarios...

ntsekouras · 2021-07-20T14:08:31Z

The benefit I see is it means a simple theme can just have one index.html and the block will handle all scenarios...

I can't see this unless I'm missing something.

If you have only an index.html and add the Query Title block, it will not match any of the supported [ 'archive', 'search', '404' ] based on templateSlug. Right now it will show a placeholder of Template is not supported and the content will be the default content from block.json.

So in cases like search where the content attribute is used, it will have the default value.

mikachan · 2021-07-21T16:01:41Z

If you have only an index.html and add the Query Title block, it will not match any of the supported [ 'archive', 'search', '404' ] based on templateSlug.

Yeah I see what you mean, the way I've currently structured this means that this block still wouldn't work by itself on a single index.html template. Even if index is added to the supported templates, it will currently show the default title which isn't based on any meaningful context. I think it's still cool that we could have one block that can base its content on the templateSlug, but that doesn't address being able to have a simple theme with a single index template.

I'm not sure how (or the best way) to achieve this... I think one of the biggest things I'm struggling with for the single index template approach is that the editor has no context of the page query, as its purpose is a default 'catch-all' template. For example, if we want the search title to be editable, we may need to go back to having variations in order to show the search content, as I don't think there's any other way to provide the 'search' context on the index template. But then, with this approach, would we then expect users to add multiple title blocks for each variation (assuming only one would display on the front end based on context)?

Another approach I've just been trying out is allowing users to set the title content from the Inspector Controls in the right sidebar. With this, I'm thinking the block would show placeholder text (similar to how the archive title currently works), and then the user could customise the title variation contents in the sidebar, with something like this:

My latest commits are for this approach, although not fully tested yet, it's more of a proof of concept at the moment.

Any thoughts on this would be appreciated, and apologies for any lack of understanding on my part!

scruffian · 2021-07-22T08:17:48Z

The way I see if, if the block is used on a page on which it doesn't have any default content then it would simply output nothing. That's what I added the early return and changed the default content to an empty string.

I'm not a fan of the idea of putting the content in the sidebar as we want the default experience to be one of direct manipulation.

swissspidy · 2021-07-22T08:44:22Z

packages/block-library/src/query-title/block.json

-		"type": {
+		"content": {
+			"type": "string",
+			"default": "Query title placeholder"


I'd avoid non-translatable default values in block.json attributes.

Put the default text in the edit component instead and make it translatable.

swissspidy · 2021-07-22T08:44:58Z

packages/block-library/src/query-title/index.php

 	}
 	$tag_name           = isset( $attributes['level'] ) ? 'h' . (int) $attributes['level'] : 'h1';
 	$align_class_name   = empty( $attributes['textAlign'] ) ? '' : "has-text-align-{$attributes['textAlign']}";
 	$wrapper_attributes = get_block_wrapper_attributes( array( 'class' => $align_class_name ) );
+	if ( 'Query title placeholder' === $title || empty( $title ) ) {


As per my other comment, this should be translatable

Now defaults to empty string

Allow search and 404 titles to be edited via usual block editor

Content attribute no longer exists

mikachan · 2021-07-22T12:45:41Z

I'm not a fan of the idea of putting the content in the sidebar as we want the default experience to be one of direct manipulation.

Yeah, I agree. It's a bit of an odd pattern as well as I'm guessing we'd never really want users editing content via the settings controls. I tried this as a way to allow the user to edit multiple types of title context at once, but I think maybe I'm over-complicating things...

I've just pushed some changes that go back to using the editor as usual, with a couple of tweaks:

If the block is added to the index template it will show a non-editable 'query title' placeholder in the editor. On the front end, it will switch to the pre-defined search, 404 or archive titles based on the query context.
If the block is added to the search or 404 template, it will show the corresponding default title and allow the user to edit it.
If the block is added to the archive template, it will show the archive placeholder in the editor and insert an archive title based on get_the_archive_title() on the front end (this is the existing functionality)

My aim here was to capture both allowing the user to edit the search and 404 titles when they're added to specific templates, but if not, still allow for a context-based title if the block is added to the index template. This means the title isn't editable via the editor when used on the index template, but this does allow for quick template/theme creation with context-based, translatable titles.

scruffian · 2022-07-14T12:23:00Z

I talked to @mtias about this and we decided that this is an unnecessary optimisation that can make things worse for the user.

By providing specific search and 404 templates, we will make it easier for users to understand which template relates to which page. If we want everything to inherit from index that actually makes it harder for users to understand the templating system. However, if the user has search or 404 templates then they can easily make edits to them.

On the other hand, if the user does not have search or 404 templates then nothing will break, the page titles on the 404 and search pages will just be empty.

ntsekouras and others added 15 commits January 18, 2021 10:11

Add Archive Title block

1744a97

remove extra lines

a6ceb90

convert to int

03b2704

Add new ArchiveTitle icon

47aae1b

query title with block variations

00d949f

Fix merge errors

82ec6c7

Add support for search in query title

b2fb7d7

Based on original work in WordPress#27989

Add support for 404 in query title

258dc44

Merge branch 'WordPress:trunk' into add/search-404-titles

4ebfe53

Remove translators comment for 404 title

be5836c

Merge branch 'WordPress:trunk' into add/search-404-titles

9dd89e5

Remove subhead from block folders array

900c95b

Merged in by mistake

Add another wording option for search title

7e3f308

Reformat type if statement

3244717

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jul 16, 2021

mikachan changed the title ~~Add/search 404 titles~~ Query Title: add Search and 404 page titles Jul 16, 2021

mikachan added 3 commits July 19, 2021 11:39

Add context to 404 template title

22d458d

Merge branch 'WordPress:trunk' into add/search-404-titles

08da5a2

Add icons to search and 404 title blocks

1f40aca

mikachan marked this pull request as ready for review July 19, 2021 11:03

mikachan requested a review from ajitbohra as a code owner July 19, 2021 11:03

mtias reviewed Jul 19, 2021

View reviewed changes

scruffian reviewed Jul 19, 2021

View reviewed changes

scruffian requested a review from ntsekouras July 19, 2021 11:31

mikachan added 2 commits July 19, 2021 17:22

Remove Query Title variations

ecdd94b

Remove type attribute from Query Title

600204c

ntsekouras reviewed Jul 20, 2021

View reviewed changes

mikachan added 3 commits July 20, 2021 14:05

Change if to elseif in Query Title index.php

5e0eef7

Remove content from archive title block

9ed105b

Content is not used in archive

Change default content to 'Query title'

152103b

This allows the user to delete the block content and write something new

scruffian mentioned this pull request Jul 20, 2021

Post Content: Add some text to the Post Content block when used in a 404 context #33570

Closed

7 tasks

mikachan added 3 commits July 20, 2021 17:08

Add __unstableMarkNextChangeAsNotPersistent to useEffect

358cb81

Add searchTitleContent & nothingFoundTitleContent attributes

b5b52a8

Add InspectorControls for title variations

9df71fa

ntsekouras requested a review from a team July 22, 2021 06:37

mikachan added 3 commits July 22, 2021 09:27

Simplify title variable names

b1c26ae

Remove RichText import

5d37503

Change placeholder title to 'Query title placeholder'

91b79cd

swissspidy reviewed Jul 22, 2021

View reviewed changes

mikachan added 6 commits July 22, 2021 11:53

Remove default placeholder

ca655a2

Now defaults to empty string

Remove content attribute

b88c5de

Remove Inspector Controls

cc02de1

Allow search and 404 titles to be edited via usual block editor

Remove content attribute from block test

6ccbdad

Add default state for index template in editor

9f15ae5

Remove title variable based on content attribute

eff88a8

Content attribute no longer exists

carolinan added the [Block] Query Title Affects the Query Title Block label Oct 7, 2021

jffng mentioned this pull request Oct 18, 2021

Index: Sticky posts are showing when you view page 2 etc WordPress/twentytwentytwo#126

Closed

scruffian closed this Jul 14, 2022

mikachan deleted the add/search-404-titles branch July 14, 2022 22:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query Title: add Search and 404 page titles #33515

Query Title: add Search and 404 page titles #33515

mikachan commented Jul 16, 2021 •

edited

Loading

github-actions bot commented Jul 16, 2021

mtias Jul 19, 2021

mikachan Jul 19, 2021

scruffian Jul 19, 2021

mikachan Jul 19, 2021

mtias Jul 19, 2021

ntsekouras commented Jul 19, 2021

scruffian commented Jul 19, 2021

ntsekouras left a comment

ntsekouras Jul 20, 2021

ntsekouras Jul 20, 2021

mikachan Jul 20, 2021

ntsekouras Jul 20, 2021

ntsekouras Jul 20, 2021

scruffian Jul 20, 2021

ntsekouras Jul 20, 2021

mikachan Jul 20, 2021

scruffian commented Jul 20, 2021

ntsekouras commented Jul 20, 2021 •

edited

Loading

mikachan commented Jul 21, 2021

scruffian commented Jul 22, 2021

swissspidy Jul 22, 2021 •

edited

Loading

swissspidy Jul 22, 2021

mikachan commented Jul 22, 2021

scruffian commented Jul 14, 2022

Query Title: add Search and 404 page titles #33515

Query Title: add Search and 404 page titles #33515

Conversation

mikachan commented Jul 16, 2021 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntsekouras commented Jul 19, 2021

scruffian commented Jul 19, 2021

ntsekouras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scruffian commented Jul 20, 2021

ntsekouras commented Jul 20, 2021 • edited Loading

mikachan commented Jul 21, 2021

scruffian commented Jul 22, 2021

swissspidy Jul 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikachan commented Jul 22, 2021

scruffian commented Jul 14, 2022

mikachan commented Jul 16, 2021 •

edited

Loading

ntsekouras commented Jul 20, 2021 •

edited

Loading

swissspidy Jul 22, 2021 •

edited

Loading