ja1984 / jackbox

Javascript library to display notifications
https://ja1984.github.io/jackbox/
GNU General Public License v3.0
17 stars 6 forks source link

Add retry button with callback #7

Open ja1984 opened 7 years ago

ja1984 commented 7 years ago

A notification should be able to have an action, something like retry.

Action and title should be optional and replace the dismiss button if present

alxlark commented 7 years ago

Can you provide more infomation what retry should do?

ja1984 commented 7 years ago

What it does should be optional, i see a third or forth parameter for all the methods. Something like

Jackbox.error("message",{settings},"text on button",callback);

What I'm thinking is that if you display an error message, maybe you would like to give the user the option to retry the action (s)he just did.

And success message maybe displays an button that will link to an post that was just created.

alxlark commented 7 years ago

Ok, I got the idea, but now as I can see lib has follow interface

Jackbox.error("message");

but your example a bit differend

Jackbox.error("message",{settings},"text on button",callback);

So need change interface to new, or just add callback at the end, like?

Jackbox.error("message", callback);
ja1984 commented 7 years ago

I'm sorry you are correct.

We probably need an second parameter for the text on the button that will get added.

Jackbox.error("message","button text",callback)

This should be available for all notifications not only error :)

alxlark commented 7 years ago

so, we do next case 1: if 2nd and 3d params present we add button with this text near delete icon, yes? and if user click on this button we run callback, correct? case 2: or we not add any button and run callback on finish progres or user click?

ja1984 commented 7 years ago

Case 1 is spot on!

Not sure about case 2 thou, having a callback on click might be confusing to user, no clear way of showing that the notification is clickable?

And if we implement case 2 maybe we should have that as an object somethink like

Jackbox.error("message","button text",{trigger: 'click/finish', buttonText: '' callback: callback}) ?

I just thought of something, if you're going to change the parameters maybe add an second parameters for settings. If we decide to implement per notification settings so we do not need to change parameters later :)

Jackbox.error("message",{},"button text",callback)

Does this make sense?

Fredrik-Oberg commented 7 years ago

Proposal 1 I suggest that we instead do the button text a part of the settings

var notificationSettings = {
   time: 5, 
   classNames: "custom-class-name",
   buttonText: "button"
}
Jackbox.error("message", notificationSettings, callback);

The settings is then optional and we will not be dependent on a button text if we want to add an callback. It's easy to add new feature (maybe custom icons). Relates to issue #11

Proposal 2 Use Promises instead of callback. Make use of the ES6 Promises feature. This will allow for a better callback handling. Currently supported in all major updated browsers, but lack support for IE(8-11) http://caniuse.com/#feat=promises

There 3d-party libs that can polyfill the feature for IE, such as https://github.com/taylorhakes/promise-polyfill

The usage will then be Jackbox.error("message").then(callback);

ja1984 commented 7 years ago

@Fredrik-Oberg Making the buttonText part of settings sound like a good idea, then we could have a global setting for it as well.

Is it possible to get the promise to trigger on the action button only so that it doesn't trigger unless the user actively want it to?