dobarkod / cookie-banner

JavaScript based cookie-info banner for complying with EU cookie law
MIT License
425 stars 84 forks source link

Now data-expires="" works as expected #72

Closed webaddicto closed 6 years ago

webaddicto commented 6 years ago

Seems that data-expires="90" or data-expires=90 is always identified as string so here is the fix:

If this.options.expires is a string but is a valid number:

data-expires="90"

It calculates today + 90 days and writes the expiration in cookie-friendly format.

And if it is a string (but not a number):

data-expires="Thursday, August 28, 2018, 1:52:31 AM"

It is written as is.

Tested and works fine like this:

data-expires="Thursday, August 28, 2018, 1:52:31 AM" or data-expires="90"

zytzagoo commented 6 years ago

I love the enthusiasm here, but I'm not sure defaulting to days as a unit (instead of seconds) is the right move here... Because, it's less inclusive (if nothing else).

Seconds work for every use case and everyone can calculate his duration in seconds one way or another. With days as the default unit (and with no way to indicate which unit one means, and no, we're not going down that road, because it will only complicate things more), you cannot specify less than 1 day for duration (which just feels wrong).

I was personally thinking of implementing it inside init(), at the same place this.options.expires is checked for some other types, and do an extra check to see if it's a number/numeric string and if so, convert it to an integer/number (which would leave existing code that sets cookie values/expiry as is).

What do you think about it?

zytzagoo commented 6 years ago

Closing since it's handled slightly differently via c3ecfc9 -- thanks for digging into it yourself and helping us resolve it!