refactor: code cleanup

Author: igordanchenko

dev/slides.ts

@@ -5,9 +5,9 @@ function imageLink(asset: string, size: number) {
 }
 
 export const slides = [
-  { asset: "image01.0800ee93.3840x5760" },
-  { asset: "image02.645bc7e4.3840x5070" },
-  { asset: "image03.13c5eeb7.3840x5120" },
+  { asset: "image01.0800ee93.3840x5760", description: "Puppy in sunglasses" },
+  { asset: "image02.645bc7e4.3840x5070", description: "Miami beach" },
+  { asset: "image03.13c5eeb7.3840x5120", description: "Flamingo" },
   { asset: "image04.2d71a97f.3840x2546" },
   { asset: "image05.c6ce32ab.3840x5760" },
   { asset: "image06.74d5e191.3840x2553" },
@@ -65,7 +65,6 @@ export const slides = [
     src: imageLink(asset, width),
     width,
     height,
-    description: "Description of the image",
     srcSet: breakpoints.map((breakpoint) => ({
       src: imageLink(asset, breakpoint),
       width: breakpoint,

src/contexts/A11yContext.tsx

@@ -24,7 +24,7 @@ export function A11yContextProvider({ children }: React.PropsWithChildren) {
   const [autoPlaying, setAutoPlaying] = React.useState(false);
 
   const context = React.useMemo(() => {
-    const trackFocusWithin = (onFocus?: React.FocusEventHandler, onBlur?: React.FocusEventHandler) => {
+    const trackFocusWithin: A11yContextType["trackFocusWithin"] = (onFocus, onBlur) => {
       const trackAndDelegate = (focusWithinValue: boolean) => (event: React.FocusEvent) => {
         if (!event.currentTarget.contains(event.relatedTarget)) {
           setFocusWithin(focusWithinValue);

src/modules/Carousel.tsx

@@ -9,13 +9,13 @@ import {
   cssClass,
   cssVar,
   getSlide,
-  getSlideIndex,
   getSlideKey,
   hasSlides,
   isImageSlide,
   makeInertWhen,
   parseLengthPercentage,
   translateLabel,
+  translateSlideCounter,
 } from "../utils.js";
 import { ImageSlide } from "../components/index.js";
 import { useController } from "./Controller/index.js";
@@ -33,10 +33,9 @@ function cssSlidePrefix(value?: string) {
 type CarouselSlideProps = {
   slide: Slide;
   offset: number;
-  index: number;
 };
 
-function CarouselSlide({ slide, offset, index }: CarouselSlideProps) {
+function CarouselSlide({ slide, offset }: CarouselSlideProps) {
   const containerRef = React.useRef<HTMLDivElement>(null);
 
   const { currentIndex, slides } = useLightboxState();
@@ -84,10 +83,6 @@ function CarouselSlide({ slide, offset, index }: CarouselSlideProps) {
     ) : null;
   };
 
-  const slideLabel = translateLabel(labels, "{{index}} of {{slidesLength}}")
-    .replace("{{index}}", String(index + 1))
-    .replace("{{slidesLength}}", String(slides.length));
-
   return (
     <div
       ref={containerRef}
@@ -100,7 +95,7 @@ function CarouselSlide({ slide, offset, index }: CarouselSlideProps) {
       style={style}
       role="region"
       aria-roledescription={translateLabel(labels, "Slide")}
-      aria-label={slideLabel}
+      aria-label={translateSlideCounter(labels, slides, currentIndex + offset)}
     >
       {renderSlide()}
     </div>
@@ -121,10 +116,7 @@ export function Carousel({ carousel, labels }: ComponentProps) {
   const paddingValue = parseLengthPercentage(carousel.padding);
 
   const preload = calculatePreload(carousel, slides, 1);
-  const items: ({ key: React.Key } & (
-    | { slide: Slide; offset: number; index: number }
-    | { slide?: never; offset?: never; index?: number }
-  ))[] = [];
+  const items: ({ key: React.Key } & ({ slide: Slide; offset: number } | { slide?: never; offset?: never }))[] = [];
 
   if (hasSlides(slides)) {
     for (let index = currentIndex - preload; index <= currentIndex + preload; index += 1) {
@@ -137,7 +129,6 @@ export function Carousel({ carousel, labels }: ComponentProps) {
           ? {
               key: [`${key}`, getSlideKey(slide)].filter(Boolean).join("|"),
               offset: index - currentIndex,
-              index: getSlideIndex(index, slides.length),
               slide,
             }
           : { key },
@@ -161,8 +152,8 @@ export function Carousel({ carousel, labels }: ComponentProps) {
       aria-roledescription={translateLabel(labels, "Carousel")}
       aria-label={translateLabel(labels, "Photo gallery")}
     >
-      {items.map(({ key, slide, offset, index }) =>
-        slide ? <CarouselSlide key={key} slide={slide} offset={offset} index={index} /> : <Placeholder key={key} />,
+      {items.map(({ key, slide, offset }) =>
+        slide ? <CarouselSlide key={key} slide={slide} offset={offset} /> : <Placeholder key={key} />,
       )}
     </div>
   );

src/modules/Controller/Controller.tsx

@@ -328,7 +328,10 @@ export function Controller({ children, ...props }: ComponentProps) {
 
   const focusOnMount = useEventCallback(() => {
     // capture focus only when rendered inside a portal
-    if (getOwnerDocument().querySelector(`.${cssClass(MODULE_PORTAL)} .${cssClass(cssContainerPrefix())}`)) {
+    if (
+      controller.focus &&
+      getOwnerDocument().querySelector(`.${cssClass(MODULE_PORTAL)} .${cssClass(cssContainerPrefix())}`)
+    ) {
       focus();
     }
   });

src/plugins/counter/Counter.tsx

@@ -30,7 +30,8 @@ export function CounterComponent({ counter }: ComponentProps) {
       className={clsx(cssClass("counter"), className || legacyClassName)}
       {...legacyRest}
       {...rest}
-      aria-hidden="true"
+      // slides already provide the "k of N" label, so there is no need to override the `aria-hidden` attribute here
+      aria-hidden
     >
       {currentIndex + 1} {separator} {slides.length}
     </div>

src/plugins/thumbnails/Thumbnail.tsx

@@ -12,7 +12,7 @@ import {
   makeComposePrefix,
   RenderThumbnailProps,
   Slide,
-  translateLabel,
+  translateSlideCounter,
   useDocumentContext,
   useEventCallback,
   useLightboxProps,
@@ -79,29 +79,20 @@ export type ThumbnailProps = {
   slide: Slide | null;
   index: number;
   onClick: () => void;
-  active: boolean;
   fadeIn?: FadeSettings;
   fadeOut?: FadeSettings;
   placeholder: boolean;
   onLoseFocus: () => void;
 };
 
-export function Thumbnail({
-  slide,
-  index,
-  onClick,
-  active,
-  fadeIn,
-  fadeOut,
-  placeholder,
-  onLoseFocus,
-}: ThumbnailProps) {
+export function Thumbnail({ slide, index, onClick, fadeIn, fadeOut, placeholder, onLoseFocus }: ThumbnailProps) {
   const ref = React.useRef<HTMLButtonElement>(null);
   const { render, styles, labels } = useLightboxProps();
-  const { slides, currentIndex } = useLightboxState();
+  const { slides, globalIndex } = useLightboxState();
   const { getOwnerDocument } = useDocumentContext();
   const { width, height, imageFit } = useThumbnailsProps();
   const rect = { width, height };
+  const active = index === globalIndex;
 
   const onLoseFocusCallback = useEventCallback(onLoseFocus);
 
@@ -111,10 +102,6 @@ export function Thumbnail({
     }
   }, [fadeOut, onLoseFocusCallback, getOwnerDocument]);
 
-  const thumbnailLabel = translateLabel(labels, "{{index}} of {{slidesLength}}")
-    .replace("{{index}}", String(index + 1))
-    .replace("{{slidesLength}}", String(slides.length));
-
   return (
     <button
       ref={ref}
@@ -143,8 +130,8 @@ export function Thumbnail({
         ...styles.thumbnail,
       }}
       onClick={onClick}
-      aria-label={thumbnailLabel}
-      aria-current={index === currentIndex ? true : undefined}
+      aria-current={active ? true : undefined}
+      aria-label={translateSlideCounter(labels, slides, index)}
     >
       {slide && renderThumbnail({ slide, render, rect, imageFit })}
     </button>

src/plugins/thumbnails/ThumbnailsTrack.tsx

@@ -11,7 +11,6 @@ import {
   cssClass,
   cssVar,
   getSlide,
-  getSlideIndex,
   getSlideKey,
   hasSlides,
   Slide,
@@ -161,7 +160,6 @@ export function ThumbnailsTrack({ visible, containerRef }: ThumbnailsTrackProps)
     >
       <nav
         ref={track}
-        role="navigation"
         style={styles.thumbnailsTrack}
         className={clsx(cssClass(cssPrefix("track")), cssClass(CLASS_FLEX_CENTER))}
         aria-label={translateLabel(labels, "Thumbnails")}
@@ -197,9 +195,8 @@ export function ThumbnailsTrack({ visible, containerRef }: ThumbnailsTrackProps)
           return (
             <Thumbnail
               key={key}
-              index={getSlideIndex(index, slides.length)}
+              index={index}
               slide={slide}
-              active={index === globalIndex}
               fadeIn={fadeIn}
               fadeOut={fadeOut}
               placeholder={!slide}

src/types.ts

@@ -217,7 +217,7 @@ export interface ControllerSettings {
   /** @deprecated for internal use only */
   touchAction: "none" | "pan-y";
   // TODO v4: remove
-  /** @deprecated describe the carousel for AT even if it is contained in a dialog */
+  /** @deprecated for internal use only */
   aria: boolean;
   /** if `true`, close the lightbox on pull-up gesture */
   closeOnPullUp: boolean;

src/utils.ts

@@ -43,6 +43,12 @@ export function label(labels: Labels | undefined, defaultLabel: Label) {
   return translateLabel(labels, defaultLabel);
 }
 
+export function translateSlideCounter(labels: Labels | undefined, slides: Slide[], index: number) {
+  return translateLabel(labels, "{{index}} of {{slidesLength}}")
+    .replace("{{index}}", String(getSlideIndex(index, slides.length) + 1))
+    .replace("{{slidesLength}}", String(slides.length));
+}
+
 export function cleanup(...cleaners: (() => void)[]) {
   return () => {
     cleaners.forEach((cleaner) => {