WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 9 days ago

#39487 new defect (bug)

Default to 'transport'=>'postMessage' for a setting associated with a selective refresh partial

Reported by: danielbachhuber Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.5
Component: Customize Keywords: has-patch needs-testing has-unit-tests
Focuses: Cc:
PR Number:

Description

As a WordPress developer, it can be confusing when registering a selective refresh partial to a setting doesn't work because I've omitted 'transport'=>'postMessage' for the setting.

When registering a selective refresh partial to a setting, I'd expect 'transport'=>'postMessage' to be set dynamically for the setting.

Attachments (2)

39487.diff (2.6 KB) - added by dlh 11 days ago.
39487.2.diff (6.9 KB) - added by dlh 9 days ago.

Download all attachments as: .zip

Change History (5)

#1 @westonruter
3 years ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

This makes sense to me.

One possible implementation would be to add a foreach loop in \WP_Customize_Manager::prepare_controls() which can loop over all of the registered partials and to get their associated settings, and if an setting has refresh as its transport, then it could be overridden to postMessage.

@dlh
11 days ago

#2 @dlh
11 days ago

  • Keywords has-patch needs-testing needs-unit-tests added; needs-patch removed
  • Milestone changed from Future Release to 5.4
  • Version set to 4.5

I've been tripped up by not using postMessage with my selectively refreshed settings many times. It'd be great if core could provide a safety net here.

39487.diff attempts to implements the logic described in comment:1.

The logic is added to the \WP_Customize_Selective_Refresh class in part because it's related to selective refresh and part to avoid adding more methods to \WP_Customize_Manager if they don't strictly have to be. The added check_transport_of_settings_with_partials() method is called via hooks to allow developers to disable it consistently in the admin and the preview.

As I was working on this patch, I learned that there's precedent for a theme to register a partial for a setting but intentionally leave the setting transport as refresh unless some condition is met. Core itself uses this pattern with the header_image setting, which is changed to postMessage in \WP_Customize_Manager::register_controls() depending the on theme support for custom-header.

The patch would also change the transport for these settings, perhaps unexpectedly. Luckily, if the partial keeps $fallback_refresh enabled, then a full refresh of the preview will continue to occur if the partial refresh fails. Accordingly, the patch doesn't modify settings associated with partials that disable $fallback_refresh.

I'll work on unit tests, but I'd love for anyone to take the patch for a spin to try to get something into the next release!

Last edited 11 days ago by dlh (previous) (diff)

@dlh
9 days ago

#3 @dlh
9 days ago

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

39487.2.diff adds tests.

Note: See TracTickets for help on using tickets.