Compare commits

...

2 Commits

Author SHA1 Message Date
lebaudantoine c75e40b0ee ️(frontend) revert GridLayout re-render optimization too
Out of precaution, also revert the previous GridLayout re-render
optimization to avoid any layout regression alongside the
CarouselLayout revert.

The useSize-based re-render optimization will be reintroduced in a
dedicated small PR and release. That will also be a good occasion to
polish the layout code along the way.
2026-08-04 20:00:46 +02:00
lebaudantoine fe6c3bacaf ️(frontend) revert CarouselLayout re-render optimization
The previous optimization of the CarouselLayout was broken: the
approach did not hold in practice, and strict-mode rendering was
hiding the issue during development.

Revert the change for now and revisit the optimization later with a
sounder approach.
2026-08-04 19:43:21 +02:00
2 changed files with 47 additions and 136 deletions
@@ -3,7 +3,6 @@ import { getScrollBarWidth } from '@livekit/components-core'
import * as React from 'react'
import { TrackLoop, useVisualStableUpdate } from '@livekit/components-react'
import { useSize } from '@/features/rooms/livekit/hooks/useResizeObserver'
import { useCallback, useEffect, useLayoutEffect } from 'react'
const MIN_HEIGHT = 130
const MIN_WIDTH = 140
@@ -11,72 +10,6 @@ const MIN_VISIBLE_TILES = 1
const ASPECT_RATIO = 16 / 10
const ASPECT_RATIO_INVERT = (1 - ASPECT_RATIO) * -1
type CarouselOrientation = 'vertical' | 'horizontal'
interface CarouselLayoutState {
orientation: CarouselOrientation
maxVisibleTiles: number
}
interface CarouselLayoutObserverProps {
asideEl: React.RefObject<HTMLDivElement>
orientation?: CarouselOrientation
onLayoutChange: (layout: CarouselLayoutState) => void
}
const CarouselLayoutObserver = ({
orientation,
asideEl,
onLayoutChange,
}: CarouselLayoutObserverProps) => {
const { width, height } = useSize(asideEl)
// Hysteresis memory: avoids flapping between N and N+1 tiles when the
// container size hovers around a breakpoint. A ref (not state) because
// updating it must not trigger a re-render.
const prevTilesRef = React.useRef(0)
const carouselOrientation: CarouselOrientation =
orientation ?? (height >= width ? 'vertical' : 'horizontal')
const tileSpan =
carouselOrientation === 'vertical'
? Math.max(width * ASPECT_RATIO_INVERT, MIN_HEIGHT)
: Math.max(height * ASPECT_RATIO, MIN_WIDTH)
const scrollBarWidth = getScrollBarWidth()
const availableSpan =
(carouselOrientation === 'vertical' ? height : width) - scrollBarWidth
const tilesThatFit = Math.max(availableSpan / tileSpan, MIN_VISIBLE_TILES)
let maxVisibleTiles: number
if (Math.abs(tilesThatFit - prevTilesRef.current) < 0.5) {
// Within the dead zone: keep the previous count.
maxVisibleTiles = Math.round(prevTilesRef.current)
} else {
maxVisibleTiles = Math.round(tilesThatFit)
prevTilesRef.current = tilesThatFit
}
// Apply cosmetic layout output straight to the DOM.
useLayoutEffect(() => {
const el = asideEl.current
if (!el) return
el.dataset.lkOrientation = carouselOrientation
el.style.setProperty('--lk-max-visible-tiles', maxVisibleTiles.toString())
}, [asideEl, carouselOrientation, maxVisibleTiles])
// Report upward only what the parent actually needs for
// `useVisualStableUpdate` (and only when it changes — see parent handler).
useEffect(() => {
onLayoutChange({ orientation: carouselOrientation, maxVisibleTiles })
}, [carouselOrientation, maxVisibleTiles, onLayoutChange])
return null
}
/** @public */
export interface CarouselLayoutProps extends React.HTMLAttributes<HTMLMediaElement> {
tracks: TrackReferenceOrPlaceholder[]
@@ -107,42 +40,52 @@ export function CarouselLayout({
...props
}: CarouselLayoutProps) {
const asideEl = React.useRef<HTMLDivElement>(null)
const [prevTiles, setPrevTiles] = React.useState(0)
const { width, height } = useSize(asideEl)
const carouselOrientation = orientation
? orientation
: height >= width
? 'vertical'
: 'horizontal'
const [layout, setLayout] = React.useState<CarouselLayoutState>({
orientation: orientation ?? 'vertical',
maxVisibleTiles: MIN_VISIBLE_TILES,
})
const tileSpan =
carouselOrientation === 'vertical'
? Math.max(width * ASPECT_RATIO_INVERT, MIN_HEIGHT)
: Math.max(height * ASPECT_RATIO, MIN_WIDTH)
const scrollBarWidth = getScrollBarWidth()
// Stable callback + identity check: the parent only re-renders when the
// derived layout genuinely changed, not on every resize tick.
const handleLayoutChange = useCallback((next: CarouselLayoutState) => {
setLayout((prev) =>
prev.orientation === next.orientation &&
prev.maxVisibleTiles === next.maxVisibleTiles
? prev
: next
)
}, [])
const tilesThatFit =
carouselOrientation === 'vertical'
? Math.max((height - scrollBarWidth) / tileSpan, MIN_VISIBLE_TILES)
: Math.max((width - scrollBarWidth) / tileSpan, MIN_VISIBLE_TILES)
const sortedTiles = useVisualStableUpdate(tracks, layout.maxVisibleTiles)
let maxVisibleTiles = Math.round(tilesThatFit)
if (Math.abs(tilesThatFit - prevTiles) < 0.5) {
maxVisibleTiles = Math.round(prevTiles)
} else if (prevTiles !== tilesThatFit) {
setPrevTiles(tilesThatFit)
}
const sortedTiles = useVisualStableUpdate(tracks, maxVisibleTiles)
React.useLayoutEffect(() => {
if (asideEl.current) {
asideEl.current.dataset.lkOrientation = carouselOrientation
asideEl.current.style.setProperty(
'--lk-max-visible-tiles',
maxVisibleTiles.toString()
)
}
}, [maxVisibleTiles, carouselOrientation])
return (
<>
<CarouselLayoutObserver
asideEl={asideEl}
orientation={orientation}
onLayoutChange={handleLayoutChange}
/>
{/* `key` intentionally remounts the container when orientation flips, */}
{/* which resets scroll position and re-runs the observer measurement. */}
<aside
key={layout.orientation}
className="lk-carousel"
ref={asideEl}
{...props}
>
<TrackLoop tracks={sortedTiles}>{props.children}</TrackLoop>
</aside>
</>
<aside
key={carouselOrientation}
className="lk-carousel"
ref={asideEl}
{...props}
>
<TrackLoop tracks={sortedTiles}>{props.children}</TrackLoop>
</aside>
)
}
@@ -10,36 +10,8 @@ import { mergeProps } from '@/utils/mergeProps'
import { PaginationIndicator } from './PaginationIndicator'
import { useGridLayout } from '../hooks/useGridLayout'
import { PaginationControl } from './PaginationControl'
import { useEffect, useRef, useState } from 'react'
import { useSpeakerPromotionTrigger } from '../hooks/useSpeakerPromotionTrigger'
interface GridLayoutObserverProps {
gridEl: React.RefObject<HTMLDivElement>
trackCount: number
onMaxTilesChange: (maxTiles: number) => void
}
/**
* Headless component that runs the layout calculation in isolation and
* reports the resulting tile capacity upward.
*
* `useGridLayout` re-renders its host on every layout recalculation
* (e.g. container resizes). Rendering it in a null child means only this
* component churns; the parent `GridLayout` re-renders solely when
* `maxTiles` actually changes, since `setState` bails out on equal values.
*/
const GridLayoutObserver = ({
gridEl,
trackCount,
onMaxTilesChange,
}: GridLayoutObserverProps) => {
const { layout } = useGridLayout(gridEl, trackCount)
useEffect(() => {
onMaxTilesChange(layout.maxTiles)
}, [onMaxTilesChange, layout.maxTiles])
return null
}
/** @public */
export interface GridLayoutProps
@@ -67,14 +39,15 @@ export interface GridLayoutProps
* @public
*/
export function GridLayout({ tracks, ...props }: GridLayoutProps) {
const gridEl = useRef<HTMLDivElement>(null)
const [maxTiles, setMaxTiles] = useState(1)
const gridEl = React.createRef<HTMLDivElement>()
const elementProps = React.useMemo(
() => mergeProps(props, { className: 'lk-grid-layout' }),
[props]
)
const pagination = usePagination(maxTiles, tracks)
const { layout } = useGridLayout(gridEl, tracks.length)
const pagination = usePagination(layout.maxTiles, tracks)
useSpeakerPromotionTrigger(pagination.tracks)
useSwipe(gridEl, {
@@ -88,13 +61,8 @@ export function GridLayout({ tracks, ...props }: GridLayoutProps) {
data-lk-pagination={pagination.totalPageCount > 1}
{...elementProps}
>
<GridLayoutObserver
gridEl={gridEl}
trackCount={tracks.length}
onMaxTilesChange={setMaxTiles}
/>
<TrackLoop tracks={pagination.tracks}>{props.children}</TrackLoop>
{tracks.length > maxTiles && (
{tracks.length > layout.maxTiles && (
<>
<PaginationIndicator
totalPageCount={pagination.totalPageCount}