Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cookieName configuration #68

Merged
merged 2 commits into from
Oct 26, 2014
Merged

Conversation

jdeniau
Copy link
Contributor

@jdeniau jdeniau commented Sep 25, 2014

It may be usefull if you want to add multiple ouibounce on different type of pages.

@jdeniau jdeniau force-pushed the cookieName branch 2 times, most recently from b087a4f to 9b81fe4 Compare September 25, 2014 08:31
It may be usefull if you want to add multiple ouibounce on different type of pages.
@kevinweber
Copy link
Contributor

@jdeniau, thanks for that cookieName configuration! I'm looking forward to use it.

@jdeniau
Copy link
Contributor Author

jdeniau commented Oct 17, 2014

up? :)

@allanedk
Copy link

I was just about to suggest this feature! Great work! Looking forward to next release.

@carlsednaoui
Copy link
Owner

Hi everyone, sorry for the delay — been busy / on vacation. Will look and merge soon :)

Thanks again for the PR @jdeniau 🍔

@@ -103,7 +104,11 @@ function ouibounce(el, config) {
cookieDomain = ';domain=' + options.cookieDomain;
}

document.cookie = 'viewedOuibounceModal=true' + cookieExpire + cookieDomain + sitewide;
if (typeof options.cookieName !== 'undefined') {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdeniau shouldn't this be if (typeof options.cookieName === 'undefined') { use viewedOuibounceModal. Otherwise use options.cookieName?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right.

It's fixed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🍔

carlsednaoui added a commit that referenced this pull request Oct 26, 2014
Add cookieName configuration
@carlsednaoui carlsednaoui merged commit cb57efa into carlsednaoui:master Oct 26, 2014
@jdeniau jdeniau deleted the cookieName branch January 22, 2015 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants