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

Add support for the `default_page_template_title` filter in page-attributes meta-box. #6948

Merged
merged 3 commits into from May 25, 2018

Conversation

Projects
None yet
2 participants
@kallehauge
Contributor

kallehauge commented May 24, 2018

Description

I have wrapped the availableTemplates call to wp_get_theme()->get_page_templates(...) in a PHP function to directly add the default template into the array so we can use the already existing default_page_template_title filter instead of hardcoding it in the editor/components/page-attributes/template.js file.

More about the reasoning can be found in #6946

Fixes #6946

How has this been tested?

The change is very isolated and will only have any effect on the page-attributes meta box. I have run manual testing by testing the following scenarios:

  • Make sure we only append the default template logic if the page have any templates (done by using the Twentysixteen theme).
  • Make a child theme of Twentysixteen and add a template.
  • Save / update the post to make sure the value is "saved correctly" (more precisely the lack of a value since the value for the default template is an empty string).

The environment was a clean vanilla install of WordPress 4.9.6 with Twentysixteen 1.5 run on a custom Docker installation (PHP 7.1 / Apache 2.4) with a manually run npm install and npm run dev.

Types of changes

Added new functionality to support already existing functionality in WordPress.

Checklist:

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

@danielbachhuber danielbachhuber self-requested a review May 25, 2018

@danielbachhuber danielbachhuber added this to the 3.0 milestone May 25, 2018

Avoid defining a new function
It's less new code to simply define this inline
@danielbachhuber

Thanks @kallehauge ! I've pushed a small improvement in b59cd28

@danielbachhuber danielbachhuber merged commit b627478 into WordPress:master May 25, 2018

2 checks passed

codecov/project 46.11% (-0.01%) compared to 3cfaf1d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kallehauge

This comment has been minimized.

Contributor

kallehauge commented May 25, 2018

Awesome @danielbachhuber ! And nice improvements to the code 💪

@kallehauge kallehauge deleted the kallehauge:add/default-template-filter branch May 25, 2018

danielbachhuber added a commit to danielbachhuber/gutenberg-migration-guide that referenced this pull request May 25, 2018

@kallehauge

This comment has been minimized.

Contributor

kallehauge commented May 25, 2018

Oi! @danielbachhuber I think I just realized a scenario your more streamlined code do not take into account (the comment here).

The '' => Default template array entry should not be added if the array is empty (there are no additional templates to choose from).
The reason for this is that the template <select> will not show up if the array is empty and it will just be assumed that the default template will be used. But if we always add Default template to the array, then the <select> will always appear.

@danielbachhuber

This comment has been minimized.

Member

danielbachhuber commented May 25, 2018

The '' => Default template array entry should not be added if the array is empty (there are no additional templates to choose from).

Ah, makes sense. Want to submit a new PR with that change?

@kallehauge

This comment has been minimized.

Contributor

kallehauge commented May 25, 2018

Sure @danielbachhuber 👍

@kallehauge kallehauge referenced this pull request May 25, 2018

Merged

Add additional condition to "Available templates" logic #6955

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment