justindomingue / ohsnap Goto Github PK

Hi!
First: There is a typing error in README.md in section Usage (osSnap) which causes my first test to fail with an error...

osSnap('Succefully created your account');

should be

ohSnap('Succefully created your account');

Second: The is an error in the Javascript ohsnap.js (line 45)
First appearance of variable html without var.
My Test: I have called ohSnap(..) two times one after another and not all notifications faded out after five seconds because the variable html is global without var in front of.
With var there is one variable html for each call of ohSnap(..) which is used later in setTimeout(ohSnapX..).

// Generate the HTML
html = $('<div class="alert ' + color_markup + '">' + icon_markup + text + '</div>').fadeIn(options['fade-duration']);

should be

// Generate the HTML
var html = $('<div class="alert ' + color_markup + '">' + icon_markup + text + '</div>').fadeIn(options['fade-duration']);

Best
Martin

Sorry for my bad english!

When a notification is called more than once, without interruption,(for example 7 in a row) you get notifications that don't fade out, they stay there sticked, only when you click them they disappear. (sorry for my english)

When using a large page with scrolldown, the output is not on the bottom of the screen.

Hello,

since you were using raw.github.com, the js is being served as
text/plain, so google chrome doesn't execute it.

To fix this, replace http://raw.github.com for http://rawgithub.com ( that's right, with no dot between raw and github )

Thanks

As far as I can see there's no option to change the default ".alert" class for a single message.

It would be useful to add an option to modify this class, since I'm actually working with bootstrap and customization of the ".alert" class also overrides the bootstrap alert boxes styling.

I would make the ".alert" class the default option if the user does not define the class.

.live is deprecated in jQuery 2+...

I've been able to replace it with (line 44)

$(document).on('click', '.alert', function() { 
  ohSnapX($(this));
});

which is supported 1.7+. Works for me but is not fully tested.

If a notification pops up while there is already one up, it fails to fade away. This happens regardless of whether a time property is given. Is this fixable?

Have you considered what license you have this code under ?

Specifying current version number for bower.json and feature update

i'm showing many snaps and i need to call some func between each one to clear the prev snaps if still in view.