lavigor / notifications

Web Push Notifications come to phpBB boards!
GNU General Public License v2.0
7 stars 6 forks source link

introConfirmation switch does not work #10

Open obagley opened 6 years ago

obagley commented 6 years ago

Hello,

Thanks for the great extension, I hope it will be released soon!

In the meantime - a minor defect for your consideration. Currently the modal dialog switch doesn't work, the intro is shown regardless of the switch state. I debugged it briefly, the problem is that the localStorage.pushConfirmation is not initialized outside of the showIntroConfirmation() function.

Therefore on the first execution this check if (pushNotifications.introConfirmation && !localStorage.pushConfirmation) is working with pushNotifications.introConfirmation = 0 (correctly passed from the config) and localStorage.pushConfirmation = undefined (wrong, should be initialized to 0 somewhere).

I'm not an expert in java script, seems a bit funny to me - when the IF condition is not defined, the IF statement is executed.

Should be easy enough to fix.

lavigor commented 6 years ago

@obagley localStorage.pushConfirmation = undefined is absolutely OK for JS. Not everything needs an initialization in this language. This is indeed the default value for anything.

is working with pushNotifications.introConfirmation = 0

Hmm, it seems that if ("0") is true in JS. I'll look into it when I have time, thank you for reporting the issue.

obagley commented 6 years ago

Thanks for the prompt response. I googled it, things are even weirder than this, see https://stackoverflow.com/questions/10275119/javascript-booleans-false-true-results-in-true:

I highlighted what I think is relevant in bold

if you are trying to calculate with logic operators, remember:

var result = true && "false";// always results (string) "false" var result = true && "true";// always results (string) "true" var result = false && "true";// always results (boolean) false var result = false && "false";// always results (boolean) false var result = "true" && true;// always results (boolean) true var result = "true" && false;// always results (boolean) false > var result = "false" && true;// always results (boolean) true var result = "false" && false;// always results (boolean) false var result = "true" && "true";// always results (string)"true" var result = "true" && "false";// always results (string) "false" var result = "false" && "true";// always results (string) "true" var result = "false" && "false";// always results (string) "false"

Looking forward to the official release!