jpillora / notifyjs

Notify.js - A simple, versatile notification library
https://notifyjs.jpillora.com/
MIT License
1.91k stars 741 forks source link

Source code formatting, more jQuery, IE 7 support and close button #13

Closed LessWrong closed 10 years ago

LessWrong commented 10 years ago

Hi, Jaime.

Thanks you for this lib, I'm using it for effective displaying AJAX notifications in my little corporative apps. In my work I'm using mercurial and I don't understand how send a git pull request right now.

So, my issue in free form:

1) I'm got the lib source code and saw that some variables have a very short and senseless names (w, _ref1, d, etc.). Yes, this is easy to write - but hard to read, understand and change. No need to compress the source code in this way - it's minify-tools task.

2) Because you use the jQuery, why not get a source code like a standart jQuery-plugin?

3) My users are still using older browsers. I know, I know - it's their problems, but I'm not a google - I must to maintain compatibility with more than two latest browser versions.

So, for IE 7 I write "notifyjs-ie7.css" (without this, notifies is not shown), something like:

.notifyjs-container
{           
    display:inline-block;
    zoom: 1;
    *display: inline;
}

.notifyjs-corner .notifyjs-wrapper 
{
    height: 100%;
    width: 100%;
}

Do you plan older browsers support?

4) And what about the close button/link? Now, if user click on notify container (clickToHide option is true) - notify will be closed. User can't copy notify message if needed.

I have no problem to implement its queries, but maybe you're already planning on any of this?

Thanks in advance.

jpillora commented 10 years ago

1) The files in dist/ are the files for distribution. The source code (the "src" folder) is in CoffeeScript - src/notify.coffee. What you're looking at is the compiled source.

2) $.fn[pluginName] = function() {...} is all a jQuery plugin is. What's your definition of a "standard jQuery plugin"

3) I can support IE8+ though I don't have access to an IE7 VM. However, I will accept PRs adding IE7 support :)

4) If you'd like the text to be selectable, then you'll need to clickToHide:false and then trigger elem.trigger('notify-hide'); on the elem on mouseUp.