-
Notifications
You must be signed in to change notification settings - Fork 458
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
Use BaguetteBox in base theme #2791
Conversation
@@ -20,7 +20,13 @@ ${template_hooks['extra_head']()} | |||
${footer.html_footer()} | |||
</div> | |||
${base.late_load_js()} | |||
% if use_cdn: | |||
<script src="//cdnjs.cloudflare.com/ajax/libs/baguettebox.js/1.8.2/baguetteBox.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This keeps it protocol-relative, is https better? It used to give a mixed-content warning :-P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protocol-relative stopped being cool a while back. Using httpS never gave mixed-content warnings, only http did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, https it is
Are captions supported if we wanted them? |
@Kwpolska yes, they are |
I have NO IDEA why this is failing. |
(PS. CSSLint is pretty crazy, so I nuked it.) |
This adds a nice, lightweight lightbox to the base theme.
Very conservative branch: no changes in bootstrap3, no captions, just the very barebones support.