WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 5 months ago

Last modified 2 months ago

#38615 closed task (blessed) (fixed)

Round out and refine starter content

Reported by: helen Owned by: helen
Milestone: 4.7 Priority: high
Severity: normal Version:
Component: Themes Keywords: needs-testing has-patch
Focuses: Cc:

Description

Starter content is in as of [38991] for #38114; however, the content could still use fleshing out and refining. Core-bundled starter content should provide what the default themes need to showcase themselves, as well as taking a look at some of the more popular themes to see if there's anything else that themes would generically benefit from.

Also needed: communication with the theme review team and a dev note.

Attachments (18)

Screen Shot 2016-11-12 at 15.26.20.png (35.2 KB) - added by iseulde 6 months ago.
38615.diff (2.4 KB) - added by helen 5 months ago.
38615.2.diff (5.8 KB) - added by helen 5 months ago.
38615.3.diff (7.2 KB) - added by helen 5 months ago.
38615.4.diff (8.0 KB) - added by helen 5 months ago.
38615.5.diff (8.1 KB) - added by flixos90 5 months ago.
38615.6.diff (8.7 KB) - added by flixos90 5 months ago.
38615.7.diff (8.7 KB) - added by flixos90 5 months ago.
38615.8.diff (9.5 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/24bc3f6..6940730
38615.9.diff (8.7 KB) - added by helen 5 months ago.
images_75.zip (543.0 KB) - added by Presskopp 5 months ago.
compress2017.jpg (90.2 KB) - added by Presskopp 5 months ago.
38615.10.diff (9.9 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/30865c7..1da2f6e
38615.11.diff (10.5 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/1da2f6e..d400b0c
38615.12.diff (12.9 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/54248b5..33f04fc
38615.13.diff (23.2 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/33f04fc..34978df
38615.14.diff (23.1 KB) - added by westonruter 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/commits/a3dd024
38615.15.diff (22.8 KB) - added by helen 5 months ago.
Δ https://github.com/xwp/wordpress-develop/pull/201/files/a3dd024..da2d931

Download all attachments as: .zip

Change History (64)

This ticket was mentioned in Slack in #themereview by djrmom. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by helen. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


6 months ago

#4 @helen
6 months ago

Generally, the idea here is to have content that showcases various WP and default theme features and gives users an idea of what sort of site X theme will help them create. Of note, I think we need to flesh out the page content that's been committed and add more that does some showcasing of formatting and media (doesn't need to be full-on theme unit testing by any means), as well as do a little more with widgets. We may also want to think about how to allow for page template setting, although not sure it's critical for 4.7.

I should also point out that images are missing entirely from this. While I think they are important, I'm okay with not considering them critical for launch either, and there are a ton of considerations around sizes and where they're loaded from and how.

First run commit content, for easy reference (options and theme mods can also be set):

Widgets

text_business_info

  • Text widget
  • Title: Find Us
  • Text:
    Address
    123 Main Street
    New York, NY 10001

    Hours
    Monday—Friday: 9:00AM–5:00PM
    Saturday & Sunday: 11:00AM–3:00PM

search

  • Search widget
  • Title: Site Search

text_credits

  • Text widget
  • Title: Site Credits
  • Text: This site was created on {{current time}}

Each of the sample content pages is available via the following IDs:

  • page_home
  • page_about
  • page_blog
  • page_contact

Links (currently geared toward commonly used social nav menus):

link_yelp

link_facebook

link_twitter

link_instagram

link_email

  • Title: Email
  • URL: mailto:wordpress@…

Posts (of any type)

home

  • Type: Page
  • Title: Homepage
  • Content: Welcome home.

about-us

  • Type: Page
  • Title: About Us
  • Content: More than you ever wanted to know.

contact-us

  • Type: Page
  • Title: Contact Us
  • Content: Call us at 999-999-9999

blog

  • Type: Page
  • Title: Blog

homepage-section

  • Type: Page
  • Title: A homepage section
  • Content: This is an example of a homepage section, which are managed in theme options.

#5 @iseulde
6 months ago

Related: #38770 (context for strings).

#6 follow-up: @iseulde
6 months ago

I'm not sure what the "Site Credits" widget is about and it looks a little bit weird. Maybe the time format needs to be adjusted? I'm not sure in general how useful this is. While translating I was assuming this is for the "Proudly powered by WordPress" text.

#7 in reply to: ↑ 6 @helen
6 months ago

Replying to iseulde:

I'm not sure what the "Site Credits" widget is about and it looks a little bit weird. Maybe the time format needs to be adjusted? I'm not sure in general how useful this is. While translating I was assuming this is for the "Proudly powered by WordPress" text.

Yeah this one definitely needs changing :) Might be better off as an "about this site" kind of widget instead, not sure how much people really think of that separately from credits.

#8 @poena
6 months ago

If we look at the popular and latest themes there are several things that are common but that will have to remain theme_mods since there is no standard. Like the multiple "Feature" or "Service" boxes with a title, text and icon, and placing section headlines above specific areas.

I was also thinking that e-commerce themes are generally more difficult to setup and preview than other themes, it would be nice if we could come up with something that could help with that.

#9 @westonruter
5 months ago

Note that #38541 has been resolved, so multiple themes can have starter content and each will apply when switching themes on a fresh site. Starter content from theme B will override starter content from theme A as long as the starter content in A was not touched in the customizer session.

This ticket was mentioned in Slack in #themereview by djrmom. View the logs.


5 months ago

#11 @helen
5 months ago

In 39255:

Theme starter content: Revamp the credits widget into an about widget.

The credits widget from the original commit was a nice test to see the date changing, but isn't really very inspirational. Also, implement it in Twenty Seventeen.

see #38615.

#12 @helen
5 months ago

In 39256:

Theme starter content: Add more social link items to select from.

New: Foursquare, GitHub, LinkedIn, Pinterest, and YouTube. They are also now all alphabetized.

see #38615.

#13 @helen
5 months ago

In 39260:

Theme starter content: Refine the content for pages.

see #38615.

#14 @helen
5 months ago

In 39261:

Theme starter content: Add reference IDs for most default widgets.

Some widgets that require more configuration are not included, such as RSS and Custom Menu. Tag Cloud is also not included because fresh sites do not have any tags to display.

Also adds a search widget to a Twenty Seventeen footer widget area.

see #38615.

@helen
5 months ago

#15 @helen
5 months ago

Noting for future reference - 38615.diff is an exploration of adding support for featured images; Twenty Seventeen would particularly benefit from this behavior, but I imagine the same holds true for many themes. It includes untested support for page templates since it's similar in terms of storage. There's also a test change to Twenty Seventeen to show what the format would be like in this case - note that I've used an actual ID from my install, and that where exactly images should come from is TBD - from within theme directory or w.org are currently floated options.

Further, there's a filter for whitelisting more args/fields for pages, though it'd be kind of funky to use for anything that's not a top level wp_insert_post() $postarr element because you'd have to also use the later filter. Just straight up allowing meta_input to be defined is also an option but complicates the format for theme authors and requires knowledge of how internal meta keys are named.

#16 @westonruter
5 months ago

The attachments themselves could be defined in the same way as other posts, with a named symbol like attachment_header_image, and then this could be referenced as the value of a thumbnail as {{attachment_header_image}}. The attachment posts could accept an additional file_url param to point to the remote location which would then get downloaded when the starter content is imported.

#17 @westonruter
5 months ago

In 39276:

Customize: Add unit tests for importing theme starter content.

Props welcher, westonruter.
See #38114, #38533, #38615.
Fixes #38540.

@helen
5 months ago

@helen
5 months ago

@helen
5 months ago

#18 @helen
5 months ago

38615.4.diff allows for sideloading of attachments from the theme directory into the media library and for them to be used as featured images. Because media is involved in a front-end context, there are ugly file includes involved. One thing that appears not to be working is setting auto-draft for the attachments. But otherwise it appears to work. Thanks to @westonruter for the help on how the $nav_menus_created_posts bit works :)

What this means is that themes would grow in size somewhat if they include images for this purpose. I don't know that we want to artificially limit the number of images to load in core but it could take a while to process the uploads if too many are registered. For Twenty Seventeen, I took the images off http://2017.wordpress.net for testing, but at the 2000px wide size because that's all that was needed, not the original full size.

#19 @helen
5 months ago

Hmm, getting an array to string conversion notice in /srv/www/wordpress-develop/src/wp-includes/class-wp-customize-manager.php L1079 when refreshing the customizer, which is $this->set_post_value( $setting_id, array_unique( array_values( $nav_menus_created_posts ) ) ).

This ticket was mentioned in Slack in #core-media by helen. View the logs.


5 months ago

@flixos90
5 months ago

#21 @flixos90
5 months ago

38615.5.diff includes a minor fix in WP_Customize_Manager where, when using a child theme that does not provide its own image replacement, the file is taken from the parent theme. Previously this would have caused an issue.

Last edited 5 months ago by flixos90 (previous) (diff)

@flixos90
5 months ago

#22 @flixos90
5 months ago

As discussed in https://wordpress.slack.com/archives/core-media/p1479488594000409, it's currently not possible to set an attachment to the post status auto-draft. This was causing previous patches to immediately have the starter content images show up in the media library. 38615.6.diff removes that limitation from the wp_insert_post() function by including auto-draft in the whitelist for attachment post statuses. The starter content media workflow worked correctly for me after applying this change.

@joemcgill Do you have any reservations regarding this? Why was this prevented in the first place?

@flixos90
5 months ago

#23 @flixos90
5 months ago

38615.7.diff is a minor tweak to make the if clause added in 38615.5.diff more readable: get_stylesheet_directory_uri() is only called when there's a child theme and it contains the file, otherwise get_template_directory_uri() is used.

#24 @helen
5 months ago

In 39294:

Twenty Seventeen: Rename the starter content menus to match the menu area names.

Saves strings and is more descriptive.

props davidakennedy.
see #38615.

This ticket was mentioned in Slack in #core-customize by helen. View the logs.


5 months ago

#26 @westonruter
5 months ago

38615.8.diff fixes the problem with array to string conversion notice. It was a trivial oversight (of mine).

Also fixed an issue an “undefined index: sizes” PHP notice. When a header image is not sourced from an attachment, then there is no sizes item in the array passed to the get_header_image_tag filter. Maybe the sizes should be defined in the array defaults to prevent that from happening unexpectedly. (But that's been the case since 4.4.)

@helen
5 months ago

#27 follow-up: @helen
5 months ago

  • Keywords has-patch needs-testing added

38615.9.diff is ready for testing. To test, you will need:

  • These images in twentyseventeen/assets/images: http://s.hyhs.me/iAFm
  • To set the fresh_site option to 1 (if you are a wp-cli user, wp option set fresh_site 1).
  • Load up the customizer to view Twenty Seventeen (both as the active theme and not) and see something that hopefully looks like this (except with the default header image): http://s.hyhs.me/iAfl

What this patch should get you is featured images set for each of those pages that are homepage sections. These attachments are sideloaded from within the theme directory and are set to auto-draft so they don't appear in the media library until you save your changes; otherwise, they should stay hidden and later be garbage collected. It also happens to enable page templates to be set; to test that, you will want to make a page template inside the twentyseventeen directory and add 'template' => 'sample-page-template.php' (or whatever the filename is) to one of the non-blog page/post definitions in functions.php.

I'm interested in:

  • Does this help present Twenty Seventeen in a better way upon first live preview? Do you see this being useful for other themes that use image attachments in various contexts (the ID can be used for theme mods and more, not just the post thumbnail)?
  • How does the load time feel? This is a concern because of the image sideloading. Themes should be encouraged not to add too many attachments this way, both for load reasons and not to bloat theme packages too much.
  • Does everything save correctly? In particular, do those sideloaded images only appear in the media library after you've saved and published your customizer changes?
  • Is enabling auto-draft for attachments a problem? Does the garbage collection work?
  • What do you think of the patch in general?
  • Are there any bugs?

And finally, if this is the first time you're taking a look at starter content, are there any changes or additions you'd want to see in the other defined content?

Last edited 5 months ago by helen (previous) (diff)

This ticket was mentioned in Slack in #core by helen. View the logs.


5 months ago

#29 @westonruter
5 months ago

  • Keywords needs-unit-tests added

@Presskopp
5 months ago

#30 @Presskopp
5 months ago

I compressed the images regarding to #38793. I used IrfanView for that, and maybe we could discuss (somewhere else) how we proceed in the future with that. See next screenshot for the settings used. I did not use progressive setting, for example.

#31 @poena
5 months ago

Both the images and the page templates are working very well, no issues found (4.7-beta4-39320 + 38615.9).

#32 @westonruter
5 months ago

  • Keywords needs-patch added; has-patch removed

In 38615.10.diff & 38615.11.diff (see changes):

  • Set auto-draft via 4th arg on media_handle_sideload() instead of an additional call to wp_update_post().
  • Allow starter content to supply attachment args for title, description, and excerpt; supply translatable titles for the images in Twenty Seventeen.
  • Sideload attachment from disk instead of downloading from URL when possible.
  • Prevent clobbering gathered attachment IDs when amending to existing changeset.

Sideloading the 3 images by first fetching each via download_url() causes the entire starter content import process to take about 2.05 seconds. When the files are already on the filesystem, as they have been in our examples, it is much faster to copy the file to the temporary location and then use temp file in the call to media_handle_sideload(). This cuts the starter content import time down to 0.07 seconds (about 30× faster).

I am noticing a probably defect with the attachment importing whereby once images are imported and you reload the customizer, the images get re-uploaded into the media library again even though they are already there as auto-drafts. Likewise, the nav_menus_created_posts setting keeps getting the new attachment IDs amended to it even though they aren't actually used because posts are skipped from being inserted (and thus the new thumbnail assigned) if there is already an auto-draft post with the expected post_name in the DB among the nav_menus_created_posts.

This ticket was mentioned in Slack in #core-customize by helen. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 months ago

#35 @westonruter
5 months ago

  • Owner set to westonruter
  • Status changed from new to accepted

#36 @westonruter
5 months ago

  • Keywords needs-patch removed

In 38615.12.diff (see changes):

  • Eliminate support for fetching starter content files by URL
  • Allow any recognized file for starter content attachments
  • Prevent attachments from being re-created and re-sideloaded when auto-drafts already exist in the customized state

#37 in reply to: ↑ 27 @westonruter
5 months ago

Replying to helen:

  • Is enabling auto-draft for attachments a problem? Does the garbage collection work?

I can confirm that the attachment posts with the auto-draft status do indeed get deleted via wp_delete_auto_drafts(), with one caveat. It turns out that the cron event that does wp_delete_auto_drafts() is only scheduled when a user lands on post-new.php:

<?php
// Schedule auto-draft cleanup
if ( ! wp_next_scheduled( 'wp_scheduled_auto_draft_delete' ) )
        wp_schedule_event( time(), 'daily', 'wp_scheduled_auto_draft_delete' );

This logic should be added to WP_Customize_Manager as well so that these starter content auto-draft posts will get garbage collected (as well as the unpublished customize_changeset posts themselves) will get garbage-collected in the rare case where a user never goes to post-new.php on a given install. A user never visiting post-new.php is entirely possible if the user does all of their site management in the customizer or via the REST API.

Update: I've opened #38899 to address this.

Last edited 5 months ago by westonruter (previous) (diff)

#38 @westonruter
5 months ago

  • Keywords has-patch added; needs-unit-tests removed

In 38615.13.diff (see changes):

  • Fix handling of theme starter content extending core content arrays.
  • Enforce subset of post fields that can be used in starter content.
  • Add unit tests for get_theme_starter_content() and WP_Customize_Manager::import_theme_starter_content().

#39 @westonruter
5 months ago

  • Owner changed from westonruter to helen
  • Status changed from accepted to assigned

Unit tests should be running soon: https://travis-ci.org/xwp/wordpress-develop/builds/177901400

I was seeing a unit test failure on Travis where an attachment file wasn't getting added successfully:

File is empty. Please upload something more substantial. This error could also be caused by uploads being disabled in your php.ini or by post_max_size being defined as smaller than upload_max_filesize in php.ini.

I tried it a second time and it seemed to not be a problem then. So I'm not sure.

This ticket was mentioned in Slack in #core by westonruter. View the logs.


5 months ago

#41 @helen
5 months ago

That was a pretty wild PHP bug chase :) 38615.15.diff fixes the issue caught in the tests - I think we're ready.

#42 @helen
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39346:

Theme starter content: Add support for featured images and page templates.

Featured image support means that attachments can now be imported. Media can be sideloaded from within theme or plugin directories. Like other posts, attachments are auto-drafts until customizer changes are published, and are not duplicated when they already exist in the customized state. Attachment IDs can be used for any number of purposes, much like post IDs. Twenty Seventeen now includes 3 images used as featured images to best showcase the multi-section homepage setup.

As featured image IDs are stored in post meta, it also made sense to add support for page templates. Twenty Seventeen does not include any such templates, but the functionality can be quite important for displaying themes to their best effect.

props westonruter, helen, flixos90.
fixes #38615.

#43 @westonruter
5 months ago

In 39561:

Customize: Deprecate page_home nav menu item starter content in favor of home_link; replace usage in Twenty Seventeen.

Props celloexpressions, westonruter.
Amends [38991].
See #38615, #38114.
Fixes #39104.

#44 @dd32
5 months ago

In 39575:

Customize: Deprecate page_home nav menu item starter content in favor of home_link; replace usage in Twenty Seventeen.

Props celloexpressions, westonruter.
See #38615, #38114, [38991].
Merges [39561] to the 4.7 branch.
Fixes #39104.

#45 @westonruter
3 months ago

In 39924:

Customize: Allow custom post types to be used in starter content.

Changes WP_Customize_Nav_Menus::insert_auto_draft_post() so it can be invoked for a post_type that is not registered (yet).

See #38615, #38114.
Fixes #39610.

#46 @dd32
2 months ago

In 40098:

Customize: Allow custom post types to be used in starter content.

Changes WP_Customize_Nav_Menus::insert_auto_draft_post() so it can be invoked for a post_type that is not registered (yet).

Props westonruter.
Merges [39924] to the 4.7 branch.
See #38615, #38114.
Fixes #39610.

Note: See TracTickets for help on using tickets.