Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Inconsistent results across different SS5 builds #17

Open
mhenden opened this issue Oct 11, 2023 · 2 comments
Open

BUG: Inconsistent results across different SS5 builds #17

mhenden opened this issue Oct 11, 2023 · 2 comments

Comments

@mhenden
Copy link

mhenden commented Oct 11, 2023

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
Copy link
Member

jsirish commented Oct 12, 2023

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
Copy link
Author

mhenden commented Oct 13, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants