Plugin for WordPress and Foundation Reveal

I figure that someone here probably knows a much better way to do this. I'm still figuring out AJAX and jQuery, so I consider that I still need to master quite a bit of knowledge.

My function extends Foundation 4 Reveal's functionality in a few ways:

• Uses WordPress AJAX to dynamically pull in content to populate a modal div.

• Centers the modal div and allows it to be variable width.

• Adds paging navigation from the modal window that when triggered will close the open modal window, then open the previous/next modal content.

While I feel like I've managed to figure out a lot on my own, my code isn't optimal; I would greatly appreciate any feedback or insights on ways to improve it, or things that I should avoid.

```
(function($) {

$.fn.displayPost = function() {

event.preventDefault();

var post_id = $(this).data("id");
var id = "#" + post_id;

// Check if the reveal modal for the specific post id doesn't already exist by checking for it's length
if($(id).length == 0 ) {
// We'll add an ID to the new reveal modal; we'll use that same ID to check if it exists in the future.
var modal = $('').attr('id', post_id ).addClass('reveal-modal').appendTo('body');
var ajaxURL = MyAjax.ajaxurl;
$.ajax({
type: 'POST',
url: ajaxURL,
data: {"action": "load-content", post_id: post_id },
success: function(response) {
modal.empty().html(response).append('×').foundation('reveal', 'open');
modal.bind('opened', function() {
// Reset visibility to hidden and set display: none on closed reveal-modal divs, for some reason not working by default when reveal close is triggered on .secondary links
$(".reveal-modal:not('.reveal-modal.open')").css({'visibility': 'hidden', 'display' : 'none'})
// Trigger resize
$(window).trigger('resize');
return false;
});
}
});
}
//If the div with the ID already exists just o

-
L15, L26, L31-60: Inconsistent indentation levels.

-
L4-43: Consider indenting this block.

-
L04: I don't see the point in storing the ID rather than the jQuery object itself. Instead of var id = "#" + post_id;, go ahead and store the object: var $id = $("#" + post_id);. Modify the references as appropriate on L11 and L33.

-
L13: Your code for appending the modal is nice in terms of the builder pattern, but it might be better just as:

var modal = $("").appendTo("body")

-
L14: There's no point in declaring the ajaxURL variable. Instead, just go ahead and use { ... url: MyAjax.ajaxurl, ... } on L17,

-
L20: The jQuery API documentation on .html() notes a specific case in which it's important to use .empty().html(). I don't believe this applies in your case; you don't need the call to .empty() since .html() will effectively empty the container first.

-
L21: Since jQuery 1.7, .on() is preferred.

-
L58-59: This part is at best senseless and wrong, and at worst unintuitive and potentially deceptive. By extracting the ID attribute and using it as a selector, you're effectively matching against the ID without the #. This either means that it doesn't do what you think it does, in which case you should use:

jQuery(this).closest("div").foundation("reveal", "close");

or you for some reason have a construct like ` and you really mean to match all span elements instead of #span`. In that case, I strongly suggest you restructure your HTML to be more semantically sensible, and to leave a comment explicitly detailing that that's your intention.

StackExchange Code Review Q#27779, answer score: 2