Avoiding JS Code Smells for Layout and Transitions

In real-world React applications, components often start small but quickly grow into complex beasts that are hard to maintain. One pattern I noticed repeatedly is using useEffect for tasks that could either live elsewhere or be handled more elegantly with CSS.

In this post, I’m walking through a refactor of a video carousel component, highlighting:

• How to move side-effect logic into reusable hooks
• How to reduce code smells like detecting mobile via JavaScript
• Lessons for making your components cleaner, testable, and maintainable

---

The Problem

Imagine a carousel component that:

• Handles multiple slides, including videos
• Tracks which slide is active
• Detects viewport size to adjust layout
• Manages UI transitions

In many implementations, developers will handle transitions and viewport detection directly in the component using useEffect. While functional, this pattern creates long, cluttered components that are harder to test and maintain.

Here’s a simplified example of the “smelly” part of the component:

useEffect(() => {
  setIsMobile(window.innerWidth < 768);
  const handleResize = () => setIsMobile(window.innerWidth < 768);
  window.addEventListener('resize', handleResize);
  return () => window.removeEventListener('resize', handleResize);
}, []);

Why it’s a smell:

1. Adds extra state and triggers re-renders on every resize
2. Harder to maintain than CSS media queries
3. Blurs the line between layout (CSS) and behavior (JS)
4. Makes testing tricky

The Refactor: Using Hooks

To clean up the component, I moved all side-effect logic into custom hooks, leaving the component itself focused purely on rendering and interaction.

1. useTransition Hook

Handles slide transitions and timing:

import { useState, useEffect } from 'react';

export function useTransition(duration: number = 500) {
  const [isTransitioning, setIsTransitioning] = useState(false);

  useEffect(() => {
    if (!isTransitioning) return;
    const timeout = setTimeout(() => setIsTransitioning(false), duration);
    return () => clearTimeout(timeout);
  }, [isTransitioning, duration]);

  const startTransition = () => setIsTransitioning(true);

  return { isTransitioning, startTransition };
}

Benefits:

• Keeps transition logic reusable, testable, and isolated
• Hooks can be unit-tested independently

---

2. useViewport Hook

Detects viewport size only if needed, but in many cases, CSS would be enough:

import { useState, useEffect } from 'react';

export function useViewport(breakpoint: number = 768) {
  const [isMobile, setIsMobile] = useState(false);
  const [hasLoaded, setHasLoaded] = useState(false);

  useEffect(() => {
    const checkViewport = () => setIsMobile(window.innerWidth < breakpoint);

    checkViewport();
    setHasLoaded(true);

    window.addEventListener('resize', checkViewport);
    return () => window.removeEventListener('resize', checkViewport);
  }, [breakpoint]);

  return { isMobile, hasLoaded };
}

Smell discussion:

• If the only reason you’re using JS here is for styling/layout, CSS media queries are simpler and more maintainable:

.carousel {
  width: 900px;
}

@media (max-width: 768px) {
  .carousel {
    width: 300px;
  }
}

JS-based detection is only justified if you need conditional rendering or behavior that CSS can’t handle.

---

The Clean Component

Here’s the refactored carousel using the hooks:

export function VideoCarouselSection({ title, subtitle, slides = [], enableInlineVideo }: Props) {
  const isEven = slides.length % 2 === 0;
  const [currentIndex, setCurrentIndex] = useState(Math.floor(slides.length / 2));
  const [hasInteracted, setHasInteracted] = useState(false);

  const { isTransitioning, startTransition } = useTransition();
  const { isMobile, hasLoaded } = useViewport();

  const handleNext = () => {
    if (isTransitioning) return;
    startTransition();
    setCurrentIndex(prev => prev > slides.length - 2 ? (isEven ? 1 : 0) : prev + 1);
  };

  const handlePrev = () => {
    if (isTransitioning) return;
    startTransition();
    setCurrentIndex(prev => prev < (isEven ? 2 : 1) ? slides.length - 1 : prev - 1);
  };

  return (
    <section>
      {title && <h1>{title}</h1>}
      {subtitle && <p>{subtitle}</p>}

      <div className="carousel-wrapper">
        <Carousel
          active={currentIndex}
          next={handleNext}
          prev={handlePrev}
          config={{
            ...(hasLoaded ? { width: isMobile ? 300 : 150 } : {}),
            maxSlideWidth: 900,
          }}
          slides={slides.map(slide => (
            <Slide
              key={slide.id}
              slide={slide}
              enableInlineVideo={enableInlineVideo}
              nextSlide={handleNext}
              hasInteracted={hasInteracted}
              setHasInteracted={setHasInteracted}
            />
          ))}
        />
      </div>
    </section>
  );
}

Key benefits of this refactor:

1. Separation of concerns – hooks handle side effects; component handles rendering
2. Reusability – hooks can be used anywhere you need transitions or viewport awareness
3. Testability – hooks can be unit-tested independently
4. Cleaner layout – avoids unnecessary JS for things CSS can handle

1. Cleaner layout – avoids unnecessary JS for things CSS can handle

---

Takeaways for Engineers

• Refactor side effects into hooks – keeps components lean and easier to reason about
• Avoid JS for styling/layout if CSS media queries can do the job
• Always clean up effects – event listeners, timers, and subscriptions can leak memory
• Focus on maintainability – even small refactors have big payoffs over time