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

Customize Transactions #61

Closed
wants to merge 109 commits into
from

Conversation

Projects
None yet
3 participants
@westonruter
Member

westonruter commented Jan 6, 2015

_README: Customizer Transactions Proposal_

Next steps:

  • Merge in #62 after committed to Core
  • Merge in #67 after committed to Core
  • Break out phpDoc and code style improvements into separate patch

Trac tickets addressed by (or related to) this PR:

  • #30937: Add Customizer transactions. Read writeup here.
  • #30028: Load Customizer preview iframe with natural URL
  • #30936: Dynamically create WP_Customize_Settings for settings created on JS client
  • #27355: Customizer: Add framework for partial preview refreshes
  • #29988: Twenty Fifteen: Use JS/postMessage to update the color scheme instead of triggering a page refresh
  • #20714: Theme customizer: Impossible to preview a search results page
  • #23225: Customizer is Incompatible with jQuery UI Tabs.
  • #29098: Send JSON success for customize_save and allow response to be filtered
  • #30448: Customizer Manager/Settings classes do not provide public save method to use outside of customize.php
  • #28721: Scheduled changes for the customizer
  • #29932: There is no error reporting in the Customizer. (Whenever a setting is changed, the transaction gets updated. This transaction update Ajax request can send back information about failures, including PHP-based sanitization of values beyond sanitization logic copied to JS.)
  • #28536: Add browser history and deep linking for navigation in Customizer preview. (This is related as we can now deep-link to Customizer previews with settings from a transaction applied persistently.)
  • #28602: Transparently route frontend browsing through Customizer for logged-in editors. (With partial refresh realized, we can load Customizer Controls into the frontend itself. And with transactions, we can safely reload the entire page to persist the state.)
  • Merge #30988 for the new preview logic and WP_Customize_Setting::value() usage.
  • #29316 The WP_Customize_Setting::preview() method should also by default add a priority>10 filter for the customize_value_* filter there, but this is dependent on having apply_filters( "customize_value_{$this->id}", $this->default, $this); or apply_filters( "customize_value_{$this->type}", $this->default, $this); in place, so that we can add this filter. Without this, you will not get the previewed value for a setting when you call WP_Customize_Setting::value() on a custom type.

TODO

  • Allow settings to be dynamically created at runtime.
  • No need for a login failure message because not ajax preview. The heartbeat will continually push the iframe's current URL and the current nonces.
  • Don't allow links with target=_blank or target=_top attrs?
  • Must include partial refresh as key aspect of redactor since full refresh is not seamless
  • Provide way of restoring a draft customizer transaction
  • Lay groundwork for inline partial refresh controls: autoloading Customizer on every logged-in page. By eliminating iframe entirely we don't need the refresh transport at all. Customizer can be loaded on every page.
  • Customizer preview should have a heartbeat and if not sent present a link to go home. Heartbeat is what will alert a user if they disconnect from Customizer and provide a URL to go back. If logged in always load the customizer preview JS so that heartbeat can run.
  • Introduce customize transaction class for managing saving of settings
  • Every customizer transaction is a draft until saved, when it becomes published. Future dated is possible too.
  • When viewing preview outside of iframe, allow admin bar to appear
  • Anyone can now preview. Only cap is to update and save
  • We need a uuid for the transaction generated upon loading. And update URL with ID once first update.
  • When loading customizer for first time the transaction ID needs to be added via replaceState after first update.
  • After saving a transaction, return a new transaction UUID for staring a new transaction.
  • Prevent prevent modifying published transactions. As soon as it is published, it becomes locked. If a published transaction is loaded in the Customizer, then its settings should be copied into a new transaction.
  • If a transaction is sharer, then it should have a private status and so not auto deleted. Draft transactions should be deleted if not modified since X.
  • If someone has a link to a customizer transaction then any settings should apply even if no cap to update with new ones.
  • Eliminate preview nonces. We only need update and save nonces.
  • We don't need Messenger channels anymore, do we?
  • Debounce updating a transaction.
  • Merge https://core.trac.wordpress.org/ticket/29098
  • Persist query vars if transaction UUID supplied
  • When the transaction is updated we can send back the php sanitized value into js
  • Send no cache headers and robots blocking if transaction uuid supplies
  • Eliminate separate theme query param. This can be included with the transaction. See https://core.trac.wordpress.org/ticket/22880#comment:6
  • Add support for publishing transaction posts, which causes the settings to save()
  • When admin bar is shown when customizing, there should only be one node, one to open the page in the customize.php with the transaction loaded.
  • If the user is not authorized when landing on customize.php and there is a UUID supplied, then redirect to the permalink (frontend) instead?
  • If a wp_transaction post gets created without a post_name supplied, we need to force it to be a new UUID.
  • Add previewing for Ajax requests.
  • Add permalink button and button to share the preview with someone else.
  • Allow preview to be viewed without being logged in.
  • If a setting gets deleted on the client, then null should be sent for that setting ID when the transaction is updated so it can be purged from the drafted transaction post.
  • Does the json key/value blob in post_content work with unfiltered_html = false? (from @ocean90)
  • Investigate replacing UUID with using a regular wp_transaction post ID. This would require creating an empty wp_transaction post whenever opening the Customizer, so that the Customizer preview URL can have this ID added to a customize_transaction_id query param. Then when location.reload() is called in the preview, this ID will be retained and will load the transaction for previewing. Potentially use history.replaceState() in the preview frame, though this would not ensure that links in the preview also get the ID.

Regressions

  • There is a flash of unloaded content now when a preview window is reloaded. This is because document.write() is no longer used to replace the iframe window's document. Overall this is a good thing because it means that the Customizer preview behaves like a normal request, fixing several side-effects of the old method. But the flash of unloaded content when the preview is reloaded is unfortunate. The solution to this is to implement partial refresh, which will not only eliminate the flash of unloaded content but it will also drastically improve the speed of previewing non-postMessage changes.

WP-CLI

In its current state, you can create a scheduled transaction post to apply settings at a specific date, though it isn't very user-friendly:

wp post create \
    --post_type=wp_transaction \
    --post_name=$(cat /proc/sys/kernel/random/uuid) \
    --post_date_gmt="2016-01-01 00:00:00" \
    --post_status="future" \
    --post_content='{"blogname":"Welcome To The Future"}'

REST API

  • Provide REST API endpoints for mutating transactions.
  • Ensure that REST API calls may include customize_transaction_uuid to apply setting previews on API responses.

Future Ideas

  • You should be able to pop-out the preview iframe into a separate window, and then pop it back in. Likewise, you should be able to pop-open the Customizer pane when on any given frontend page.

westonruter added some commits Dec 20, 2014

Replace iframe written via document.write() with traditional iframe src
* Remove Customizer signature
* Use location.reload() instead of re-navigating to a URL
* Persist links in preview to retain Customizer context
* Send errors back to Customizer pane via postMessage (instead of Ajax)
* Move external link click interception into Customizer preview
* Remove keeping track of scroll position since location.reload() is used

Signed-off-by: Weston Ruter <weston@xwp.co>
Move all not-allowed checks to preview
* Add support for previewing form submissions
* Show not-allowed cursor on links and forms in preview which are not allowed.
Clean up customize-setting classes for coding standards, phpDoc
Conflicts:
	src/wp-admin/customize.php
	src/wp-includes/class-wp-customize-setting.php

git cherry-pick f79ef57
Obtain setting overrides from transaction instead of POST data
 * Make Customizer saved state dirty when transaction is a draft
 * Apply setting previews in Customizer pane (not just preview) when existing transaction is loaded
Introduce WP_Customize_Transaction class
 * Move methods and properties out of WP_Customize_Manager and into new class
 * Remove capability check when saving settings, since transaction's settings are cap-checked when updated.
 * Add $action param to WP_Customize_Manager::doing_ajax()
 * Deprecate post_data() methods in favor of transaction_data()
 * Register wp_transaction post type with other builtins
 * Show admin_bar if Customizer not loaded in iframe (initial stab)
 * Prevent calling setting->preview() if doing a customize_save
 * Store JSON as pretty-printed
Introduce dynamic (data-driven) settings and debounced transaction up…
…dates via wp.customize.utils.updateTransaction()

 * Register dynamic settings for Widgets up-front so that widgets can initialize properly without needing "prepreview" code, which is now also removed.
 * Add transaction back-compat for plugins expecting $_POST['customized']
Check capabilities for editing and publishing transactions
 * Map transaction caps to the 'customize' cap
 * Show "Submit for Review" instead of "Save & Publish" if user cannot publish_posts.
 * Update transaction post_status when saving, in addition to saving settings.
 * If user doesn't have publish_posts cap, then make transaction post pending.
 * Add public WP_Customize_Transaction::post() method
 * Configure permalink and edit_post_link for wp_transactions

westonruter added some commits Jan 6, 2015

Allow anonymous access to preview an extant transaction on the frontend
Add customize_preview_anonymous_access_allowed filter to configure behavior
Merge branch 'master' into customize-2.0
Conflicts:
	src/wp-includes/class-wp-customize-setting.php
Show outdated Hide outdated src/wp-includes/class-wp-customize-manager.php
if ( isset( $this->_post_values[ $setting->id ] ) )
return $setting->sanitize( $this->_post_values[ $setting->id ] );
_deprecated_function( __FUNCTION__, '0.4.2', 'WP_Customize_Manager::transaction::get()' );

This comment has been minimized.

@ocean90

ocean90 Jan 9, 2015

Contributor

Shouldn't __FUNCTION__ be __METHOD__?

@ocean90

ocean90 Jan 9, 2015

Contributor

Shouldn't __FUNCTION__ be __METHOD__?

This comment has been minimized.

@westonruter

westonruter Jan 9, 2015

Member

Thx. Fixed in 46f613c.

@westonruter

westonruter Jan 9, 2015

Member

Thx. Fixed in 46f613c.

Show outdated Hide outdated src/wp-includes/class-wp-customize-manager.php
if ( is_404() ) {
status_header( 200 );
}
_doing_it_wrong( __FUNCTION__, 'No longer used', '4.2.0' );

This comment has been minimized.

@ocean90

ocean90 Jan 9, 2015

Contributor

Should be _deprecated_function( __METHOD__, …

@ocean90

ocean90 Jan 9, 2015

Contributor

Should be _deprecated_function( __METHOD__, …

This comment has been minimized.

@westonruter

westonruter Jan 9, 2015

Member

Thx. Fixed in 46f613c.

@westonruter

westonruter Jan 9, 2015

Member

Thx. Fixed in 46f613c.

westonruter added some commits Feb 3, 2015

Merge branch 'master' into customize-2.0
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
	src/wp-includes/class-wp-customize-setting.php

@westonruter westonruter changed the title from Customize 2.0 to Customize Transactions Feb 3, 2015

Show outdated Hide outdated src/wp-includes/class-wp-customize-transaction.php
*/
class WP_Customize_Transaction {
const POST_TYPE = 'wp_transaction';

This comment has been minimized.

This comment has been minimized.

Show outdated Hide outdated src/wp-includes/class-wp-customize-transaction.php
'post_name' => $this->uuid,
'post_status' => $status,
'post_author' => get_current_user_id(),
'post_content' => $post_content,

This comment has been minimized.

@westonruter

westonruter Feb 4, 2015

Member

Use post_content_filtered instead. This will eliminate the need to base64-encode.

Via @nacin https://wordpress.slack.com/archives/core-customize/p1423086534000438

@westonruter

westonruter Feb 4, 2015

Member

Use post_content_filtered instead. This will eliminate the need to base64-encode.

Via @nacin https://wordpress.slack.com/archives/core-customize/p1423086534000438

This comment has been minimized.

Show outdated Hide outdated src/wp-includes/link-template.php
}
if ( 'wp_transaction' === $post->post_type ) {
$edit_post_link = add_query_arg( array( 'customize_transaction_uuid' => $post->post_name ), $edit_post_link );

This comment has been minimized.

@westonruter

westonruter Feb 4, 2015

Member

From @nacin:

I think I may prefer to see the stuff in link-template.php be added by a filter, but I don't know if we always load the manager in the contexts we need it.
Shoehorning stuff into get_edit_post_link() and what looks like get_permalink() feels like... shoehorning.
Minor note, and can go either way.

https://wordpress.slack.com/archives/core-customize/p1423086466000433

@westonruter

westonruter Feb 4, 2015

Member

From @nacin:

I think I may prefer to see the stuff in link-template.php be added by a filter, but I don't know if we always load the manager in the contexts we need it.
Shoehorning stuff into get_edit_post_link() and what looks like get_permalink() feels like... shoehorning.
Minor note, and can go either way.

https://wordpress.slack.com/archives/core-customize/p1423086466000433

Show outdated Hide outdated src/wp-includes/class-wp-customize-transaction.php
*
* @return string
*/
static public function generate_uuid() {

This comment has been minimized.

@westonruter

westonruter Feb 4, 2015

Member

From @nacin:

generate_uuid() is insane. :simple_smile: I wonder if our own rand functions may be better.
But no real complaint there, just still reviewing.

https://wordpress.slack.com/archives/core-customize/p1423086302000427

@westonruter

westonruter Feb 4, 2015

Member

From @nacin:

generate_uuid() is insane. :simple_smile: I wonder if our own rand functions may be better.
But no real complaint there, just still reviewing.

https://wordpress.slack.com/archives/core-customize/p1423086302000427

* @since 3.4.0
*/
public function customize_preview_signature() {
echo 'WP_CUSTOMIZER_SIGNATURE';
_doing_it_wrong( __FUNCTION__, 'Function no longer used.', '4.2.0' );

This comment has been minimized.

if ( value._dirty ) {
dirtyCustomized[ key ] = value();
}
} );

This comment has been minimized.

@westonruter

westonruter Feb 5, 2015

Member

Add api.trigger( 'updateTransaction', dirtyCustomized ) to allow the value to be manipulated?

@westonruter

westonruter Feb 5, 2015

Member

Add api.trigger( 'updateTransaction', dirtyCustomized ) to allow the value to be manipulated?

customize_transaction_uuid: api.settings.transaction.uuid
});
req.done( function () {

This comment has been minimized.

@westonruter

westonruter Feb 5, 2015

Member

Should the response have the sanitized JS values to push back into the settings?

@westonruter

westonruter Feb 5, 2015

Member

Should the response have the sanitized JS values to push back into the settings?

This comment has been minimized.

@kienstra

kienstra Feb 19, 2015

This would be good for usability. The user will see exactly what the page looks like after the sanitization.

The implementation could be similar to customize-controls.js, line 78:

$.each( response.sanitized(), function( key, value ) { 
           api.settings.settings[ key ] = value;
});
@kienstra

kienstra Feb 19, 2015

This would be good for usability. The user will see exactly what the page looks like after the sanitization.

The implementation could be similar to customize-controls.js, line 78:

$.each( response.sanitized(), function( key, value ) { 
           api.settings.settings[ key ] = value;
});

This comment has been minimized.

@westonruter

westonruter Feb 19, 2015

Member

Or rather, they would see in the controls what is the actual value being used in the preview. I think it would involve:

$.each( response.sanitized(), function( key, value ) { 
    var setting = api( key ), oldTransport = setting.transport;
    setting.transport = 'postMessage'; // so that we don't inadvertently cause infinite reload of preview, if JS sanitize != PHP sanitize
    setting.set( value );
    setting.transport = oldTransport;
});
@westonruter

westonruter Feb 19, 2015

Member

Or rather, they would see in the controls what is the actual value being used in the preview. I think it would involve:

$.each( response.sanitized(), function( key, value ) { 
    var setting = api( key ), oldTransport = setting.transport;
    setting.transport = 'postMessage'; // so that we don't inadvertently cause infinite reload of preview, if JS sanitize != PHP sanitize
    setting.set( value );
    setting.transport = oldTransport;
});
} else {
return $this->_post_values;
}
_deprecated_function( __METHOD__, '0.4.2', 'WP_Customize_Manager::transaction::data()' );

This comment has been minimized.

@westonruter

westonruter Feb 5, 2015

Member

Actually, 4.2.0

@westonruter

westonruter Feb 5, 2015

Member

Actually, 4.2.0

Show outdated Hide outdated src/wp-includes/class-wp-customize-manager.php
} else {
return $default;
}
_deprecated_function( __METHOD__, '0.4.2', 'WP_Customize_Manager::transaction::get()' );

This comment has been minimized.

@westonruter

westonruter Feb 5, 2015

Member

Actually, 4.2.0

@westonruter

westonruter Feb 5, 2015

Member

Actually, 4.2.0

westonruter and others added some commits Feb 5, 2015

Fix widget form from reverting to default state after editing newly-c…
…reated widget

Override the incoming $_POST['customized'] for a newly-created widget's setting with the new $instance so that the preview filter currently in place from WP_Customize_Setting::preview() will use this value instead of the default widget instance value (an empty array).

Add WP_Customize_Manager::set_post_value() to override incoming $_POST['customized'] data.
Merge branch 'master' into trac-30936-dynamic-settings
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
Merge branch 'master' into trac-30936-dynamic-settings
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
	tests/phpunit/tests/customize/widgets.php
Merge branch 'trac-30988' into customize-2.0
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
Merge branch 'trac-30936-dynamic-settings' into customize-2.0
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
	src/wp-includes/class-wp-customize-setting.php
	src/wp-includes/class-wp-customize-widgets.php
Merge commit r31421 into customize-2.0
Conflicts:
	src/wp-includes/class-wp-customize-manager.php
Show outdated Hide outdated src/wp-includes/class-wp-customize-transaction.php
* @access public
* @var string
*/
public $uuid;

This comment has been minimized.

@kienstra

kienstra Feb 19, 2015

Possibly create a get_uuid() method for WP_Customize_Transaction, instead of allowing public access to the $uuid property.
It seems that you've considered this, and maybe you want public access.
Several files access the $uuid property with transaction->uuid:

wp-admin/customize.php
wp-includes/theme.php
wp-includes/class-wp-customize-manager.php

But a get_uuid() method might prevent bugs, and make it easier to change.

@kienstra

kienstra Feb 19, 2015

Possibly create a get_uuid() method for WP_Customize_Transaction, instead of allowing public access to the $uuid property.
It seems that you've considered this, and maybe you want public access.
Several files access the $uuid property with transaction->uuid:

wp-admin/customize.php
wp-includes/theme.php
wp-includes/class-wp-customize-manager.php

But a get_uuid() method might prevent bugs, and make it easier to change.

*/
if ( ! isset( $this->data[ $setting_id ] ) ) {
// @todo Should this instead return $setting_obj->default? Or only if is_null( $default )?

This comment has been minimized.

@kienstra

kienstra Feb 20, 2015

I think it would be better to return $setting_obj->default only if is_null( $default )
It might be confusing to pass $default as an argument, but have a different default returned.

@kienstra

kienstra Feb 20, 2015

I think it would be better to return $setting_obj->default only if is_null( $default )
It might be confusing to pass $default as an argument, but have a different default returned.

westonruter added some commits Apr 21, 2016

Merge branch 'master' into customize-2.0
Conflicts:
	src/wp-admin/customize.php
	src/wp-admin/js/customize-controls.js
	src/wp-admin/js/customize-widgets.js
	src/wp-includes/class-wp-customize-manager.php
	src/wp-includes/class-wp-customize-setting.php
	src/wp-includes/class-wp-customize-widgets.php
	src/wp-includes/js/customize-preview.js
	src/wp-includes/link-template.php
	src/wp-includes/script-loader.php
	src/wp-includes/theme.php

[ci skip]
@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Oct 2, 2016

Member

Closing in favor of #161.

Member

westonruter commented Oct 2, 2016

Closing in favor of #161.

@westonruter westonruter closed this Oct 2, 2016

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