fix(Carousel): Use event handler instead of useEffect for scroll

useEffect was causing problems on React 18, so this change reworks the scrolling mechanism to explicitly call `scrollTo` on every single manual event (clicks, keyboard) as well as when the `currentIndex` prop changes. It also sets `isManuallyUpdating` to true.

After that we listen to the 'scroll' event and reset `isManuallyUpdating` to false after the very last scroll event.

The intersection observer is used for syncing the active dot with scroll events that are triggered natively (e.g. dragging on mobile). We check for `isManuallyUpdating` here to avoid messing with manual `scrollTo` calls.

packages/iTwinUI-react/src/core/Carousel/Carousel.tsx

@@ -62,23 +62,32 @@ export const Carousel = Object.assign(
     useTheme();
 
     const isManuallyUpdating = React.useRef(false);
-    const scrollInstantly = React.useRef(false);
     const carouselRef = React.useRef<HTMLElement>(null);
     const refs = useMergedRefs(carouselRef, ref);
 
-    const [currentIndex, _setCurrentIndex] = React.useState(userActiveIndex);
+    const [currentIndex, setCurrentIndex] = React.useState(userActiveIndex);
+
+    const scrollToSlide = React.useRef<
+      (index?: number, options?: { instant?: boolean }) => void
+    >(() => {}); // stub function populated in CarouselSlider
+
+    const justMounted = React.useRef(true);
     React.useEffect(() => {
-      _setCurrentIndex(userActiveIndex);
-    }, [userActiveIndex]);
-
-    const setCurrentIndex = React.useCallback(
-      (index: number | ((old: number) => number)) => {
-        _setCurrentIndex(index);
-        isManuallyUpdating.current = true;
-        carouselRef.current?.focus();
-      },
-      [],
-    );
+      if (userActiveIndex != undefined) {
+        setCurrentIndex(userActiveIndex);
+        scrollToSlide.current(userActiveIndex, {
+          instant: justMounted.current,
+        });
+      }
+
+      // re-focus the carousel for keyboard nav, but not on first mount
+      // because it shows outline and might interfere with other components
+      if (!justMounted.current) {
+        carouselRef.current?.focus({ preventScroll: true });
+      }
+
+      justMounted.current = false;
+    }, [scrollToSlide, userActiveIndex]);
 
     const [slideCount, setSlideCount] = React.useState(0);
 
@@ -100,12 +109,16 @@ export const Carousel = Object.assign(
       switch (event.key) {
         case 'ArrowLeft': {
           setKeysPressed((old) => ({ ...old, ArrowLeft: false }));
-          setCurrentIndex((old) => (slideCount + old - 1) % slideCount);
+          const prevIndex = (slideCount + currentIndex - 1) % slideCount;
+          scrollToSlide.current(prevIndex);
+          setCurrentIndex(prevIndex);
           break;
         }
         case 'ArrowRight': {
           setKeysPressed((old) => ({ ...old, ArrowRight: false }));
-          setCurrentIndex((old) => (slideCount + old + 1) % slideCount);
+          const nextIndex = (slideCount + currentIndex + 1) % slideCount;
+          scrollToSlide.current(nextIndex);
+          setCurrentIndex(nextIndex);
           break;
         }
       }
@@ -136,7 +149,7 @@ export const Carousel = Object.assign(
             keysPressed,
             idPrefix: id,
             isManuallyUpdating,
-            scrollInstantly,
+            scrollToSlide,
           }}
         >
           {children}

packages/iTwinUI-react/src/core/Carousel/CarouselContext.tsx

@@ -37,9 +37,11 @@ export const CarouselContext = React.createContext<
        */
       isManuallyUpdating: React.MutableRefObject<boolean>;
       /**
-       * Ref object used to skip smooth scrolling when button is clicked too many times.
+       * Function that scrolls to the current slide. Should be called on all managed events (clicks and keydowns).
        */
-      scrollInstantly: React.MutableRefObject<boolean>;
+      scrollToSlide: React.MutableRefObject<
+        (slideIndex: number, options?: { instant?: boolean }) => void
+      >;
     }
   | undefined
 >(undefined);

packages/iTwinUI-react/src/core/Carousel/CarouselDotsList.tsx

@@ -71,6 +71,7 @@ export const CarouselDotsList = React.forwardRef<
     (index: number) => {
       if (context) {
         context.setCurrentIndex(index);
+        context.scrollToSlide.current(index);
       }
       onSlideChange?.(index);
     },

packages/iTwinUI-react/src/core/Carousel/CarouselNavigation.tsx

@@ -20,9 +20,10 @@ const PreviousButton = React.forwardRef<HTMLButtonElement, IconButtonProps>(
 
     const {
       slideCount,
+      currentIndex,
       setCurrentIndex,
       keysPressed,
-      scrollInstantly,
+      scrollToSlide,
     } = context;
 
     return (
@@ -34,10 +35,9 @@ const PreviousButton = React.forwardRef<HTMLButtonElement, IconButtonProps>(
         ref={ref}
         {...props}
         onClick={(e) => {
-          if (e.detail > 3) {
-            scrollInstantly.current = true;
-          }
-          setCurrentIndex((old) => (slideCount + old - 1) % slideCount);
+          const prevIndex = (slideCount + currentIndex - 1) % slideCount;
+          setCurrentIndex(prevIndex);
+          scrollToSlide.current(prevIndex, { instant: e.detail > 3 });
           props?.onClick?.(e);
         }}
       >
@@ -57,9 +57,10 @@ const NextButton = React.forwardRef<HTMLButtonElement, IconButtonProps>(
 
     const {
       slideCount,
+      currentIndex,
       setCurrentIndex,
       keysPressed,
-      scrollInstantly,
+      scrollToSlide,
     } = context;
 
     return (
@@ -71,10 +72,9 @@ const NextButton = React.forwardRef<HTMLButtonElement, IconButtonProps>(
         ref={ref}
         {...props}
         onClick={(e) => {
-          if (e.detail > 3) {
-            scrollInstantly.current = true;
-          }
-          setCurrentIndex((old) => (slideCount + old + 1) % slideCount);
+          const nextIndex = (slideCount + currentIndex + 1) % slideCount;
+          setCurrentIndex(nextIndex);
+          scrollToSlide.current(nextIndex, { instant: e.detail > 3 });
           props?.onClick?.(e);
         }}
       >

packages/iTwinUI-react/src/core/Carousel/CarouselSlide.tsx

@@ -32,18 +32,14 @@ export const CarouselSlide = React.forwardRef<
     throw new Error('CarouselSlide must be used within Carousel');
   }
 
-  const { currentIndex, isManuallyUpdating, setCurrentIndex } = context;
+  const { isManuallyUpdating, setCurrentIndex } = context;
 
   const updateActiveIndexOnScroll = React.useCallback(() => {
     // only update index if scroll was triggered by browser
-    if (!isManuallyUpdating.current && currentIndex !== index) {
+    if (!isManuallyUpdating.current) {
       setCurrentIndex(index);
     }
-    // when manual scroll completes, reset the state of `isManuallyUpdating` so that it's ready for future actions
-    if (currentIndex === index) {
-      isManuallyUpdating.current = false;
-    }
-  }, [currentIndex, index, isManuallyUpdating, setCurrentIndex]);
+  }, [index, isManuallyUpdating, setCurrentIndex]);
 
   const intersectionRef = useIntersection(
     updateActiveIndexOnScroll,

packages/iTwinUI-react/src/core/Carousel/CarouselSlider.tsx

@@ -5,7 +5,7 @@
 import React from 'react';
 import cx from 'classnames';
 import { CarouselContext } from './CarouselContext';
-import { getWindow, useMergedRefs, useResizeObserver } from '../utils';
+import { getWindow, useMergedRefs } from '../utils';
 
 /**
  * `CarouselSlider` is the scrollable list that should consist of `CarouselSlide` components.
@@ -22,11 +22,10 @@ export const CarouselSlider = React.forwardRef<
   }
 
   const {
-    currentIndex,
     setSlideCount,
     idPrefix,
+    scrollToSlide,
     isManuallyUpdating,
-    scrollInstantly,
   } = context;
 
   const items = React.useMemo(
@@ -46,53 +45,52 @@ export const CarouselSlider = React.forwardRef<
     setSlideCount(items.length);
   }, [items.length, setSlideCount]);
 
-  const [width, setWidth] = React.useState<number>();
-  const [resizeRef] = useResizeObserver(({ width }) => setWidth(width));
-
   const sliderRef = React.useRef<HTMLOListElement>(null);
-  const refs = useMergedRefs(sliderRef, resizeRef, ref);
-  const justMounted = React.useRef(true);
+  const refs = useMergedRefs(sliderRef, ref);
 
-  const previousWidth = React.useRef<number>();
-  React.useLayoutEffect(() => {
-    const slideToShow = sliderRef.current?.children.item(currentIndex) as
+  scrollToSlide.current = (
+    slideIndex: number,
+    { instant }: { instant?: boolean } = {},
+  ) => {
+    isManuallyUpdating.current = true; // start manual update
+
+    const slideToShow = sliderRef.current?.children.item(slideIndex) as
       | HTMLElement
       | undefined;
 
-    if (
-      !sliderRef.current ||
-      !slideToShow ||
-      (!isManuallyUpdating.current && previousWidth.current === width)
-    ) {
+    if (!sliderRef.current || !slideToShow) {
       return;
     }
 
-    // instant scroll on first mount
-    if (justMounted.current) {
-      scrollInstantly.current = true;
-      justMounted.current = false;
-    }
-
     const motionOk = getWindow()?.matchMedia(
       '(prefers-reduced-motion: no-preference)',
     )?.matches;
 
     sliderRef.current.scrollTo({
       left: slideToShow.offsetLeft - sliderRef.current.offsetLeft,
-      behavior: (scrollInstantly.current || !motionOk
-        ? 'instant'
-        : 'smooth') as ScrollBehavior, // scrollTo accepts 'instant' but ScrollBehavior type is wrong
+      behavior: (instant || !motionOk ? 'instant' : 'smooth') as ScrollBehavior, // scrollTo accepts 'instant' but ScrollBehavior type is wrong
     });
+  };
+
+  const scrollTimeout = React.useRef<number>();
+
+  // reset isManuallyUpdating.current to false after the last scroll event
+  const handleOnScroll = React.useCallback(() => {
+    if (scrollTimeout.current) {
+      clearTimeout(scrollTimeout.current);
+    }
 
-    scrollInstantly.current = false;
-    previousWidth.current = width;
-  }, [currentIndex, isManuallyUpdating, scrollInstantly, width]);
+    scrollTimeout.current = window.setTimeout(() => {
+      isManuallyUpdating.current = false;
+    }, 100);
+  }, [isManuallyUpdating]);
 
   return (
     <ol
       aria-live='polite'
       className={cx('iui-carousel-slider', className)}
       ref={refs}
+      onScroll={handleOnScroll}
       {...rest}
     >
       {items}