Skip to content

Commit 5ff57ea

Browse files
[dev-overlay] Fix error dialog resizing logic (#77830)
This PR improves our height resizing strategy. Animating the height on page load or when the dialog opens is not ideal and it would be better if the dialog would just fade in and assume it's intrinsic height without motion. This is what happens before this PR when you open errors, and it can be distracting to experience such amount of motion at once: https://github.com/user-attachments/assets/eb03cc23-395e-4ff4-9ed2-ace1694ad917 The reason this happens is because some of the dialog contents load in async—for example the errors are actually suspended so that the message can be viewed without waiting for the stack frames to load: https://github.com/vercel/next.js/blob/1751db082d8c791e05ac1481a5036c8fb381d831/packages/next/src/client/components/react-dev-overlay/ui/container/errors.tsx#L202-L208 --- After this PR, we still measure the `height` of the dialog and animate it in response to user input, but only use the value when the height has stabilised. You can see what happens by observing the blue height value: https://github.com/user-attachments/assets/cf4c1e0f-5e2e-49ed-b94a-1df9822f20cc And another video from the fixture app: https://github.com/user-attachments/assets/a285253f-f320-4638-b19e-b7a27a434ee6 --- Closes NDX-983
1 parent b84a02b commit 5ff57ea

File tree

10 files changed

+128
-94
lines changed

10 files changed

+128
-94
lines changed

Diff for: packages/next/src/client/components/react-dev-overlay/ui/components/dialog/dialog.tsx

+1-17
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as React from 'react'
22
import { useOnClickOutside } from '../../hooks/use-on-click-outside'
3-
import { useMeasureHeight } from '../../hooks/use-measure-height'
43

54
export type DialogProps = {
65
children?: React.ReactNode
@@ -37,9 +36,6 @@ const Dialog: React.FC<DialogProps> = function Dialog({
3736
: undefined
3837
)
3938

40-
const ref = React.useRef<HTMLDivElement | null>(null)
41-
const [height, pristine] = useMeasureHeight(ref)
42-
4339
useOnClickOutside(
4440
dialogRef.current,
4541
CSS_SELECTORS_TO_EXCLUDE_ON_CLICK_OUTSIDE,
@@ -102,19 +98,7 @@ const Dialog: React.FC<DialogProps> = function Dialog({
10298
}}
10399
{...props}
104100
>
105-
<div
106-
ref={dialogResizerRef}
107-
data-nextjs-dialog-sizer
108-
// [x] Don't animate on initial load
109-
// [x] No duplicate elements
110-
// [x] Responds to content growth
111-
style={{
112-
height,
113-
transition: pristine ? undefined : 'height 250ms var(--timing-swift)',
114-
}}
115-
>
116-
<div ref={ref}>{children}</div>
117-
</div>
101+
{children}
118102
</div>
119103
)
120104
}

Diff for: packages/next/src/client/components/react-dev-overlay/ui/components/dialog/styles.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const styles = `
33
--next-dialog-radius: var(--rounded-xl);
44
--next-dialog-max-width: 960px;
55
--next-dialog-row-padding: 16px;
6-
--next-dialog-padding-x: 12px;
6+
--next-dialog-padding: 12px;
77
--next-dialog-notch-height: 42px;
88
--next-dialog-border-width: 1px;
99
@@ -14,7 +14,7 @@ const styles = `
1414
max-width: var(--next-dialog-max-width);
1515
margin-right: auto;
1616
margin-left: auto;
17-
scale: 0.98;
17+
scale: 0.97;
1818
opacity: 0;
1919
transition-property: scale, opacity;
2020
transition-duration: var(--transition-duration);
@@ -28,7 +28,7 @@ const styles = `
2828
[data-nextjs-scroll-fader][data-side="top"] {
2929
left: 1px;
3030
top: calc(var(--next-dialog-notch-height) + var(--next-dialog-border-width));
31-
width: calc(100% - var(--next-dialog-padding-x));
31+
width: calc(100% - var(--next-dialog-padding));
3232
opacity: 0;
3333
}
3434
}
@@ -82,7 +82,7 @@ const styles = `
8282
display: flex;
8383
flex-direction: column;
8484
position: relative;
85-
padding: 16px var(--next-dialog-padding-x);
85+
padding: var(--next-dialog-padding);
8686
}
8787
8888
[data-nextjs-dialog-content] > [data-nextjs-dialog-header] {

Diff for: packages/next/src/client/components/react-dev-overlay/ui/components/errors/error-overlay-layout/error-overlay-layout.test.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const renderTestComponent = () => {
2424
rendered={true}
2525
transitionDurationMs={200}
2626
isTurbopack={false}
27+
errorCount={1}
2728
versionInfo={{
2829
installed: '15.0.0',
2930
staleness: 'fresh',

Diff for: packages/next/src/client/components/react-dev-overlay/ui/components/errors/error-overlay-layout/error-overlay-layout.tsx

+50-23
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import type { ReadyRuntimeError } from '../../../../utils/get-error-by-type'
3636
import { EnvironmentNameLabel } from '../environment-name-label/environment-name-label'
3737
import { useFocusTrap } from '../dev-tools-indicator/utils'
3838
import { Fader } from '../../fader'
39+
import { Resizer } from '../../resizer'
3940

4041
export interface ErrorOverlayLayoutProps extends ErrorBaseProps {
4142
errorMessage: ErrorMessageType
@@ -59,6 +60,7 @@ export function ErrorOverlayLayout({
5960
errorType,
6061
children,
6162
errorCode,
63+
errorCount,
6264
error,
6365
debugInfo,
6466
isBuildError,
@@ -83,6 +85,10 @@ export function ErrorOverlayLayout({
8385
} as React.CSSProperties,
8486
}
8587

88+
const [animating, setAnimating] = React.useState(
89+
Boolean(transitionDurationMs)
90+
)
91+
8692
const faderRef = React.useRef<HTMLDivElement | null>(null)
8793
const hasFooter = Boolean(footerMessage || errorCode)
8894
const dialogRef = React.useRef<HTMLDivElement | null>(null)
@@ -95,9 +101,23 @@ export function ErrorOverlayLayout({
95101
}
96102
}
97103

104+
function onTransitionEnd({ propertyName, target }: React.TransitionEvent) {
105+
// We can only measure height after the `scale` transition ends,
106+
// otherwise we will measure height as a multiple of the animating value
107+
// which will give us an incorrect value.
108+
if (propertyName === 'scale' && target === dialogRef.current) {
109+
setAnimating(false)
110+
}
111+
}
112+
98113
return (
99114
<ErrorOverlayOverlay fixed={isBuildError} {...animationProps}>
100-
<div data-nextjs-dialog-root ref={dialogRef} {...animationProps}>
115+
<div
116+
data-nextjs-dialog-root
117+
onTransitionEnd={onTransitionEnd}
118+
ref={dialogRef}
119+
{...animationProps}
120+
>
101121
<ErrorOverlayNav
102122
runtimeErrors={runtimeErrors}
103123
activeIdx={activeIdx}
@@ -119,30 +139,37 @@ export function ErrorOverlayLayout({
119139
)
120140
}
121141
>
122-
<DialogContent>
123-
<ErrorOverlayDialogHeader>
124-
<div
125-
className="nextjs__container_errors__error_title"
126-
// allow assertion in tests before error rating is implemented
127-
data-nextjs-error-code={errorCode}
128-
>
129-
<span data-nextjs-error-label-group>
130-
<ErrorTypeLabel errorType={errorType} />
131-
{error.environmentName && (
132-
<EnvironmentNameLabel
133-
environmentName={error.environmentName}
134-
/>
135-
)}
136-
</span>
137-
<ErrorOverlayToolbar error={error} debugInfo={debugInfo} />
138-
</div>
139-
<ErrorMessage errorMessage={errorMessage} />
140-
</ErrorOverlayDialogHeader>
142+
<Resizer
143+
ref={dialogResizerRef}
144+
measure={!animating}
145+
data-nextjs-dialog-sizer
146+
>
147+
<DialogContent>
148+
<ErrorOverlayDialogHeader>
149+
<div
150+
className="nextjs__container_errors__error_title"
151+
// allow assertion in tests before error rating is implemented
152+
data-nextjs-error-code={errorCode}
153+
>
154+
<span data-nextjs-error-label-group>
155+
<ErrorTypeLabel errorType={errorType} />
156+
{error.environmentName && (
157+
<EnvironmentNameLabel
158+
environmentName={error.environmentName}
159+
/>
160+
)}
161+
</span>
162+
<ErrorOverlayToolbar error={error} debugInfo={debugInfo} />
163+
</div>
164+
<ErrorMessage errorMessage={errorMessage} />
165+
</ErrorOverlayDialogHeader>
166+
167+
<ErrorOverlayDialogBody>{children}</ErrorOverlayDialogBody>
168+
</DialogContent>
169+
</Resizer>
141170

142-
<ErrorOverlayDialogBody>{children}</ErrorOverlayDialogBody>
143-
</DialogContent>
144171
<ErrorOverlayBottomStack
145-
errorCount={runtimeErrors?.length ?? 0}
172+
errorCount={errorCount}
146173
activeIdx={activeIdx ?? 0}
147174
/>
148175
</ErrorOverlayDialog>

Diff for: packages/next/src/client/components/react-dev-overlay/ui/components/errors/error-overlay/error-overlay.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,21 @@ export interface ErrorBaseProps {
1313
transitionDurationMs: number
1414
isTurbopack: boolean
1515
versionInfo: OverlayState['versionInfo']
16+
errorCount: number
1617
}
1718

1819
export function ErrorOverlay({
1920
state,
2021
runtimeErrors,
2122
isErrorOverlayOpen,
2223
setIsErrorOverlayOpen,
24+
errorCount,
2325
}: {
2426
state: OverlayState
2527
runtimeErrors: ReadyRuntimeError[]
2628
isErrorOverlayOpen: boolean
2729
setIsErrorOverlayOpen: (value: boolean) => void
30+
errorCount: number
2831
}) {
2932
const isTurbopack = !!process.env.TURBOPACK
3033

@@ -38,6 +41,7 @@ export function ErrorOverlay({
3841
transitionDurationMs,
3942
isTurbopack,
4043
versionInfo: state.versionInfo,
44+
errorCount,
4145
}
4246

4347
if (state.buildError !== null) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { useEffect, useRef, useState } from 'react'
2+
3+
export function Resizer({
4+
children,
5+
measure,
6+
...props
7+
}: {
8+
children: React.ReactNode
9+
measure: boolean
10+
} & React.HTMLProps<HTMLDivElement> & { ref?: React.Ref<HTMLDivElement> }) {
11+
const ref = useRef<HTMLDivElement | null>(null)
12+
const [height, measuring] = useMeasureHeight(ref, measure)
13+
14+
return (
15+
<div
16+
{...props}
17+
// [x] Don't animate on initial load
18+
// [x] No duplicate elements
19+
// [x] Responds to content growth
20+
style={{
21+
height: measuring ? 'auto' : height,
22+
transition: 'height 250ms var(--timing-swift)',
23+
}}
24+
>
25+
<div ref={ref}>{children}</div>
26+
</div>
27+
)
28+
}
29+
30+
function useMeasureHeight(
31+
ref: React.RefObject<HTMLDivElement | null>,
32+
measure: boolean
33+
): [number, boolean] {
34+
const [height, setHeight] = useState<number>(0)
35+
const [measuring, setMeasuring] = useState<boolean>(true)
36+
37+
useEffect(() => {
38+
if (!measure) {
39+
return
40+
}
41+
42+
let timerId: number
43+
const el = ref.current
44+
45+
if (!el) {
46+
return
47+
}
48+
49+
const observer = new ResizeObserver(([{ contentRect }]) => {
50+
clearTimeout(timerId)
51+
52+
timerId = window.setTimeout(() => {
53+
setMeasuring(false)
54+
}, 100)
55+
56+
setHeight(contentRect.height)
57+
})
58+
59+
observer.observe(el)
60+
return () => observer.disconnect()
61+
// eslint-disable-next-line react-hooks/exhaustive-deps
62+
}, [measure])
63+
64+
return [height, measuring]
65+
}

Diff for: packages/next/src/client/components/react-dev-overlay/ui/container/errors.tsx

+1-13
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useState, useMemo, useEffect, useRef, Suspense } from 'react'
1+
import { useState, useMemo, useRef, Suspense } from 'react'
22
import type { DebugInfo } from '../../types'
33
import { Overlay } from '../components/overlay'
44
import { RuntimeError } from './runtime-error'
@@ -89,18 +89,6 @@ export function Errors({
8989
}: ErrorsProps) {
9090
const dialogResizerRef = useRef<HTMLDivElement | null>(null)
9191

92-
useEffect(() => {
93-
// Close the error overlay when pressing escape
94-
function handleKeyDown(event: KeyboardEvent) {
95-
if (event.key === 'Escape') {
96-
onClose()
97-
}
98-
}
99-
100-
document.addEventListener('keydown', handleKeyDown)
101-
return () => document.removeEventListener('keydown', handleKeyDown)
102-
}, [onClose])
103-
10492
const isLoading = useMemo<boolean>(() => {
10593
return runtimeErrors.length < 1
10694
}, [runtimeErrors.length])

Diff for: packages/next/src/client/components/react-dev-overlay/ui/dev-overlay.stories.tsx

+1-2
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,8 @@ export const Default: Story = {
9292
src={imgApp}
9393
style={{
9494
width: '100%',
95-
height: '100%',
95+
height: '100vh',
9696
objectFit: 'contain',
97-
filter: 'invert(1)',
9897
}}
9998
/>
10099
<DevOverlay

Diff for: packages/next/src/client/components/react-dev-overlay/ui/dev-overlay.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export function DevOverlay({
5050
<ErrorOverlay
5151
state={state}
5252
runtimeErrors={runtimeErrors}
53+
errorCount={totalErrorCount}
5354
isErrorOverlayOpen={isErrorOverlayOpen}
5455
setIsErrorOverlayOpen={setIsErrorOverlayOpen}
5556
/>

Diff for: packages/next/src/client/components/react-dev-overlay/ui/hooks/use-measure-height.ts

-35
This file was deleted.

0 commit comments

Comments
 (0)