search

washingtonstateuniversity / magazine.wsu.edu

WSU Magazine WordPress theme
1 stars 0 forks source link

[PR] Colorbox support #128

Closed philcable closed 8 years ago

philcable commented 8 years ago

@jeremyfelt Can you please review this and see if it looks like an okay direction to go for supporting ad hoc lightbox functionality? No merge yet, as I would still need to build it out a little more, add better documentation and default styles, etc.

Basic ideas borrowed from TablePress. The shortcode would accept two attributes: selector - .class or #id, and settings - key/value pairs documented at http://www.jacklmoore.com/colorbox/.

jeremyfelt commented 8 years ago

I think this is a good start. You many be able to merge the script enqueue into the shortcode method, though the way it's being checked for now is good.

The escaping of various settings will be interesting. We may need to decide if things need to be whitelisted.

philcable commented 8 years ago

@jeremyfelt Thoughts on this for escaping the settings?

philcable commented 8 years ago

If it checks out okay, I think this could be ready. @jeremyfelt #reviewmerge?

jeremyfelt commented 8 years ago

This looks great! If/when we turn this into a platform plugin (or before), we may be able to...

  1. Use the new wp_add_inline_script() to inject the scripts into the output.
  2. Provide a few default settings that make sense for the theme and then override them with what is passed through the shortcode.

But I'm mostly leaving that for documentations sake and merging. 👍