Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Error Overlay] New design #75679

Merged
merged 52 commits into from
Feb 10, 2025
Merged

[Error Overlay] New design #75679

merged 52 commits into from
Feb 10, 2025

Conversation

raunofreiberg
Copy link
Member

@raunofreiberg raunofreiberg commented Feb 5, 2025

This PR implements a new design for the experimental Next.js Error Overlay:

  • Border-less body design
  • Notch cut-out for the navigation and version info
image

@ijjk ijjk added CI approved Approve running CI for fork type: next labels Feb 5, 2025
@ijjk
Copy link
Member

ijjk commented Feb 5, 2025

Tests Passed

@ijjk
Copy link
Member

ijjk commented Feb 5, 2025

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
buildDuration 16.3s 14.6s N/A
buildDurationCached 13.9s 11.7s N/A
nodeModulesSize 392 MB 393 MB ⚠️ +34.5 kB
nextStartRea..uration (ms) 387ms 410ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
5306-HASH.js gzip 54.2 kB 54.2 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.46 kB 5.46 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 240 B 242 B N/A
main-HASH.js gzip 34.5 kB 34.5 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.59 kB 4.58 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.35 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.59 kB 3.59 kB
Client Build Manifests
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
_buildManifest.js gzip 748 B 747 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
index.html gzip 523 B 524 B N/A
link.html gzip 538 B 539 B N/A
withRouter.html gzip 519 B 520 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
edge-ssr.js gzip 130 kB 130 kB N/A
page.js gzip 211 kB 211 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
middleware-b..fest.js gzip 673 B 672 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.3 kB 31.3 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes Overall increase ⚠️
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
app-page-exp...dev.js gzip 394 kB 394 kB ⚠️ +507 B
app-page-exp..prod.js gzip 132 kB 132 kB
app-page-tur..prod.js gzip 145 kB 145 kB
app-page-tur..prod.js gzip 141 kB 141 kB
app-page.run...dev.js gzip 381 kB 381 kB ⚠️ +541 B
app-page.run..prod.js gzip 128 kB 128 kB
app-route-ex...dev.js gzip 39.3 kB 39.3 kB
app-route-ex..prod.js gzip 25.6 kB 25.6 kB
app-route-tu..prod.js gzip 25.6 kB 25.6 kB
app-route-tu..prod.js gzip 25.4 kB 25.4 kB
app-route.ru...dev.js gzip 40.9 kB 40.9 kB
app-route.ru..prod.js gzip 25.4 kB 25.4 kB
dist_client_...dev.js gzip 356 B 356 B
dist_client_...dev.js gzip 349 B 349 B
pages-api-tu..prod.js gzip 9.69 kB 9.69 kB
pages-api.ru...dev.js gzip 11.8 kB 11.8 kB
pages-api.ru..prod.js gzip 9.68 kB 9.68 kB
pages-turbo...prod.js gzip 21.9 kB 21.9 kB
pages.runtim...dev.js gzip 31.5 kB 31.5 kB
pages.runtim..prod.js gzip 21.9 kB 21.9 kB
server.runti..prod.js gzip 60.5 kB 60.5 kB
Overall change 1.67 MB 1.67 MB ⚠️ +1.05 kB
build cache
vercel/next.js canary vercel/next.js rf/overlay-cleanup Change
0.pack gzip 2.1 MB 2.1 MB N/A
index.pack gzip 75.7 kB 75 kB N/A
Overall change 0 B 0 B
Diff details
Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Commit: c17d634

@raunofreiberg raunofreiberg changed the title [Error Overlay] TBD [Error Overlay] New design Feb 10, 2025
Comment on lines -105 to -110
&[data-animate='true'] {
filter: blur(4px);
animation: fadeIn 250ms var(--timing-swift) forwards
calc(var(--index) * 25ms);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dropping frames on Safari with large lists, lets kill

Comment on lines -59 to -60
dialog.style.height = `${initialDialogHeight.current + ignoreListHeight}px`
setIsIgnoreListOpen(!isIgnoreListOpen)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just simplifies the logic, we don't need to add the height manually, parent ResizeObserver will do its job

@@ -92,7 +92,7 @@ export const Terminal: React.FC<TerminalProps> = function Terminal({
const fileExtension = stackFrame?.file?.split('.').pop()

return (
<div data-nextjs-terminal>
<div data-nextjs-codeframe>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing crazy here, just reusing styles from the code frame component

Comment on lines -12 to -16
if (!versionInfo) return null
const { staleness } = versionInfo
let { text, indicatorClass, title } = getStaleness(versionInfo)

if (!text) return null
Copy link
Member Author

@raunofreiberg raunofreiberg Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component should just naively render whatever it could, we check for null in the parent

@raunofreiberg raunofreiberg marked this pull request as ready for review February 10, 2025 15:27

display: flex;
flex-direction: column;
flex-direction: column-reverse;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reversed so we can leverage natural DOM order to keep the notch navigation a layer above the dialog contents on the Z-axis without explicitly declaring z-index

Comment on lines -54 to -63
if (buttonLeft.current) {
buttonLeft.current.focus()
}
handlePrevious && handlePrevious()
} else if (e.key === 'ArrowRight') {
e.preventDefault()
e.stopPropagation()
if (buttonRight.current) {
buttonRight.current.focus()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these removed?

@ijjk ijjk added the tests label Feb 10, 2025
@devjiwonchoi devjiwonchoi merged commit f859963 into canary Feb 10, 2025
130 checks passed
@devjiwonchoi devjiwonchoi deleted the rf/overlay-cleanup branch February 10, 2025 18:14
devjiwonchoi added a commit that referenced this pull request Feb 10, 2025
Following up of #75679

### Why?

Clicking the navigation escapes the overlay. It is because the
navigation component is outside of the dialog component.

#### Before


https://github.com/user-attachments/assets/e7c63261-c44b-4af7-936d-1750dff07527

#### After


https://github.com/user-attachments/assets/979421fa-fb71-481d-bd58-6703298db28b

### How?

Since we already have logic to prevent closing overlay when clicked
outside the component for dev indicator, add the header to this CSS
selectors list as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI approved Approve running CI for fork tests type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants