search

TheWebShop / bootstrap-shortcodes

Wordpress plugin to add shortcodes for Twitter Bootstrap 3.0
36 stars 15 forks source link

Load TWBS js & css by default #39

Closed bpongy closed 9 years ago

bpongy commented 10 years ago

I think TWBS js & css should be unchecked by default, because this plugin is used by themes with TWBS already loaded.

infn8 commented 9 years ago

Agreed. You can make a notice on activation indication as much to the user.

Sinetheta commented 9 years ago

I like the idea of a notification, but you really think the default should be "if you install this plugin to a new site it will not work"?

infn8 commented 9 years ago

Very good point. This is a big issue though because @redpik has a good point. Perhaps the plugin can do all of the following:

  1. Make a notification on activation saying that it's adding them and the default is that those options are set to true (checked).
  2. Check if there are styles / scripts already enqueued with the names 'bootstrap-css' and 'bootstrap-js' and if so don't re-register them just enqueue them (so there would only be one copy of each).
  3. Have a set of filters, eg: 'bs_shortcodes_add_css' that will override the settings in the admin ui

This way, theme developers have two methods to ensure no collisions occur. By enqueuing their resources with the same names as the plugin uses, and by filtering them out like:

add_filter("bs_shortcodes_add_css" "__return_false");
add_filter("bs_shortcodes_add_js" "__return_false");
Sinetheta commented 9 years ago

I think the "check if added" is actually something that is okay in our case. Since "bootstrap" is a non-standard lib, there's no way for us to avoid collisions just by enqueuing (since we have no control of what, if anything, other plugins/themes are enqueuing bootstrap as). What we can do, is modify our lib with a guard, eg: if($.fn.alert).

It's kinda crappy to modify a lib in this way, but WP doesn't give us a lot of options here.

macedd commented 9 years ago

I am already happy it can be disabled..

But there are other BS components which deserves place (higher priority, IMHO)

Sinetheta commented 9 years ago

I'm gonna close this issue as "working as intended", but I would be happy to merge any PRs that improved communication with users, eg: a notification, or explanation after installation.