MWDelaney / bootstrap-3-shortcodes

WordPress shortcodes for easier use of Bootstrap elements in your content.
https://wordpress.org/plugins/bootstrap-3-shortcodes/
MIT License
377 stars 118 forks source link

Add Bootstrap Tab Naming and Hooks for Extending Behavior #128

Closed jlmaners closed 8 years ago

jlmaners commented 8 years ago

This is still a WIP because of the need for documentation for the end user. This PR is work towards #127 to add browser history, and deep linking to bootstrap tabs.

Changes:

In this code base I assume folks want to have tab history. I've talked with many developers on our campus and we couldn't come up with why someone wouldn't want this functionality, so I made the call to default to on.

Possible values and explanations for new history attribute:

While this whole PR is still WIP, I'd appreciate any feedback from regular contributors.

MWDelaney commented 8 years ago

Thank you for your work on this.

I like the inclusion of optionally naming tab groups, and of naming the tabs themselves with perhaps a more concise name than the ugly "custom-tab-big-long-md5-string" method we have now, but I honestly think that the tab history functionality belongs in a separate plugin.

The goal of this plugin is to implement Bootstrap's components as closely as possible, and since tab history isn't a function of Bootstrap, in the interest of avoiding feature creep I recommend it be separated.

But! I would love to make the tab naming changes to this plugin so that it can more easily interoperate with a tab history plugin (and with other tools that need to concisely and predictably reference tab names). I'll take a look at the tab naming code here and write again.

Thank you again. I'm thrilled to hear that you are using this tool in your environment. Have a great day!

jlmaners commented 8 years ago

Thanks for the feedback. As requested, I've pulled out the tab history functionality but added two filter hooks to filter the supplied shortcode arguments as well as the final output from bs_tabs. I believe this should give me enough to hook into to write my own plugin.

By hooking into the shortcode arguments, someone could add an argument that I could translate into data pairs which your plugin already supports. I could also just parse the final output and overwrite there.

Please let me know your thoughts!

jlmaners commented 8 years ago

@MWDelaney just wanted to follow up with you on this and see if you had a chance to review the changes I have made? I have removed the history functionality but added the opportunity to use filters to tweak the output of the tabs to inject the JS history functionality.

We're looking to use this at NC State University as part of a theme we are using built around Bootstrap and I'd rather not keep a fork of your plugin with these changes going for a long period of time.

Thanks for any feedback!

jlmaners commented 8 years ago

Really hate to be a nag here but any chance this will get reviewed and integrated?