Skip to content

Commit 6a8bbcc

Browse files
committed
feat: simplify VirtualElemt to avoid racecondition between useEffect and useLayoutEffect. When using the 'contain: 'size layout' option, a static placeholder is fine Chrome
1 parent aa92e49 commit 6a8bbcc

File tree

1 file changed

+13
-109
lines changed

1 file changed

+13
-109
lines changed
Lines changed: 13 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,7 @@
1-
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useState, useRef } from 'react'
1+
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react'
22
import { InView } from 'react-intersection-observer'
33
import { getViewPortScrollingState } from './viewPort'
44

5-
interface IElementMeasurements {
6-
width: string | number
7-
clientHeight: number
8-
marginLeft: string | number | undefined
9-
marginRight: string | number | undefined
10-
marginTop: string | number | undefined
11-
marginBottom: string | number | undefined
12-
id: string | undefined
13-
}
14-
155
const IDLE_CALLBACK_TIMEOUT = 100
166
// Increased delay for Safari, as Safari doesn't have scroll time when using 'smooth' scroll
177
const SAFARI_VISIBILITY_DELAY = /^((?!chrome|android).)*safari/i.test(navigator.userAgent) ? 100 : 0
@@ -64,30 +54,25 @@ export function VirtualElement({
6454
}>): JSX.Element | null {
6555
const [inView, setInView] = useState(initialShow ?? false)
6656
const [isShowingChildren, setIsShowingChildren] = useState(inView)
67-
const [measurements, setMeasurements] = useState<IElementMeasurements | null>(null)
68-
const [ref, setRef] = useState<HTMLDivElement | null>(null)
69-
const [childRef, setChildRef] = useState<HTMLElement | null>(null)
7057

7158
// Track the last visibility change to debounce
7259
const lastVisibilityChangeRef = useRef<number>(0)
7360

74-
const isMeasured = !!measurements
75-
7661
const styleObj = useMemo<React.CSSProperties>(
7762
() => ({
78-
width: width ?? measurements?.width ?? 'auto',
79-
height: (measurements?.clientHeight ?? placeholderHeight ?? '0') + 'px',
80-
marginTop: measurements?.marginTop,
81-
marginLeft: measurements?.marginLeft,
82-
marginRight: measurements?.marginRight,
83-
marginBottom: measurements?.marginBottom,
63+
width: width ?? 'auto',
64+
height: (placeholderHeight ?? '0') + 'px',
65+
marginTop: 0,
66+
marginLeft: 0,
67+
marginRight: 0,
68+
marginBottom: 0,
8469
// These properties are used to ensure that if a prior element is changed from
8570
// placeHolder to element, the position of visible elements are not affected.
8671
contentVisibility: 'auto',
87-
containIntrinsicSize: `0 ${measurements?.clientHeight ?? placeholderHeight ?? '0'}px`,
72+
containIntrinsicSize: `0 ${placeholderHeight ?? '0'}px`,
8873
contain: 'size layout',
8974
}),
90-
[width, measurements, placeholderHeight]
75+
[width, placeholderHeight]
9176
)
9277

9378
const onVisibleChanged = useCallback(
@@ -142,9 +127,6 @@ export function VirtualElement({
142127
}
143128
idleCallback = window.requestIdleCallback(
144129
() => {
145-
if (childRef) {
146-
setMeasurements(measureElement(childRef))
147-
}
148130
setIsShowingChildren(false)
149131
},
150132
{
@@ -164,43 +146,9 @@ export function VirtualElement({
164146
window.clearTimeout(optimizeTimeout)
165147
}
166148
}
167-
}, [childRef, inView, measurements])
168-
169-
const showPlaceholder = !isShowingChildren && (!initialShow || isMeasured)
170-
171-
useLayoutEffect(() => {
172-
if (!ref || showPlaceholder) return
173-
174-
const el = ref?.firstElementChild
175-
if (!el || el.classList.contains('virtual-element-placeholder') || !(el instanceof HTMLElement)) return
176-
177-
setChildRef(el)
178-
179-
let idleCallback: number | undefined
180-
const refreshSizingTimeout = window.setTimeout(() => {
181-
idleCallback = window.requestIdleCallback(
182-
() => {
183-
const newMeasurements = measureElement(el)
184-
setMeasurements(newMeasurements)
185-
186-
// Set CSS variable for expected height on parent element
187-
if (ref && newMeasurements) {
188-
ref.style.setProperty('--expected-height', `${newMeasurements.clientHeight}px`)
189-
}
190-
},
191-
{
192-
timeout: IDLE_CALLBACK_TIMEOUT,
193-
}
194-
)
195-
}, 1000)
149+
}, [inView])
196150

197-
return () => {
198-
if (idleCallback) {
199-
window.cancelIdleCallback(idleCallback)
200-
}
201-
window.clearTimeout(refreshSizingTimeout)
202-
}
203-
}, [ref, showPlaceholder, measurements])
151+
const showPlaceholder = !isShowingChildren && !initialShow
204152

205153
return (
206154
<InView
@@ -210,57 +158,13 @@ export function VirtualElement({
210158
className={className}
211159
as="div"
212160
>
213-
<div
214-
ref={setRef}
215-
style={
216-
// We need to set undefined if the height is not known, to allow the parent to calculate the height
217-
measurements
218-
? {
219-
height: measurements.clientHeight + 'px',
220-
}
221-
: undefined
222-
}
223-
>
161+
<div>
224162
{showPlaceholder ? (
225-
<div
226-
id={measurements?.id ?? id}
227-
className={`virtual-element-placeholder ${placeholderClassName}`}
228-
style={styleObj}
229-
></div>
163+
<div id={id} className={`virtual-element-placeholder ${placeholderClassName}`} style={styleObj}></div>
230164
) : (
231165
children
232166
)}
233167
</div>
234168
</InView>
235169
)
236170
}
237-
238-
function measureElement(el: HTMLElement): IElementMeasurements | null {
239-
const style = window.getComputedStyle(el)
240-
241-
// Get children to be measured
242-
const segmentTimeline = el.querySelector('.segment-timeline')
243-
const dashboardPanel = el.querySelector('.rundown-view-shelf.dashboard-panel')
244-
245-
if (!segmentTimeline) return null
246-
247-
// Segment height
248-
const segmentRect = segmentTimeline.getBoundingClientRect()
249-
let totalHeight = segmentRect.height
250-
251-
// Dashboard panel height if present
252-
if (dashboardPanel) {
253-
const panelRect = dashboardPanel.getBoundingClientRect()
254-
totalHeight += panelRect.height
255-
}
256-
257-
return {
258-
width: style.width || 'auto',
259-
clientHeight: totalHeight,
260-
marginTop: style.marginTop || undefined,
261-
marginBottom: style.marginBottom || undefined,
262-
marginLeft: style.marginLeft || undefined,
263-
marginRight: style.marginRight || undefined,
264-
id: el.id,
265-
}
266-
}

0 commit comments

Comments
 (0)