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.

Sign up for GitHub

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

Jump to bottom

[DevOverlay] Decouple Dialog component from Error Overlay #74638

Merged
merged 2 commits into from
Jan 10, 2025
Merged

[DevOverlay] Decouple Dialog component from Error Overlay #74638

merged 2 commits into from
Jan 10, 2025

Conversation

devjiwonchoi
Copy link
Member

@devjiwonchoi devjiwonchoi commented Jan 8, 2025 •
edited
Loading

This PR decoupled the Dialog component with the "Error" overlay. Removed unused styles in the progress.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Jan 8, 2025
This was referenced Jan 8, 2025
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@devjiwonchoi Graphite App
Copy link
Member Author

devjiwonchoi commented Jan 8, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from 5d7e7f9 to c132925 Compare January 8, 2025 12:11
@devjiwonchoi devjiwonchoi changed the base branch from 01-08-_devoverlay_apply_turbopack_styling to 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message January 8, 2025 12:11
@devjiwonchoi devjiwonchoi marked this pull request as ready for review January 8, 2025 12:14
@ijjk
Copy link
Member

ijjk commented Jan 8, 2025 •
edited
Loading

Tests Passed

@ijjk
Copy link
Member

ijjk commented Jan 8, 2025 •
edited
Loading

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
buildDuration 36.3s 37.2s ⚠️ +871ms
buildDurationCached 33.9s 30.8s N/A
nodeModulesSize 417 MB 417 MB ⚠️ +44.5 kB
nextStartRea..uration (ms) 938ms 923ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
5306-HASH.js gzip 53.3 kB 53.3 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.44 kB 5.44 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.1 kB 34.1 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 01-08-_devoverlay_decouple_dialog_component_from_error_overlay 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 01-08-_devoverlay_decouple_dialog_component_from_error_overlay 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.57 kB 4.57 kB N/A
index-HASH.js gzip 268 B 268 B ✓
link-HASH.js gzip 2.35 kB 2.34 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 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
_buildManifest.js gzip 749 B 747 B N/A
Overall change 0 B 0 B ✓
Rendered Page Sizes
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
index.html gzip 522 B 521 B N/A
link.html gzip 538 B 535 B N/A
withRouter.html gzip 518 B 518 B ✓
Overall change 518 B 518 B ✓
Edge SSR bundle Size
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 207 kB 207 kB N/A
Overall change 0 B 0 B ✓
Middleware size
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
middleware-b..fest.js gzip 671 B 667 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.2 kB 31.2 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 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
274-experime...dev.js gzip 322 B 322 B ✓
274.runtime.dev.js gzip 314 B 314 B ✓
app-page-exp...dev.js gzip 368 kB 368 kB ⚠️ +187 B
app-page-exp..prod.js gzip 129 kB 129 kB ✓
app-page-tur..prod.js gzip 142 kB 142 kB ✓
app-page-tur..prod.js gzip 138 kB 138 kB ✓
app-page.run...dev.js gzip 356 kB 356 kB ⚠️ +197 B
app-page.run..prod.js gzip 126 kB 126 kB ✓
app-route-ex...dev.js gzip 37.6 kB 37.6 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 39.2 kB 39.2 kB ✓
app-route.ru..prod.js gzip 25.4 kB 25.4 kB ✓
pages-api-tu..prod.js gzip 9.69 kB 9.69 kB ✓
pages-api.ru...dev.js gzip 11.6 kB 11.6 kB ✓
pages-api.ru..prod.js gzip 9.68 kB 9.68 kB ✓
pages-turbo...prod.js gzip 21.7 kB 21.7 kB ✓
pages.runtim...dev.js gzip 27.5 kB 27.5 kB ✓
pages.runtim..prod.js gzip 21.7 kB 21.7 kB ✓
server.runti..prod.js gzip 916 kB 916 kB ✓
Overall change 2.46 MB 2.46 MB ⚠️ +384 B
build cache
vercel/next.js canary vercel/next.js 01-08-_devoverlay_decouple_dialog_component_from_error_overlay Change
0.pack gzip 2.09 MB 2.09 MB N/A
index.pack gzip 76 kB 76 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 
failed to diff
Diff for app-page.runtime.dev.js 
failed to diff
Commit: dec59d3

@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from daa0478 to fe5331c Compare January 8, 2025 14:00
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from c132925 to ed56af2 Compare January 8, 2025 14:01
@devjiwonchoi devjiwonchoi mentioned this pull request Jan 8, 2025
Merged
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from fe5331c to 79d144e Compare January 9, 2025 04:45
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from ed56af2 to f1d1fa4 Compare January 9, 2025 04:46
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from 79d144e to ea87b89 Compare January 9, 2025 07:24
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from f1d1fa4 to 8f3be6e Compare January 9, 2025 07:24
@devjiwonchoi devjiwonchoi mentioned this pull request Jan 9, 2025
Merged
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from ea87b89 to 8453b68 Compare January 9, 2025 09:40
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from 8f3be6e to eccabb2 Compare January 9, 2025 09:40
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from d58ce46 to 171d8dd Compare January 10, 2025 09:04
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from d2993f5 to 209241d Compare January 10, 2025 09:04
eps1lon
eps1lon approved these changes Jan 10, 2025
View reviewed changes
...t/src/client/components/react-dev-overlay/_experimental/internal/components/Dialog/styles.ts
box-shadow: var(--shadow-md);
max-height: calc(100% - 56px);
position: relative;
z-index: 50;
Copy link
Member

Choose a reason for hiding this comment

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

z-index is always a good example for extraction into variables. But no need to follow-up. Just good to keep in mind when we add more layering.

Copy link
Member

Choose a reason for hiding this comment

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

yea probably good to have some constants z-indexes, so we can order them easily. I've seen z-index: 2, 48 and 50 in error overlay which vary a lot

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, will follow up!

@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from 171d8dd to f7e946b Compare January 10, 2025 11:56
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from 209241d to d0813da Compare January 10, 2025 11:56
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from f7e946b to d6a00dd Compare January 10, 2025 12:08
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from d0813da to be27d0d Compare January 10, 2025 12:08
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from d6a00dd to 05987be Compare January 10, 2025 12:31
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from be27d0d to abc3bbe Compare January 10, 2025 12:31
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from 05987be to 04bd08a Compare January 10, 2025 13:11
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from abc3bbe to 00bf4d8 Compare January 10, 2025 13:11
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch 3 times, most recently from ccb7f81 to dc00432 Compare January 10, 2025 14:21
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from 00bf4d8 to b92753d Compare January 10, 2025 14:21
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from dc00432 to ccb7f81 Compare January 10, 2025 14:21
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch 2 times, most recently from 0b2ecf0 to a3fef5f Compare January 10, 2025 14:41
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message branch from 152eaaf to b09bb48 Compare January 10, 2025 15:55
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch from a3fef5f to dec59d3 Compare January 10, 2025 15:55
Base automatically changed from 01-08-_devoverlay_polish_style_details_for_error_type_label_and_message to canary January 10, 2025 17:08
@devjiwonchoi devjiwonchoi force-pushed the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch 2 times, most recently from 2adbb2d to 5492a54 Compare January 10, 2025 17:09
@graphite-app Graphite App
Copy link

graphite-app bot commented Jan 10, 2025

Merge activity

  • Jan 10, 12:10 PM EST: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.

@devjiwonchoi devjiwonchoi merged commit 4ce61ca into canary Jan 10, 2025
13 checks passed
@devjiwonchoi devjiwonchoi deleted the 01-08-_devoverlay_decouple_dialog_component_from_error_overlay branch January 10, 2025 17:10
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gaojude gaojude gaojude approved these changes

@eps1lon eps1lon eps1lon approved these changes

@huozhi huozhi Awaiting requested review from huozhi

Assignees
No one assigned
Labels
created-by: Next.js team PRs by the Next.js team. locked type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@devjiwonchoi @ijjk @huozhi @eps1lon @gaojude