Opened 8 years ago
Closed 8 years ago
#38737 closed defect (bug) (fixed)
Add selective refresh to new custom header functions
Reported by: | peterwilsoncc | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Themes | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
Forked from #38639
cc: @bradyvercher, @celloexpressions, @westonruter
Adding to milestone per original ticket
Original description for this part of the ticket:
Selective Refresh
A Customizer partial is registered by default and uses the_custom_header_markup()
for the render callback, which allows both header images and videos to automatically support selective refresh. If a header image or video hasn't been set, we don't want to output empty markup on the front end, however, the container div needs to exist in the Customizer for selective refresh to work. Otherwise, setting an image or video will result in a full refresh.
the_custom_logo()
, along with other features in core, output placeholders in the Customizer preview for the optimal experience, so making sure the container is always available in the preview isn't really breaking new ground. Themes can always use .wp-custom-header:empty { display: none;}
to hide it if it causes styling issues in the preview.
Header Image Settings
It may also be worth changing the transport for the header_image
and header_image_data
settings to postMessage
automatically when the the theme registers support for videos. Otherwise, it's kind of a weird experience when videos use selective refresh and images don't.
Attachments (6)
Change History (18)
#2
@
8 years ago
If this isn't in 4.7 we won't be able to add selective refresh for headers at all, so we should do it now. And the first part of the fix here is definitely required - ensuring that the container is always present in the customize preview.
This ticket was mentioned in Slack in #core-themes by joemcgill. View the logs.
8 years ago
#4
@
8 years ago
- Keywords has-patch added; needs-patch removed
38737.diff adds selective refresh support for custom headers. It:
- Ensures the container is always present in the Customizer.
- Sets the transport for
header_image
andheader_image_data
topostMessage
when support for video headers is registered. - Updates the default theme implementations.
The front page restrictions will need to be updated in #38738.
#5
@
8 years ago
38737.2.diff fixes the conditional at the bottom of the_custom_header_markup()
to make sure a video isn't loaded on pages other than the front page in the Customizer preview.
This ticket was mentioned in Slack in #core-customize by helen. View the logs.
8 years ago
#8
@
8 years ago
The other benefit of showing the custom header element, even when there is no header image/video selected, is that the edit shortcut will appear.
#9
@
8 years ago
- Keywords needs-testing added
@bradyvercher please review my additions to your patch: https://github.com/xwp/wordpress-develop/pull/200/files/c15bcf0..0c0d18e
- Ensure that edit shortcut is not positioned off-screen in custom header.
- Ensure that
has-custom-header
class is toggled on the body when the header image/video is changed.
@bradyvercher following up your request for feedback on the previous ticket.
I think this is a no-brainer enhancement but whether it goes on the 4.7 milestone or in 4.8 is something I need to defer to the component maintainers and lead devs.
I see no forward compatibility concerns with either committing now or later. With RC1 next Wednesday, the patch would need to be in soon for it to have any chance for 4.7.