search

ewels / CPT-Bootstrap-Carousel

WordPress plugin which generates a custom post type for choosing images and content. Outputs Bootstrap Image Carousel (slider) HTML from a shortcode.
http://wordpress.org/plugins/cpt-bootstrap-carousel/
GNU General Public License v2.0
51 stars 36 forks source link

Markup fixes #35

Closed reddo closed 10 years ago

reddo commented 10 years ago

Added data attributes to the container div, removed unnecessary data-interval attributes from carousel controls <li> and the script that became unnecessary after the data-interval has been added to the container.

ewels commented 10 years ago

Hi @reddo,

Thanks for the PR. This is actually the markup that I was originally using on one of the first releases of the plugin. However, due to some reported problems I changed it to use explicit javascript instead.

Although I agree that using data attributes is cleaner, I'm not keen to revert to this approach as I suspect that it will cause people problems. It seems to work at the moment - and for now I'm going to take the approach that if it ain't broke, don't fix it...

Cheers,

Phil

ewels commented 10 years ago

..though re-reading your changes I guess this could be because I incorrectly had the data-interval attribute on the <li> elements instead of the container. I'll have a play and do some testing.

ewels commented 10 years ago

These changes are also in your other PR (#37) so I'll close this one for now..