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

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

Open
1 of 3 tasks
awacode21 opened this issue Dec 4, 2020 · 14 comments

Comments

@awacode21
Copy link

awacode21 commented Dec 4, 2020

This is a (multiple allowed):

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.

Related Issues that got closed before without any solution:
#1584
#290
#699

@ntsim
Copy link

ntsim commented Apr 23, 2021

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 😄

@awacode21
Copy link
Author

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

banjahman commented Jun 27, 2022

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

josh2112 commented Jan 27, 2023

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

  1. It only fixes the slide scroll position when you swipe, not on focus change, and
  2. 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
Copy link

oreoorbitz commented Feb 26, 2023

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

This bug should be resolved in version 8.0.0

@chunglam2525
Copy link

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

  1. It only fixes the slide scroll position when you swipe, not on focus change, and
  2. 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

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

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

@ankursehdev
Copy link

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

@mediaformat
Copy link

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

Repository owner deleted a comment from mediaformat May 28, 2023
@claygriffiths
Copy link

claygriffiths commented Jun 23, 2023

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

anneria commented Oct 9, 2023

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

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

  1. It only fixes the slide scroll position when you swipe, not on focus change, and
  2. 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

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

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.

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

No branches or pull requests