Remember preview url query params #129
Conversation
PatelUtkarsh
changed the title
mohdsayed
changed the title
|
Ready for review. I am not sure if |
js/compat/customize-snapshots.js
Outdated
| var retval = originalQuery.apply( this, arguments ); | ||
|
|
||
| if ( 'undefined' !== typeof CustomizerBrowserHistory ) { | ||
| retval.customize_preview_url_query_vars = JSON.stringify( CustomizerBrowserHistory.getQueryParams( location.href ) ); |
There was a problem hiding this comment.
I don't think that Customizer Browser History should be made a dependency for this feature.
This can be replaced with:
retval.customize_preview_url_query_vars = wp.customize.utils.parseQueryString( location.search.substr( 1 ) );though on 4.6 a polyfill for wp.customize.utils.parseQueryString would be needed.
js/customize-snapshots.js
Outdated
| var retval = originalQuery.apply( this, arguments ); | ||
|
|
||
| if ( 'undefined' !== typeof CustomizerBrowserHistory ) { | ||
| retval.customize_preview_url_query_vars = JSON.stringify( CustomizerBrowserHistory.getQueryParams( location.href ) ); |
| $allowed_devices = array( | ||
| 'mobile', | ||
| 'desktop', | ||
| 'tablet', |
There was a problem hiding this comment.
This list needn't be hard-coded. You can use \WP_Customize_Manager::get_previewable_devices().
| ( | ||
| in_array( $param, $allowed_panel_section_control_params ) | ||
| && | ||
| preg_match( '/[a-z|\[|\]|_|-|0-9]+/', $value ) |
There was a problem hiding this comment.
This regex seems to be missing the initial ^( and ending )$ if it is doing what I think.
There was a problem hiding this comment.
The panel/sections/control ids can be like
post[post][1437]
title_tagline
blogname
nav_menu-61
postmeta[posttype][27450][keywords]
It doesn't follow any initial or ending pattern?
There was a problem hiding this comment.
Isn't the purpose to validate that the $param is a valid ID string? If you want to ensure that only the valid characters are included then you should do:
preg_match( '/^[a-z|\[|\]|_|-|0-9]+$/', $value )Adding the ^ and $ anchors, as otherwise a user could supply an ID like: !@#$%^&*()title_tagline⁄€‹›fifl‡°·‚
There was a problem hiding this comment.
@westonruter I think I am not able to get it, Can you see this http://regexr.com/3ffv4 ?
There was a problem hiding this comment.
I should probably be \[a-z|\[|\]|_|\-|0-9]+\ ( escaped - sign ) but /^[a-z|\[|\]|_|-|0-9]+$/ doesn't match anything.
There was a problem hiding this comment.
@sayedtaqui from that example, you can see that it is matching all of those strings, the last one incorrect.
Compare:
and
| add_filter( 'removable_query_args', array( $this, 'filter_removable_query_args' ) ); | ||
| add_action( 'save_post_customize_changeset', array( $this, 'create_initial_changeset_revision' ) ); | ||
| add_action( 'save_post_' . $this->get_post_type(), array( $this, 'create_initial_changeset_revision' ) ); | ||
| add_action( 'save_post_' . $this->get_post_type() , array( $this, 'save_customize_preview_url_query_vars' ) ); |
There was a problem hiding this comment.
There's an extra space before the comment here.
php/class-post-type.php
Outdated
| $preview_url_query_vars = array(); | ||
|
|
||
| // If customizer browser history plugin is active. | ||
| if ( function_exists( 'customizer_browser_history_enqueue_scripts' ) && $post_id ) { |
There was a problem hiding this comment.
Per above, I think Customizer Browser History shouldn't be made a dependency.
php/class-post-type.php
Outdated
| $preview_url_query_vars = array(); | ||
|
|
||
| if ( $post_id ) { | ||
| $preview_url_query_vars = (array) get_post_meta( $post_id, '_preview_url_query_vars', true ); |
There was a problem hiding this comment.
If there is no postmeta on the changeset post, where get_post_meta() returns false, this will result in $preview_url_query_vars === array( false ).
So this should probably be:
$preview_url_query_vars = get_post_meta( $post_id, '_preview_url_query_vars', true );
if ( ! is_array( $preview_url_query_vars ) ) {
$preview_url_query_vars = array();
}
|
Not sure why the build is failing. |
@sayedtaqui it looks like an issue with PHPUnit 6.0. I'm noticing the same thing locally after upgrading from 5.x. See also WordPress/WordPress-Coding-Standards#870 (comment) See also:
|
westonruter
force-pushed
the
enhancement/remember-preview-url
branch
from
cccae03 to
83dc09f
Compare
westonruter
force-pushed
the
enhancement/remember-preview-url
branch
from
83dc09f to
4caa963
Compare
| add_action( 'wp_before_admin_bar_render', array( $this, 'print_admin_bar_styles' ) ); | ||
| add_filter( 'removable_query_args', array( $this, 'filter_removable_query_args' ) ); | ||
| add_action( 'save_post_customize_changeset', array( $this, 'create_initial_changeset_revision' ) ); | ||
| add_action( 'save_post_' . $this->get_post_type(), array( $this, 'create_initial_changeset_revision' ) ); |
… for Customize admin bar link
|
@sayedtaqui Ready for merge after you give a final review of my tweaks. |
|
👍 Lets merge. |
3 participants
Fixes #107