#39281 closed enhancement (fixed)
Twenty Seventeen: header.php forces thumbnails on all post types
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.6 | Priority: | normal |
| Severity: | normal | Version: | 4.8 |
| Component: | Bundled Theme | Keywords: | has-patch needs-testing has-test-info |
| Focuses: | ui, template | Cc: |
Description
My plugin has custom post type and custom 'single-post-type' views and doesn't utilize thumbnails in them. As a result layout appears broken, and there is nothing I can do to make the plugin compatible with Twentyseventeen.
I went to have a look how WooCommerce deals with this, and it turns out that there is this piece of code in the style of twentyseventeen:
.single-product .single-featured-image-header {
display: none
}
While that does work, I don't think it's a solution. At the very least there has to be a filter to dynamically disable the thumbnail from header.php
Attachments (9)
Change History (38)
#1
@
9 years ago
- Focuses performance removed
- Resolution set to worksforme
- Status changed from new to closed
#2
@
9 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
Hey @JPry, thanks for the response. However, I have to disagree. My custom-post-type does support thumbnails, but I utilize them in a different way. Because of the current state of twentyseventeen I can't make the plugin compatible with the default WordPress theme. I think every WordPress plugin should look and feel great out of the box, with best practices intact. Currently that's impossible for me.
For my Photography Portfolio plugin - featured images are used in archives, but once a gallery is opened - a different set of images are used. I don't need to show the featured-image there, so at the moment, I have to resort to a hack like this one: https://github.com/justnorris/photography-portfolio/commit/d85b14b0232830fbceeb95eac10e5b196d10ae35
I'd really appreciate it if at least a filter was introduced. I can't be the only person on earth doing a custom view for a custom post type.
In any case - I think the header.php readability will too benefit from something like this:
<?php // If a regular post or page, and not the front page, show the featured image. if ( twentyseventeen_should_show_featured_image() ) : echo '<div class="single-featured-image-header">'; the_post_thumbnail( 'twentyseventeen-featured-image' ); echo '</div><!-- .single-featured-image-header -->'; endif; // functions.php function twentyseventeen_should_show_featured_image() { return apply_filters( 'twentyseventeen_should_show_featured_image', ( has_post_thumbnail() && ( is_single() || ( is_page() && ! twentyseventeen_is_frontpage() ) ) ) ); } ?>
Normally things like thumbnails belong to single.php, archive.php, etc., Twenty Seventeen does things a bit differently, that's alright, as long as it doesn't break things for others.
#3
@
9 years ago
- Keywords has-patch added
- Type changed from defect (bug) to enhancement
Replying to justnorris:
My custom-post-type does support thumbnails, but I utilize them in a different way.
That's different than what you said originally, which is why my original suggestion was to ensure your plugin wasn't providing support for thumbnails.
Having a filter certainly seems feasible to me.
#5
@
9 years ago
- Keywords reporter-feedback added
Hey @justnorris! Thanks for the report!
As a result layout appears broken, and there is nothing I can do to make the plugin compatible with Twentyseventeen.
Can you add some screenshots so I can see how it's broken? It would help determine the best path forward.
Also, thanks @JPry for the initial patch.
#6
@
9 years ago
- Summary changed from [twentyseventeen] header.php forces thumbnails on all post types to Twenty Seventeen: header.php forces thumbnails on all post types
#7
@
8 years ago
@davidakennedy Ha, I didn't get a notification about the issue, and unfortunately the issue is still present 12 months later, even with the patch 😔
Here are the screenshots, I had to zoom out to 50% just to capture the general idea of the layout. It's a view of a single portfolio entry with a description and a gallery. The featured image is really unnecessary there.
#9
@
8 years ago
Having just found this issue myself, I concur with the OP that header.php is not a logical place for the featured image unless that image is visually integrated into the header – e.g. replacing the header image. This does not appear to be the case with Twenty Seventeen.
In my case, I am developing a personal child theme so was able to override header.php and comment out the relevant block. It took me a while to figure out where the image was coming from!
This ticket was mentioned in PR #6157 on WordPress/wordpress-develop by @poena.
21 months ago
#10
Trac ticket:
https://core.trac.wordpress.org/ticket/39281
This is a refresh of patch https://core.trac.wordpress.org/attachment/ticket/39281/39281.diff
#11
@
21 months ago
Testing instructions
Apply PR https://github.com/WordPress/wordpress-develop/pull/6157.
Activate the theme
Create two pages and a post with featured images.
View the pages and post on the front.
Expected Results
When testing a patch to validate it works as expected:
- ✅ On the single post and the single page, the featured image shows.
Assign one of the pages as the front page:
Go to the WordPress admin, Settings, Reading, and change
"Your homepage displays" to a static page, and select the page you just created.
Expected Results
When testing a patch to validate it works as expected:
- ✅ The theme header image and the page featured image shows once.
To test the filter, add the following in functions.php after twentyseventeen_should_show_featured_image(), or in a child theme or plugin:
add_filter( 'twentyseventeen_should_show_featured_image', '__return_false' );
Expected Results
When testing a patch to validate it works as expected:
- ✅ On the static front page with the page assigned, the theme header image and the page featured image should show once.
- ✅ On the single post and the single page, the featured image should not show.
#14
@
21 months ago
The reason why the featured image still shows on the static front page is because of the themes "panel image" feature.
Without the condition in twentyseventeen_should_show_featured_image(), the panels are broken.
This ticket was mentioned in Slack in #core-themes by karmatosed. View the logs.
19 months ago
This ticket was mentioned in Slack in #core by nhrrob. View the logs.
18 months ago
#17
@
18 months ago
- Keywords has-testing-info added
This one has testing instructions and needs testing.
Updating keywords accordingly.
This ticket was mentioned in Slack in #core-test by nhrrob. View the logs.
18 months ago
#19
@
18 months ago
Hello @poena and @nhrrob,
I have tested as per given instructions with PR.
Expected Results
When testing a patch to validate it works as expected:
✅ On the single post and the single page, the featured image shows.
Assign one of the pages as the front page:
Go to the WordPress admin, Settings, Reading, and change
"Your homepage displays" to a static page, and select the page you just created.
PASS
Expected Results
When testing a patch to validate it works as expected:
✅ The theme header image and the page featured image shows once.
To test the filter, add the following in functions.php after twentyseventeen_should_show_featured_image(), or in a child theme or plugin:
add_filter( 'twentyseventeen_should_show_featured_image', 'return_false' );
PASS
Expected Results
When testing a patch to validate it works as expected:
✅ On the static front page with the page assigned, the theme header image and the page featured image should show once.
PASS
✅ On the single post and the single page, the featured image should not show.
It is working as aspected.
PASS
I have also attached the screenshot.
Thanks,
Rajan Lama (@lamarajan)
This ticket was mentioned in Slack in #core-test by lamarajan. View the logs.
18 months ago
This ticket was mentioned in Slack in #core-test by lamarajan. View the logs.
18 months ago
#22
@
18 months ago
- Keywords commit added
- Owner set to karmatosed
- Status changed from reopened to assigned
It looks like this is tested and ready to move to commit which is great. I will add that keyword and look to move it there. Thanks everyone.
#24
@
18 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
This is being reopened to get the documentation also in and assigned to @poena to see that through.
This ticket was mentioned in PR #6657 on WordPress/wordpress-develop by @poena.
18 months ago
#27
…tured_image
Trac ticket: https://core.trac.wordpress.org/ticket/39281
The new function `twentyseventeen_should_show_featured_image is missing documentation for the custom filter with the same name.
This PR updates the summary and adds an example of how to use the filter.
@SergeyBiryukov commented on PR #6657:
11 months ago
#29
Thanks for the PR! This was merged in r58237.
Hey @justnorris,
Thanks for taking the time to log a ticket here on Trac!
The twentyseventeen theme will only display a thumbnail when
has_post_thumbnail()is true. As long there's no featured image set, then the theme won't try to display one. That means that there's no need to disable the thumbnail. Here's the relevant section ofheader.php:It appears that WooCommerce is simply taking an extra precaution with hiding a possible featured image.
When registering your custom post type, you can also make sure that it does not support
thumbnailin thesupportsparameter. This will ensure that users cannot even set a featured image to begin with.