New Option: add additional css class(es) to close link

[christiannaths]

I know your intentions are to keep this as absolutely simple as possible, but I'd love the ability to add a custom class to the close button.

This way custom CSS can be written to override the default look for the close button, including the ability to utilize icon fonts. This is especially handy if they are already being used in the project at hand (which, for me, nowadays is almost always.)

This pull represents the simplest implementation of this feature that I could think of, leaving the styles completely up to the user.

An example has been added to the demo, and the README has been updated as well.

[kylefox]

Interesting. Why not just target the element using the .modal a.close-modal selector?

[christiannaths]

Well, my thought was by using a plugin option the modal instances could be extended in two ways: per-instance by passing that option to a single modal, or universally by setting options once:

$.modal({
    closeClass: 'my-custom-class'
});

For me it feels much cleaner than the alternative:

$(document).on($.modal.BEFORE_OPEN, function(event, modal){
    $('.modal a.close-modal').addClass('my-custom-class');
});

Unless I completely missed something and there is a cleaner way to do this without adding this new option (which is a distinct possibility).

[christiannaths]

I just noticed a huge (and stupid) mistake in this fork too. I inadvertently doubled up the minified JS code. :S Still curious what you think of this addition, however, it's probably best not to pull this until I get my act together, haha.

[kylefox]

There's an option to change the modal's root class (defaults to "modal"):

$.modal({
    modalClass: "modal-with-icon'
});

And then in your CSS:

.modal-with-icon a.close-modal {
    /* Icon font styles go here. */
}

Would something like that work?

I guess what I'm missing is why the class of the actual close element needs to be changed.

[christiannaths]

It's because I'd like to stick to the class naming convention the icon font uses (currently in my case Fontawesome), since I use it everywhere else in my project. It results in fewer lines CSS, and fewer places to update it if I ever choose to swap out the icons. It occurred to me there are probably other (similar) uses for this option as well.

Of course, if this pull doesn't fit your project I'll just continue using my fork :)

[ghostrydr]

I was just about to make the same request but it seems someone has beat me to it... lol. I'm with Christian though. By simply adding a close class you can use easily use icon fonts (I use Fontello) without having to redeclare all the styles for that icon.

[kylefox]

Yeah, I've warmed up to this idea!

@christiannaths It looks like your pull request can no longer be automatically merged, but I'm totally down to merge if you have a peek at the conflicts ;)

[christiannaths]

Sure, I'll have a look

[christiannaths]

You can try merging this now, the latest commit includes the up-to-date repo along with my original changes. I was able to successfully merge my fork with the original locally.

[kylefox]

Awesome, thanks man!