kenwheeler / slick

the last carousel you'll ever need
kenwheeler.github.io/slick
MIT License
28.52k stars 5.88k forks source link

Aria-selected is not updating correctly on dot or slide arrow navigation #2706

Open jasonday opened 7 years ago

jasonday commented 7 years ago

updateDots is executing, but changes to _.$dots are not occurring (expanding accessibility and correcting dots roles)

Demo: https://jsfiddle.net/b5bqk68j/29/

Steps to reproduce the problem

Code changes: Line 1300 - change role to tab (selected state is not recognized unless child elements of tablist have role of tab

_.$dots.attr('role', 'tablist').find('li').each(function(i) {
                $(this).attr({
                    'role': 'tab',
                    'aria-selected': 'false',
                    'aria-controls': 'navigation' + _.instanceUid + i + '',
                    'id': 'slick-slide' + _.instanceUid + i + ''
                });
            })

Line 2836 - added aria-selected changes:

  Slick.prototype.updateDots = function() {

        var _ = this;

        if (_.$dots !== null) {

            _.$dots
                .find('li')
                .removeClass('slick-active')
                .attr('aria-hidden','true')
                .attr('aria-selected', 'false');

            _.$dots
                .find('li')
                .eq(Math.floor(_.currentSlide / _.options.slidesToScroll))
                .addClass('slick-active')
                .attr('aria-hidden','false')
                .attr('aria-selected', 'true');

        }

    };

====================================================================

What is the expected behaviour?

aria-selected="true" should be applied appropriately to the active dot

====================================================================

What is observed behaviour?

aria-selected="true" sticks on the first dot li and does not update with updateDots. Additionally, aria-selected does not update on arrow navigation.

====================================================================

More Details

jasonday commented 7 years ago

I did not realize that initADA is executing multiple times, so I was able to move aria-selected out of that function and keep it in updateDots, which gives the expected behavior.

jasonday commented 7 years ago

Because initADA executes on any change line 1307 is an issue, because it always sets the first dot li to aria-selected: true and in line 1302, setting the rest to aria-selected: false.

It appears that initADA should not be executing repeatedly (or that the expectation is that it would not execute multiple times).

chenchenalex commented 7 years ago

Is this issue be fixed, I've found the same issue which slick always set the first item in the pips to be aria-select = "true" and not updating correctly

jasonday commented 7 years ago

I ended up correcting the code and improving accessibility overall. Look for my open pull request.

On Feb 7, 2017 6:58 PM, "A.C" notifications@github.com wrote:

Is this issue be fixed, I've found the same issue which slick always set the first item in the pips to be aria-select = "true" and not updating correctly

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/kenwheeler/slick/issues/2706#issuecomment-278184667, or mute the thread https://github.com/notifications/unsubscribe-auth/AAtp6yr36SIOnT4IumjgnJwORcnhzBqgks5raQU2gaJpZM4LmFTc .

chenchenalex commented 7 years ago

Thanks!

jasonday commented 7 years ago

Here's the link: https://github.com/kenwheeler/slick/pull/2708

johncionci commented 2 years ago

Has this been resolved? I still see this issue happening when calling more than 1 slideToScroll/Show.

    dots: true,
    infinite: true,
    speed: 300,
    slidesToShow: 3,
    slidesToScroll: 3,
    adaptiveHeight: true,