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

list based slider #212

Open
qasimalyas opened this issue May 7, 2014 · 28 comments · May be fixed by #2983
Open

list based slider #212

qasimalyas opened this issue May 7, 2014 · 28 comments · May be fixed by #2983

Comments

@qasimalyas
Copy link

From the time i've spent so far with the plugin I think its great and very flexible. I have however run into an issue which, whether this is or not.

It doesn't seem that one can create a slider based upon an unordered list:

<ul>
    <li>item one</li>
    <li>item one</li>
    <li>item one</li>
</ul>

Is there a reason for this? Would be nice if markup didn't matter.

@willdavidow
Copy link

You can do this already, you just need to use the 'slide' config option:

$('.slider').slick({
    slide: 'li'
    // other options
})

http://jsfiddle.net/davewillidow/z943d/3/

@qasimalyas
Copy link
Author

Ok that works but it kind of destroys the semantics of the markup. The generated output is now:

<ul>
    <div>
           <div>
               <li></li>
               <li></li>
               <li></li>
           </div>
    </div>
<ul>

@kenwheeler
Copy link
Owner

Those div elements get wrapped around your stuff after the page loads, via JavaScript , so anything that measured semantic value would never see it.

@MattDiMu
Copy link

@kenwheeler I doubt, that "anything that measures semantic value" never sees the generated dom:

IMO there should be a way to stay semantically correct when using slick on lists (which are probably most slideshows).

@kenwheeler
Copy link
Owner

Crawlers do not execute javascript, they have an escapedFragment callback that allows developers to serve a rendered page independently of the original source

@MattDiMu
Copy link

@kenwheeler thx for your reply. I still think, however, that (at least Google) does nowadays execute JavaScript when crawling:

Traditionally, we were only looking at the raw textual content that we’d get in the HTTP response body and didn't really interpret what a typical browser running JavaScript would see. When pages that have valuable content rendered by JavaScript started showing up, we weren’t able to let searchers know about it, which is a sad outcome for both searchers and webmasters.

In order to solve this problem, we decided to try to understand pages by executing JavaScript.

Source: http://googlewebmastercentral.blogspot.co.at/2014/05/understanding-web-pages-better.html

@paslandau
Copy link

@kenwheeler fyi
escapedFragement is deprecated since a few days ( see http://googlewebmastercentral.blogspot.de/2015/10/deprecating-our-ajax-crawling-scheme.html ).

Cheers
Pascal

@qikkeronline
Copy link

I also believe this should be adressed, I think a semantically correct output regardless of wether crawlers do not parse the JS would be great (wich they do for quite a while already). Another use case: people with screen readers.

@qikkeronline
Copy link

Wouldn't a simple config flag do the trick? All we need to be able to do is set the .slick-track element type e.g. trackType: 'ul' or so

@lsterling03
Copy link

I arrived here looking for this solution as well. I agree with qikkeronline -- would be great to be able to set the slick-track element as a ul to keep it semantic with the li slides.

@schnubb
Copy link

schnubb commented Oct 19, 2016

so, any chance to see this issue reopend with an updated pov from today?? :)
@kenwheeler your statement was right, two years ago, but today is another time.

@qasimalyas
Copy link
Author

@schnubb I personally do think that this should be reopened and addressed. Personally i've moved on to using other solutions which keep original HTML structure and with options provide aria support too.

@greenandlonely
Copy link

Adding my name to the list of people who'd love to see this addressed. Slick is great but, for me, this is the only flaw.

@MichaelJGW
Copy link

+1

@kenwheeler kenwheeler reopened this Nov 21, 2016
@kenwheeler
Copy link
Owner

Ok gang, sold.

@kenwheeler
Copy link
Owner

@simeydotme @leggomuhgreggo thoughts? should we let any tag be specified and then just hit it with a couple of styles that undo any tag based style defaults that could disrupt things? Or just switch to uls? or make it an enum?

@simeydotme
Copy link
Collaborator

simeydotme commented Nov 21, 2016

I don't know man... if you allow <ul> and <li> I can see the plethora of issues relating to padding/margin cockups by over-zealous frameworks 😛
That said, however, maybe set an option for slick-track to be a dom-reference, and if it is, it's direct-children should be the slides? (as #212 (comment) said) Still need to wrap it in a container, though

footnote: anyone talking about semantics should take a long think about having a carousel on their site. usability, readability and accessibility should be higher up your concerns than semantics of lists 😆

@kenwheeler
Copy link
Owner

lol

@qikkeronline
Copy link

<sarcasm>
@simeydotme So you're contributing to a caroussel js repo, but you think that serious developers shouldnt use a caroussel at all? That seems like a constructive relationship.
</sarcasm>

Adding this feature will only improve accessibility IMHO. An <ul> is much more screenreader friendly then a couple of nested <div> elements.

I agree, usability, readability and accessibility are valid concerns. But that's what we're trying to improve here right? I believe carousels have become a valid UX pattern and users by now know how to use them, so we can't simply say "serious developers shouldn't use a carousel" anymore.

To me there are 2 valid choices here:

  • Simply adding the option to just pick the .slick-track element could suffice. So don't reset the styles of the <ul> and <li> elements. If the developer has the insight that his carousel needs to be more accessible / he doesn't like the semantics of it, then I also think he would get around resetting those styles himself.
  • Make Slick use an <ul> element by default, and do reset the styles. Possibly still add the option for people to be able to set the slick-track element (just for customisation purposes). Personally I'm all up for this, as it would improve accessibility for everyone implementing Slick from here on.

@Rafaelki
Copy link

Rafaelki commented Dec 7, 2016

Thank you for reopen this issue.
I made the trick basically modifying the line 521, wrapping the element with the slick-track div instead of appending it: _.$slideTrack = _.$slider.wrap('<div class="slick-track"></div>').parent();
Then with some changes here and there I make it work with perfect semantics.
I have only tested this parameters: dots:true, arrows: true, slidesToShow:4, slidesToScroll:1, autoplay: true, autoplaySpeed: 4000, vertical: true/false, slide: 'li'.
I hope this help.
slick.js.txt

@johnrom
Copy link

johnrom commented Jul 11, 2017

Here's my two cents (glad to see this reopened). This shouldn't introduce any breaking changes to existing sliders. I think it's important to accept a callback function for this in the event that sliders cannot be identified otherwise (think CMS widgets).

The Source

<div class="slider">
    <ul class="slider__list">
        <li class="slider__slide">
            <img src="/image.jpg" alt="">
        </li>
    </ul>
</div>
<script>
    $('.slider').slick({
        track: function() { return $(this).children('slider__list'); }
        // etc
    });
</script>

The Result

<div class="slider slick-initialized slick-slider">
    <div aria-live="polite" class="slick-list draggable">
        <ul class="slider__list slick-track" style="opacity: 1;" role="listbox">
            <li class="slider__slide slick-slide slick-cloned slick-active" role="option">  
                <img src="/image.jpg" alt="">
            </li>
        </ul>
    </div>
</div>

Option accepts: selector, element, jquery element, callback

track: '.selector'
track: document.getElementById('selector')
track: $('.selector')
track: function() { return $('.selector'); }

@leggomuhgreggo
Copy link
Collaborator

@johnrom definitely think that's the best approach. I can't say this is at the top of our list of priorities and I'm wary of expanding the API, but if you whipped up a PR I think it could work.

Thanks!

@congson95dev
Copy link

@johnrom solution doesn't worked for me.

@johnrom
Copy link

johnrom commented Aug 26, 2018

@saxsax1995 my comment above is a suggestion for how the API for this could look. I have a PR that implements this, but it was never merged in.

@garrettrathbone
Copy link

garrettrathbone commented Nov 16, 2018

Has there been any progress on this and getting the PR merged in?

@puttykitties
Copy link

5 years and ongoing, is this supported now and any progress made? Thanks for the hard work though.

@johnrom
Copy link

johnrom commented Feb 20, 2019

I haven't heard anything and I've migrated to using component based sliders like the creator of this project's own https://github.com/FormidableLabs/nuka-carousel

Honestly, I think this project will need to be deprecated as the majority of major organizations (who support development) will have moved on to component-based Angular, React, Vue by now.

Just my opinion and may not be shared by the author.

@babymamapat
Copy link

This should be solved by now :/

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 a pull request may close this issue.