The slider display will inevitably break if a link, placed in one of the slides gets focused by tab key.

awacode21 commented 3 years ago

This is a (multiple allowed):

[x] bug
[ ] enhancement
[ ] feature-discussion (RFC)
Swiper Version: 5.4.5, but i guess it's relevant also for latest version as the bug also exists on swipers demo page (keyboard control example https://swiperjs.com/demos/260-keyboard-control.html)
Platform/Target and Browser Versions: macOS with Chrome Browser, but should be a global problem
Live Link or JSFiddle/Codepen or website with isssue: https://codepen.io/awacode/pen/NWRxVNj
Explanation how to fix it: https://grumpy.blog/en/js_sliders_and_the_tab_key/
Slider which already fixed the issue can be testet here: https://grumpy.blog/en/peppermint_touch_slider/

What you did

Use the tab key to tab through slide item links.

Expected Behavior

Slider should show the slide item which contains the current focus as active so that it does not break the slider because browser scrolled the block to the focused element.

Actual Behavior

Slider breaks, as its scrollLeft property has changed. Here’s the catch: when a link hidden by overflow: hidden catches focus, browser scrolls the content of the block so you can see the link. Yes, blocks with overflow: hidden also have scrollLeft property and they act just like overflow: auto blocks.

How you can fix it

This link describes the bug in detail and presents the needed solution to fix it: https://grumpy.blog/en/js_sliders_and_the_tab_key/ on this link https://grumpy.blog/en/peppermint_touch_slider/ you can see a slide which already uses this fix. Tab through the links within the slider and you can see.

This should definitely be fixed for the great and widely used swiper! The effort to correct it is not high, but the added value for the user is many times higher.

ntsim commented 3 years ago

Taking the advice from the blog post, you can implement the fix using something like this:

const Carousel = () => {
  const swiperRef = useRef();

  return (
    <Swiper
      onSwiper={(swiper) => {
        swiperRef.current = swiper;
      }}
    >
      <SwiperSlide
        onFocus={() => {
          if (!swiperRef.current) {
            return;
          }

          swiperRef.current.el.scrollLeft = 0;
          // Replace 0 with the slide index
          swiperRef.current?.slideTo(0);
        }}
      >
        ...
      </SwiperSlide>
    </Swiper>
  );
};

Of course, the ideal solution would be for Swiper to handle this automatically :smile:

awacode21 commented 3 years ago

it is a bug of Swiper, so it should be fixed in Swiper too. I also perceived that I can work around it, but I think that is the wrong approach. Swiper is so widely used and it is an elementary bug, i guess everybody who uses swiper should also get the fix.

banjahman commented 2 years ago

@ntsim do you have an example of how to do this with the angular version? I've given it a few tries but can't find a good way to get this to consistently work.

So far I've been able to halfway work around the issue by doing the following, however in some cases when you tab to an invisible slide, it shows like 95% of it, but since the entire slide hasn't fully been made visible it doesn't throw any events. When there are no events fired, there's no way we can correct the issue.

<swiper
    #swiper
    (activeIndexChange)="onSlideChange($event)"
    [config]="swiperOptions">
    <ng-container *ngFor="let banner of banners; let index = i">
          <ng-template swiperSlide> ... </ng-template>
    </ng-container>
</swiper>

@ViewChild( 'swiper', { static: false } ) swiper?: SwiperComponent;

onSlideChange( event ) {
    this.swiper.swiperRef.el.scrollLeft = 0;
    this.swiper.swiperRef.el.scrollTop = 0;
    this.swiper.swiperRef.slideTo( event[ 0 ].activeIndex );
  }

Note: I've tried all of the events I can think of that would work here.. slideChange, slideTransitionStart/End, etc..

josh2112 commented 1 year ago

Any progress on this?

I'm running into this when using swiper with Vue and putting text inputs on my slides. I have reimplemented the workaround mentioned by @ntsim to the best of my ability in Vue 3 (composition API):

<script setup lang="ts">
import { Swiper, SwiperSlide } from "swiper/vue";
import "swiper/css";

function onSlideChanged(swiper: any) {
  swiper.el.scrollLeft = 0;

  // This is pointles because the active index hasn't changed
  swiper.slideTo(swiper.activeIndex);
}
</script>

<template>
  <swiper @slideChange="onSlideChanged">
    <swiper-slide v-for="i in [1, 2, 3, 4, 5]">
      <div>
        <input type="text" :value="i" />
      </div>
    </swiper-slide>
  </swiper>
</template>

But it doesn't quite work for me because

It only fixes the slide scroll position when you swipe, not on focus change, and
swiper doesn't change its slide index when the focus changes to an element on another slide, so it still thinks it's on the same slide as before the tabbing began.

Example:

Run the app
Click on the first text box
Hit [tab] 4 times. You're now in the text box on slide 5, and swiper's scroll position is somewhere between slides 4 and 5.
If you swipe left (i.e. next slide), you end up at slide 2, because swiper still thinks we were at slide 1.
Similarly, you can't swipe right (i.e. previous slide) because swiper still thought we were at slide 1.

How do we get swiper to understand that the slide index has changed when tabbing to a control on another slide? Possibly by watching for focus change for all input elements, walking up the DOM until we find the slide, figure out what index it is, and slide there manually? I'm not smart enough to do that.

oreoorbitz commented 1 year ago

The key is to use a focus eventListener on the swiper container element, so that you can execute a call every time an element in the swiper is focused on,then you use closest to go to a swiper slide element, then you get the index from there.

    swiperInstance.el.addEventListener('focus', event => {
     const swiperSlideToGoTo = getClosestSwiperSlide(event.target)
     swiperSlideToGoTo && goToSlide(swiperSlideToGoTo, this.baseSwiper)
    }, true)

oreoorbitz commented 1 year ago

This bug should be resolved in version 8.0.0

chunglam2525 commented 1 year ago

Any progress on this?

I'm running into this when using swiper with Vue and putting text inputs on my slides. I have reimplemented the workaround mentioned by @ntsim to the best of my ability in Vue 3 (composition API):
<script setup lang="ts">
import { Swiper, SwiperSlide } from "swiper/vue";
import "swiper/css";

function onSlideChanged(swiper: any) {
  swiper.el.scrollLeft = 0;

  // This is pointles because the active index hasn't changed
  swiper.slideTo(swiper.activeIndex);
}
</script>

<template>
  <swiper @slideChange="onSlideChanged">
    <swiper-slide v-for="i in [1, 2, 3, 4, 5]">
      <div>
        <input type="text" :value="i" />
      </div>
    </swiper-slide>
  </swiper>
</template>
But it doesn't quite work for me because

It only fixes the slide scroll position when you swipe, not on focus change, and

swiper doesn't change its slide index when the focus changes to an element on another slide, so it still thinks it's on the same slide as before the tabbing began.

Example:

Run the app

Click on the first text box

Hit [tab] 4 times. You're now in the text box on slide 5, and swiper's scroll position is somewhere between slides 4 and 5.

If you swipe left (i.e. next slide), you end up at slide 2, because swiper still thinks we were at slide 1.

Similarly, you can't swipe right (i.e. previous slide) because swiper still thought we were at slide 1.

How do we get swiper to understand that the slide index has changed when tabbing to a control on another slide? Possibly by watching for focus change for all input elements, walking up the DOM until we find the slide, figure out what index it is, and slide there manually? I'm not smart enough to do that.

This issue should be fixed with v8.0.0 a11y changes

a11y: automatically switch slides on focus (Tab) in inactive slides (1288271), closes https://github.com/nolimits4web/Swiper/issues/3149

<script setup lang="ts">
import { Swiper, SwiperSlide } from "swiper/vue";
import "swiper/css";
import { A11y } from "swiper";
</script>

<template>
  <swiper
    :modules="[A11y]"
    :a11y="{
      enabled: true,
    }"
  >
    <swiper-slide v-for="i in [1, 2, 3, 4, 5]">
      <div>
        <input type="text" :value="i" />
      </div>
    </swiper-slide>
  </swiper>
</template>

mediaformat commented 1 year ago

This is still an issue with Swiper v9 https://codepen.io/MediaFormat/full/qBMZeaW

ankursehdev commented 1 year ago

any ETA on this? This is still an issue in v9

mediaformat commented 1 year ago

Essentially #3149 was closed with these 2 remaining items:

Prevent tabbing to focusable items within out-of-view slides
Make sure disabled controls/buttons are not focusable

claygriffiths commented 1 year ago

We ran into this as well.

Adding the inert attribute to any slides outside of the active slide resolved this issue: https://html.spec.whatwg.org/multipage/interaction.html#the-inert-attribute

anneria commented 1 year ago

We solved this issue in vue3 composition api with focusin and passing the index. Also works okay with the keyboard module enabled, but only for 1 instance. The event is set on the document, so no matter where the focus is, left and right will swipe it.

<script setup lang="ts">
import { Keyboard } from 'swiper/modules';

const mainSwiper = ref<SwiperClass>();

const onSlideFocus = (index: number) => {
    mainSwiper.value?.slideTo(index);
};
</script>

<template>
    <swiper
        :modules="[Keyboard]"
        :keyboard="{
            enabled: true
        }">
        <swiper-slide
            v-for="(item, index) in Items"
            @focusin="() => onSlideFocus(index)">
            <nuxt-link>link</nuxt-link>
        </swiper-slide>
    </swiper>
</template>

cferdinandi commented 11 months ago

Any progress on this? I'm running into this when using swiper with Vue and putting text inputs on my slides. I have reimplemented the workaround mentioned by @ntsim to the best of my ability in Vue 3 (composition API):
<script setup lang="ts">
import { Swiper, SwiperSlide } from "swiper/vue";
import "swiper/css";

function onSlideChanged(swiper: any) {
  swiper.el.scrollLeft = 0;

  // This is pointles because the active index hasn't changed
  swiper.slideTo(swiper.activeIndex);
}
</script>

<template>
  <swiper @slideChange="onSlideChanged">
    <swiper-slide v-for="i in [1, 2, 3, 4, 5]">
      <div>
        <input type="text" :value="i" />
      </div>
    </swiper-slide>
  </swiper>
</template>
But it doesn't quite work for me because

It only fixes the slide scroll position when you swipe, not on focus change, and

swiper doesn't change its slide index when the focus changes to an element on another slide, so it still thinks it's on the same slide as before the tabbing began.

Example:

Run the app

Click on the first text box

Hit [tab] 4 times. You're now in the text box on slide 5, and swiper's scroll position is somewhere between slides 4 and 5.

If you swipe left (i.e. next slide), you end up at slide 2, because swiper still thinks we were at slide 1.

Similarly, you can't swipe right (i.e. previous slide) because swiper still thought we were at slide 1.

How do we get swiper to understand that the slide index has changed when tabbing to a control on another slide? Possibly by watching for focus change for all input elements, walking up the DOM until we find the slide, figure out what index it is, and slide there manually? I'm not smart enough to do that.
This issue should be fixed with v8.0.0 a11y changes

a11y: automatically switch slides on focus (Tab) in inactive slides (1288271), closes Improve Accessibility Features #3149
<script setup lang="ts">
import { Swiper, SwiperSlide } from "swiper/vue";
import "swiper/css";
import { A11y } from "swiper";
</script>

<template>
  <swiper
    :modules="[A11y]"
    :a11y="{
      enabled: true,
    }"
  >
    <swiper-slide v-for="i in [1, 2, 3, 4, 5]">
      <div>
        <input type="text" :value="i" />
      </div>
    </swiper-slide>
  </swiper>
</template>

This is not a fix. This is a bug. That's not the desired behavior.

geekysaurabh001 commented 11 months ago

This isn't fixed. Adding a11y.enabled = true messes up everything else but fixes the display issue on link tabbed to different slide. So, any idea? Like it messes up role attributes of the html elements inside, aria label is messed up and replaced with 1/4, 2/4, etc.

jeffreyfabrique commented 3 months ago

For me the fix was upgrading to v8 and using the A11y module as explained above.

nolimits4web / swiper