icecoder / ICEcoder

Browser code editor awesomeness
http://icecoder.net
Other
1.41k stars 347 forks source link

Consider better alerts/confirms/prompts #580

Open mattpass opened 9 years ago

mattpass commented 9 years ago

Rather than use the browser dialogs, consider using maybe SweetAlert http://t4t5.github.io/sweetalert/

Will need to ensure that this is a simple drop in replacement and won't have any processing issues as these browser based dialogs are blocking.

ssimono commented 8 years ago

+1 for better alerts. However I really doubt that this will be a drop in replacement since sweetalert won't block the script execution. Anything that is currently waiting for user confirm to proceed will have to be placed in a callback function...

I still think this is a necessary refactoring though

mattpass commented 8 years ago

@ssimono agreed. Looked into this and it's not going to block further JS from happening without callbacks, which as you say, means refactoring.

Would love this improvement but it's a fair bit of work, especially as we need to cover alert, confirm and prompt calls. :-/

Kasara commented 7 years ago

Maybe this is the right for you?

https://github.com/hubspot/vex

vex.dialog.alert('Hello world!')

mattpass commented 7 years ago

It looks very nice @Kasara - however, it would I suspect suffer from the same issue as others and mean a lot of refactoring.

eg, let's say I have current code:

alert('Press OK to go to Google');
window.location = 'http://google.com';

I know that the alert function will cause the browser to show it's native alert message and it will go no further till you press the button, then it'll go to the Google site.

I don't believe I can simply replace the alert line with Vex's version:

vex.dialog.alert('Press OK to go to Google');
window.location = 'http://google.com';

While I haven't tried any of the above, I'd suspect it'd display a very nice looking message on screen for a millisecond, then redirect to the Google site. This is different behaviour to the above.

This then means I need to refactor quite a lot of areas to take into account this different behaviour.

mattpass commented 7 years ago

Yeah, just as I thought. Tested the above and it does just that - displays a the fake alert message and continues to go to the Google site straight away.

demolitions commented 7 years ago

If you'd like a hand, I'll try tackling this in the weekend. Is either proposed lib ok with you, or do you have any preference?

mattpass commented 7 years ago

@demolitions A hand would always be good in covering issues. Out of the 2 libs I'd only really consider Sweet Alert. If you can get a demo of it working that'd be ace but suspect this may all be trickier to implement that it seems.

demolitions commented 7 years ago

Take a look, I added sweetAlert and converted only the ICEcoder.message() calls for now, you can pull a working version from my fork, 46379aca72753616559306b27f51443115ff132f You were right about it being tricky, I started converting the ICEcoder.ask() calls and it's not ready to be moved to an asynchronous model. It'll be a longer work than I expected, but stay tuned. (btw, I minified swettAlert.css since it's distributed as non-minified file, I thought that you might like loading a 16KB file instead of a 22KB one)

mattpass commented 7 years ago

@demolitions Oh man, it is looking pretty sweet! May need to tweak the CSS a little to make it tie in with ICEcoders look, but far nicer than default browser dialogs. If you can nail the async side of ICEcoders 3 dialog functions - message, ask and getInput, we can certainly move to this - looking great.

Also, thanks v much for minifying sweetalert.css - yes, I'm the kind of person who wants to make every byte count and justify being there, so appreciated. :-)

Look forward to more you do here and maybe a near finished version one day!