dynamic / silverstripe-carousel

A simple Bootstrap carousel for Silverstripe websites
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

BUG: Inconsistent results across different SS5 builds #17

Open mhenden opened 9 months ago

mhenden commented 9 months ago

Describe the bug I have a total of four different sites that include the Silverstripe Carousel feature. I am getting different results across these sites.

The templates for this site are loosely based on the simple theme. They include a renamed version of script.js (the responsive javascript that changes the navigation menu for smaller screens). I have called this file navscript.js so I know what its purpose is. I have also added bootstrap.min.js and bootstrap.css as Silverstripe Carousel uses these libraries.

Two sites also use the clicky-menus javascript to improve the UX/UI design

Scripts are added thus:

<% require javascript('//code.jquery.com/jquery-3.6.1.min.js') %> <% require themedJavascript('navscript') %> <% require themedJavascript('clicky-menus') %> <% require themedJavascript('bootstrap.min') %>

Scripts are placed between and tags (I have tried alternative positions).

At this stage I have not found a 'reliable' solution. I have also tried the jquery-3.3.1 library with no change to problem.

Site 1: www.thevictoriatavern.co.nz has been published. Slider works as expected, 'clicky-menus' works and site navigation is responsive

Site 2: Sister site to 'The Vic' with same features. Slider works. 'clicky-menus' and 'navscript' don't. I am unable to get this functionality

Site 3: Site includes slider and responsive navigation. On Desktop view Slider is a static image, in 'Mobile' (smaller screen) view slider is a list of four images.

Site 4: Test site ('vanilla build') that uses Silverstripe's simple theme. Responsive navigation (script.js) works as expected. Initially slider was static image on Desktop and a list view of two images on Mobile. After I reordered the slides and resaved slider worked as expected (yay).

To Reproduce I don't know how to reproduce errors as similar code is giving different results.

Expected behavior I would have expected everythign to work across the board.

Question Are there any known incompatibilities with the scripts used on this slider?

Is the DOM not loading completely?

Please advise?

jsirish commented 9 months ago

Hi @mhenden

We build all of our Silverstripe themes using Boostrap. The default module in this template is specifically written for Bootstrap 5. Because of this, we can drop this into any of our projects and the CSS and JS are already loaded by the theme.

I don't know if I'd recommend pulling the Bootstrap CSS and JS into Simple theme or themes not built in Bootstrap - I'm not saying it won't work, but it really wasn't intended to work that way.

A different way to think about this module if you're not building your themes in Bootstrap is to roll your own template but use the data model in the CMS. For example, you could copy the Carousel.ss template into your theme, and update it to work with your theme or front-end framework. If you do this, you can change the HTML and include any JS and CSS you'd like for the carousel functionality.

Here's an example from Splide, I library we used a lot in Silverstripe 4:

https://splidejs.com/guides/getting-started/

You could use this as your base, pull in their CSS and JS, and put the <% loop $Slides %> call in there to generate the slides.

We've discussed including alternate templates in the module that uses non-Bootstrap HTML, CSS, and JS. However, for now, we're focusing on features and functionality as this is a drop-in module for all of our projects.

Thanks again for the issue ticket! I'll review your other ones as well. Appreciate you testing the module, hope this write-up helps.

mhenden commented 9 months ago

Hi Jason

Thanks for getting back to me on this — useful feedback. I’ll check out the link for Splide you supplied.

I can definitely understand that your team built Silverstripe Carousel to work with sites that are based in Boostrap, and ‘bolting on’ Bootstrap to a theme based on another discipline might be problematic.

What I don’t understand is why I can get the slider working on SOME of my themes, where in two instances for sure the codebase is almost identical, but not others. In fact, after a bit of faffing about (which seems to be down to issues with that particular install) I was able to get Silverstripe Carousel running on Simple theme with very little hassle (unlike the two other sites I am working with).

As it happens I do use Bootstrap on a lot of my sites, at least as far as the Grid/Responsive facility goes. Not too sure what features Bootstrap.js covers but obviously there are some functions in there that Silverstripe Carousel uses!

cheers mike

14 Brees Street, Lower Hutt, Wellington, New Zealand p: 04-577 0243, m: 027 4419 273, e: @. @.> w: www.mhdesign.co.nz http://www.mhdesign.co.nz/

On 13/10/2023, at 11:14 AM, Jason Irish @.***> wrote:

Hi @mhenden https://github.com/mhenden We build all of our Silverstripe themes using Boostrap. The default module in this template is specifically written for Bootstrap 5. Because of this, we can drop this into any of our projects and the CSS and JS are already loaded by the theme.

I don't know if I'd recommend pulling the Bootstrap CSS and JS into Simple theme or themes not built in Bootstrap - I'm not saying it won't work, but it really wasn't intended to work that way.

A different way to think about this module if you're not building your themes in Bootstrap is to roll your own template but use the data model in the CMS. For example, you could copy the Carousel.ss template into your theme, and update it to work with your theme or front-end framework. If you do this, you can change the HTML and include any JS and CSS you'd like for the carousel functionality.

Here's an example from Splide, I library we used a lot in Silverstripe 4:

https://splidejs.com/guides/getting-started/ https://splidejs.com/guides/getting-started/ You could use this as your base, pull in their CSS and JS, and put the <% loop $Slides %> call in there to generate the slides.

We've discussed including alternate templates in the module that uses non-Bootstrap HTML, CSS, and JS. However, for now, we're focusing on features and functionality as this is a drop-in module for all of our projects.

Thanks again for the issue ticket! I'll review your other ones as well. Appreciate you testing the module, hope this write-up helps.

— Reply to this email directly, view it on GitHub https://github.com/dynamic/silverstripe-carousel/issues/17#issuecomment-1760445995, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIH3TC3PWDT4WVQZZWJTB3X7BTUDAVCNFSM6AAAAAA53KDH5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRQGQ2DKOJZGU. You are receiving this because you were mentioned.