Skip to content

Permit default_title, default_content, and default_excerpt filters to work as expected #10362

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

Merged

Conversation

mnelson4
Copy link

@mnelson4 mnelson4 commented Oct 5, 2018

Description

Don't assume new posts have only blank content. They may have defaults set using the filters default_content, default_title and default_excerpt

How has this been tested?

Add this code snippet to functions.php:

add_filter( 'default_title', function() {
	return 'My default title';
} );

add_filter( 'default_content', function() {
	return 'My default content';
} );

add_filter( 'default_excerpt', function() {
	return 'My default excerpt';
} );

when you create a new post with the classic editor, those default values will appear immediately when you begin a new post.
However, when you create a new post in Gutenberg, those default values don't appear.

Screenshots

Classic editor:
classic
Gutenberg Master:
gutenberg ignoring defaults
Gutenberg this branch:
gutenberg fixed2

Types of changes

Bug fix. Addresses #8757 (comment)
All we needed to do is not ignore the values already set on the global $post.

Checklist:

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

Mike Nelson added 2 commits October 5, 2018 11:16
…default_content, and default_excerpt may have been used to set a deafult
… when rendering the content for display to the user, not when populating the textareas for editing
@tofumatt tofumatt requested a review from a team October 5, 2018 21:30
@tofumatt
Copy link
Member

tofumatt commented Oct 5, 2018

Interesting! This makes sense but I'm not really familiar enough with the filter-world to review this. Flagging the core team for a review.

@aduth
Copy link
Member

aduth commented Oct 5, 2018

I've yet to test this, but I expect without the default_title filter present, the new post will show with a "Auto-Draft" title ?

@danielbachhuber
Copy link
Member

danielbachhuber commented Oct 5, 2018

For the record, it looks like the original source of apply_filters( 'the_title' ) was #1967 a2ffbd2#diff-620130744b6b73b822a3df609671d930R548

It'd be great to have e2e tests around this too.

@danielbachhuber danielbachhuber added this to the 4.1 milestone Oct 8, 2018
@danielbachhuber danielbachhuber added Backwards Compatibility Issues or PRs that impact backwards compatability [Type] Bug An existing feature does not function as intended labels Oct 8, 2018
@mnelson4
Copy link
Author

mnelson4 commented Oct 8, 2018

I've yet to test this, but I expect without the default_title filter present, the new post will show with a "Auto-Draft" title ?

From my testing, that doesn't happen. On this branch, without adding those filter callbacks to functions.php, the UX is the same as master.
Here's a video that should help. At first, I show the code snippet is active, and the Gutenberg editor gets pre-populated with the default values specified in those callbacks. Then I commented out those code snippets, and began adding a new post with Gutenberg, and the title, body, and excerpt all began as blank, as expected. See https://drive.google.com/file/d/1v9Ff3JLAtAfDhFhU8yyMs8TmAXoaks6n/view

So: the title "Auto-Draft" doesn't appear in the post editor. (I believe that's because the auto-draft that's created gets that title, but that's different from the $post object used to populate the post-new.php form; and when the auto-draft is update via a REST API call later, its post_title gets updated to be whatever is currently in the title field in the editor.)

It'd be great to have e2e tests around this too.

I have yet to write any end-to-end tests but am happy for an opportunity! I'll see what I can do...

@mnelson4
Copy link
Author

mnelson4 commented Oct 8, 2018

oh also I see the title's raw value was originally set to apply_filters( 'the_title', '', $post->ID ), on cd6ae73755 for the reason @aduth mentioned, but like I said from my testing that's isn't currently the case. (Maybe it was at the time?)

Also, it was strange that we were applying the filter on the raw value. Elsewhere we apply that filter to the rendered value

@Chouby
Copy link
Contributor

Chouby commented Oct 8, 2018

The "auto-draft" title is coming from the REST API, used there: https://github.com/WordPress/gutenberg/blob/master/lib/client-assets.php#L1305

By default, for a new post, the global $post (and the corresponding post in database) has an empty title.
The purpose of the $initial_edits array is to overwrite the values coming from the REST API. That's why the title is already filled with an empty value before applying this PR, just to avoid "auto-draft" to be displayed.
Since the PR still always overwrites the title coming from the REST API, there is no risk see the "auto-draft' title.

It also looks strange to me to apply the filter 'the_title' here.

@danielbachhuber danielbachhuber changed the title dont assume a new post starts with all blank content. default_title, … Permit default_title, default_content, and default_excerpt filters to work as expected Oct 10, 2018
@mnelson4
Copy link
Author

I added another test file and a supporting plugin. I also considered adding the test as part of new-post.test.js. Let me know if that would be better.

I'm new to Puppeteer and Jest, so had difficulty selecting the excerpt in order to assert it also got changed. I left the code commented out, hoping someone could help me figure that part out.

Otherwise, I think this is pretty helpful, I think.

@mnelson4
Copy link
Author

Ok now the tests are asserting the excerpt is correct too.

So I believe this is ready for a review.

@danielbachhuber
Copy link
Member

Thanks @mnelson4 !

Could you add one more e2e test (if there isn't one already) asserting that the default state of title is empty when creating a new post without your custom plugin active?

@mnelson4
Copy link
Author

Could you add one more e2e test (if there isn't one already) asserting that the default state of title is empty when creating a new post without your custom plugin active?

I added an assertion to the already existing new-post.test.js's test 'should show the New Post page in Gutenberg'. It seemed to me those tests are meant to assert the new editor's default state, and this seems to be part of it. (And I'm still open to adding my new test to that existing file instead of a new file)

@danielbachhuber
Copy link
Member

💯 Thanks for your work on this, @mnelson4

@danielbachhuber danielbachhuber merged commit 5b16226 into WordPress:master Oct 11, 2018
@tmatsuur
Copy link

Great. I am looking forward to the next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants