Skip to content

Feature: Add option to use Slick Slider - #6

Open
umac10 wants to merge 5 commits into
masterfrom
feature/add-slick-slider
Open

umac10 wants to merge 5 commits into
masterfrom
feature/add-slick-slider

Conversation

@umac10

@umac10 umac10 commented Oct 24, 2019

Copy link
Copy Markdown

Added a new slideshow style option that uses basic Slick Slider functionality.

@lcaple

lcaple commented Oct 28, 2019

Copy link
Copy Markdown
Contributor

@umac10 Can we add the adaptiveHeight attribute to the slide for images of different sizes?

@tjsuw can you review this as well and offer any feedback to how the slider functions or the code? Any improvements that have been made to Immersives we can incorporate would be great.

@lcaple
lcaple requested review from lcaple and tjsuw October 28, 2019 20:46

@tjsuw tjsuw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

One thing that came up in one of my impactful story reviews with ATS is that the captions for Slick Slider are not accessible (they are triggered by hovering on a :before which screen readers can't see). Perhaps a future discussion about improving them vs finding an eventual alternative (e.g. we are using Bootstrap's carousel in the storytelling modules plugin).

Another piece of feedback I got on the story plugin is that the captions cover too much of the images on mobile and I think we run into that here too. This is something we're still working on so I'm happy to share what we come up with when we have a solution ready.

I would like to see some kind of navigation arrows on mobile so the user knows there are slides to flip through and it's not just a static image - and realizing they can't take up too much space (same issue as above!).

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

Successfully merging this pull request may close these issues.

3 participants