patternjavascriptMinor
First Wordpress plugin for saving a post
Viewed 0 times
wordpresspostpluginfirstforsaving
Problem
This is my first Wordpress plugin. I would love some input from the WP/PHP/JavaScript gurus out there. It's a simple plugin that saves a post using ctrl-Q and opens a preview in a new or current window.
JavaScript:
```
jQuery(document).ready(function(){
if (document.cookie.indexOf("previewCookie") >= 0){
//expires added for IE
document.cookie="previewCookie=true; max-age=0;expires=0;path=/wp-admin/";
//quickPreviewOption is set in quick-preview.php
var previewURL = document.getElementById('post-preview');
if(quickPreviewOption === 'current'){
window.location = previewURL;
}
if(quickPreviewOption === 'new'){
window.open(previewURL,"wp_PostPreview","","true");
}
}
jQuery(document).keydown(function(e){
if((e.ctrlKey || e.metaKey) && e.which == 81){
//Find #save post if it's a draft. If post is published, #save-post doesn't exist.
if(jQuery('#save-post').length){
jQuery('#save-post').click();
}
else if(jQuery('publish').length){
jQuery('#publish').click();
}
Choose whether you would like the post preview to open in a new window or the current window.';
}
function plugin_setting_string() {
$options = get_option('quick_preview_options');?>
/> Current Window
/> New WindowIf popups are disabled you will need to add your site to the exception list in your broswer. trim($input['window_preference']));
if( $newinput['window_preference'] != "new" && $newinput['window_preference'] != "current" ) {
$newinput[ 'window_preference' ] = "";
}
return $newinput;
}
function qp_options_page() { ?>
Quick Preview Options
" />
JavaScript:
```
jQuery(document).ready(function(){
if (document.cookie.indexOf("previewCookie") >= 0){
//expires added for IE
document.cookie="previewCookie=true; max-age=0;expires=0;path=/wp-admin/";
//quickPreviewOption is set in quick-preview.php
var previewURL = document.getElementById('post-preview');
if(quickPreviewOption === 'current'){
window.location = previewURL;
}
if(quickPreviewOption === 'new'){
window.open(previewURL,"wp_PostPreview","","true");
}
}
jQuery(document).keydown(function(e){
if((e.ctrlKey || e.metaKey) && e.which == 81){
//Find #save post if it's a draft. If post is published, #save-post doesn't exist.
if(jQuery('#save-post').length){
jQuery('#save-post').click();
}
else if(jQuery('publish').length){
jQuery('#publish').click();
}
Solution
Well, I can't help you with the WordPress specifics, or the jquery, which means not much at all really. But here are a few things that might help you with your PHP.
Foremost, I would definitely consider separating your HTML from your logic. Or at the very least separate your escapes from PHP so that your PHP does not run into your HTML. It makes for much cleaner code that is also much easier to adapt and reuse. You can also take a look at heredocs and includes as another possible solution.
You should also make sure you declare a variable before using it.
Since we are looking at this anyways, here's a couple of minor things. First, the absolute equality comparison
But then, a better way to write this would be to make this the default value and only assign the new one if necessary. This makes the code lighter and much easier to read.
Hopefully this helps, sorry I couldn't answer any more specific concerns.
Foremost, I would definitely consider separating your HTML from your logic. Or at the very least separate your escapes from PHP so that your PHP does not run into your HTML. It makes for much cleaner code that is also much easier to adapt and reuse. You can also take a look at heredocs and includes as another possible solution.
//proper escaping
?>
HTML
<?php//notice not on the same line as the HTML
//heredoc
echo <<<HTML
Long
bit
of
HTML
maybe even with some $php_variables though no statements
HTML;//must not be indented, even if current line is 5 tabs in, this must be all the way to the leftYou should also make sure you declare a variable before using it.
$newinput came out of nowhere. Is it a global? Assuming for now that its not, you should declare $newinput as an array before manipulating it. This insures that $newinput definitely doesn't have any previous values, whether from a global state or some other fluke. This means no accidental appending onto another array, and no out of bounds warnings from accidentally treating a string as an array. So:$newinput = array(
'window_preference' => trim( $input[ 'window_preference' ] )
);Since we are looking at this anyways, here's a couple of minor things. First, the absolute equality comparison
=== is usually only used when type is a factor. For example, TRUE === TRUE but "TRUE" !== TRUE. If we use the loose comparison ==, it accomplishes the same thing and allows for the type conversion, meaning that second comparison is TRUE as well. With strings this usually isn't a factor unless comparing a false string (empty, 0, "null", "false"). Second, I believe you have comparisons confused with assignments. The body of your if and else statements don't accomplish anything. If we were to read them, it would read like so, "if( TRUE ) { TRUE; } else { TRUE; }". Which makes no sense. If instead you meant to assign the value of$newinput[ 'window_preference' ]` to itself, you needn't bother, that's redundant. If all you want to do is remove the value from the array element given the opposite of that if statement, then you can alter the if statement slightly and use an actual assignment operator like so.if( $newinput[ 'window_preference' ] != 'new' && $newinput[ 'window_preference' ] != 'current' ) {
$newinput[ 'window_preference' ] = '';
}But then, a better way to write this would be to make this the default value and only assign the new one if necessary. This makes the code lighter and much easier to read.
$newinput = array(
'window_preference' => ''
);
$preference = trim( $input[ 'window_preference' ] );
if( $preference == 'new' || $preference == 'current' ) {
$newinput[ 'window_preference' ] = $preference;
}Hopefully this helps, sorry I couldn't answer any more specific concerns.
Code Snippets
//proper escaping
?>
HTML
<?php//notice not on the same line as the HTML
//heredoc
echo <<<HTML
Long
bit
of
HTML
maybe even with some $php_variables though no statements
HTML;//must not be indented, even if current line is 5 tabs in, this must be all the way to the left$newinput = array(
'window_preference' => trim( $input[ 'window_preference' ] )
);if( $newinput[ 'window_preference' ] != 'new' && $newinput[ 'window_preference' ] != 'current' ) {
$newinput[ 'window_preference' ] = '';
}$newinput = array(
'window_preference' => ''
);
$preference = trim( $input[ 'window_preference' ] );
if( $preference == 'new' || $preference == 'current' ) {
$newinput[ 'window_preference' ] = $preference;
}Context
StackExchange Code Review Q#15752, answer score: 5
Revisions (0)
No revisions yet.