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

Set the has_js cookie for authenticated users #9

Closed

Conversation

deviantintegral
Copy link

@pwolanin
Copy link
Contributor

here's a simpler variant for just the has_js part:
https://github.com/pwolanin/6/commit/c59dd7705165050f44b4dd020beb16aac6f64583

@elliotttf
Copy link
Member

This looks pretty good to me. Is there a reason you put the JavaScript in the header and not the footer? Even though what you're doing is super light weight it's generally better practice to put JS that doesn't need to run for the page to work in the footer.

@elliotttf
Copy link
Member

And actually I think I like @pwolanin's approach a little better, it is definitely simpler. That won't blow up though if JS is disabled, will it?

@pwolanin
Copy link
Contributor

In mine it's in the footer - that seems to work fine for batch+js. We have that in production.

elliotttf added a commit to elliotttf/6 that referenced this pull request Feb 22, 2012
@elliotttf
Copy link
Member

I've moved this to another pull request (#29) that's closer to what @pwolanin implemented. Closing this one.

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.

3 participants