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

[dev-overlay] change dev indicator position by next config option #76077

Merged
merged 4 commits into from
Feb 17, 2025
Merged

[dev-overlay] change dev indicator position by next config option #76077

merged 4 commits into from
Feb 17, 2025

Conversation

devjiwonchoi
Copy link
Member

@devjiwonchoi devjiwonchoi commented Feb 15, 2025 •
edited
Loading

What?

This PR uses the devIndicators.position option to set the position of the dev indicator.
The default option is changed from "bottom-right" to "bottom-left" to sync the previous dev indicator behavior, but have covered backward compatibility for the default position of old overlay's build activity indicator.

Success Criteria

  • Does setting buildActivityPosition correctly set the dev indicator position?
  • Does setting the position correctly set the dev indicator position?
  • Does it default to "bottom-left"?
  • Does the dev indicator popover display properly based on the position?

Backwards Compatible:

Are there any breaking changes?

  • Is the position "bottom-right" on the old overlay if the user hasn't set the position?
  • Is the position "bottom-right" on the old overlay if the user hasn't set the buildActivityPosition?
  • Does the position follow the position option?
  • Does the position follow the buildActivityPosition option?
CleanShot.2025-02-15.at.23.03.46.mp4

Closes NDX-838

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Feb 15, 2025
@devjiwonchoi devjiwonchoi mentioned this pull request Feb 15, 2025
Merged
@devjiwonchoi Graphite App
Copy link
Member Author

devjiwonchoi commented Feb 15, 2025 •
edited
Loading

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

@devjiwonchoi devjiwonchoi mentioned this pull request Feb 15, 2025
Merged
6 tasks
@ijjk
Copy link
Member

ijjk commented Feb 15, 2025 •
edited
Loading

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
buildDuration 21.3s 17s N/A
buildDurationCached 16.3s 13.5s N/A
nodeModulesSize 393 MB 393 MB ⚠️ +26.4 kB
nextStartRea..uration (ms) 443ms 450ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
5306-HASH.js gzip 55.3 kB 55.3 kB N/A
7048.HASH.js gzip 168 B 168 B ✓
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 245 B 245 B ✓
main-HASH.js gzip 34.9 kB 34.9 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB ✓
Overall change 55.1 kB 55.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option 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 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
_app-HASH.js gzip 194 B 194 B ✓
_error-HASH.js gzip 193 B 192 B N/A
amp-HASH.js gzip 513 B 511 B N/A
css-HASH.js gzip 342 B 342 B ✓
dynamic-HASH.js gzip 1.84 kB 1.84 kB N/A
edge-ssr-HASH.js gzip 265 B 264 B N/A
head-HASH.js gzip 363 B 360 B N/A
hooks-HASH.js gzip 393 B 390 B N/A
image-HASH.js gzip 4.59 kB 4.59 kB N/A
index-HASH.js gzip 268 B 266 B N/A
link-HASH.js gzip 2.35 kB 2.35 kB ✓
routerDirect..HASH.js gzip 327 B 326 B N/A
script-HASH.js gzip 397 B 397 B ✓
withRouter-HASH.js gzip 325 B 325 B ✓
1afbb74e6ecf..834.css gzip 106 B 106 B ✓
Overall change 3.72 kB 3.72 kB ✓
Client Build Manifests
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option 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 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
index.html gzip 522 B 522 B ✓
link.html gzip 538 B 536 B N/A
withRouter.html gzip 519 B 518 B N/A
Overall change 522 B 522 B ✓
Edge SSR bundle Size
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option 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 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
middleware-b..fest.js gzip 676 B 674 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.4 kB 31.4 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 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
app-page-exp...dev.js gzip 395 kB 395 kB ⚠️ +190 B
app-page-exp..prod.js gzip 133 kB 133 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 384 kB 384 kB ⚠️ +181 B
app-page.run..prod.js gzip 129 kB 129 kB ✓
app-route-ex...dev.js gzip 39.4 kB 39.4 kB ✓
app-route-ex..prod.js gzip 25.7 kB 25.7 kB ✓
app-route-tu..prod.js gzip 25.7 kB 25.7 kB ✓
app-route-tu..prod.js gzip 25.5 kB 25.5 kB ✓
app-route.ru...dev.js gzip 39.1 kB 39.1 kB ✓
app-route.ru..prod.js gzip 25.5 kB 25.5 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.72 kB 9.72 kB ✓
pages-api.ru...dev.js gzip 11.8 kB 11.8 kB ✓
pages-api.ru..prod.js gzip 9.72 kB 9.72 kB ✓
pages-turbo...prod.js gzip 22 kB 22 kB ✓
pages.runtim...dev.js gzip 31.6 kB 31.6 kB ✓
pages.runtim..prod.js gzip 22 kB 22 kB ✓
server.runti..prod.js gzip 61.2 kB 61.2 kB ✓
Overall change 1.68 MB 1.68 MB ⚠️ +371 B
build cache
vercel/next.js canary vercel/next.js 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option Change
0.pack gzip 2.12 MB 2.12 MB N/A
index.pack gzip 77.1 kB 75.8 kB N/A
Overall change 0 B 0 B ✓
Diff details
Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

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: cb62e3b

@ijjk
Copy link
Member

ijjk commented Feb 15, 2025 •
edited
Loading

Tests Passed

@devjiwonchoi devjiwonchoi marked this pull request as ready for review February 15, 2025 15:53
@vercel Vercel
Copy link

vercel bot commented Feb 15, 2025 •
edited
Loading

Notifying the following users due to files changed in this PR based on this repo's notify modifiers:

@timneutkens, @ijjk, @shuding, @huozhi:

packages/next/src/server/config.ts

@devjiwonchoi devjiwonchoi marked this pull request as draft February 15, 2025 16:21
@devjiwonchoi devjiwonchoi mentioned this pull request Feb 15, 2025
Merged
3 tasks
@ijjk ijjk added the tests label Feb 15, 2025
@devjiwonchoi devjiwonchoi marked this pull request as ready for review February 15, 2025 17:04
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_deprecate_devindicators.buildactivityposition_and_rename_to_position_ branch from 1b3dd49 to 77db582 Compare February 17, 2025 07:51
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch from 4a96171 to fe4f0f0 Compare February 17, 2025 07:51
@devjiwonchoi devjiwonchoi mentioned this pull request Feb 17, 2025
Merged
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_deprecate_devindicators.buildactivityposition_and_rename_to_position_ branch 2 times, most recently from 65fc568 to 4718b68 Compare February 17, 2025 08:25
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch from fe4f0f0 to 118747f Compare February 17, 2025 08:25
@devjiwonchoi devjiwonchoi changed the base branch from 02-15-_dev-overlay_deprecate_devindicators.buildactivityposition_and_rename_to_position_ to graphite-base/76077 February 17, 2025 08:54
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch from 118747f to 099b370 Compare February 17, 2025 08:55
@devjiwonchoi devjiwonchoi changed the base branch from graphite-base/76077 to canary February 17, 2025 08:55
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch from 099b370 to aa39796 Compare February 17, 2025 08:55
@devjiwonchoi devjiwonchoi force-pushed the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch from aa39796 to cb62e3b Compare February 17, 2025 11:22
@devjiwonchoi devjiwonchoi merged commit c2085df into canary Feb 17, 2025
130 of 131 checks passed
@devjiwonchoi Graphite App
Copy link
Member Author

Merge activity

  • Feb 17, 6:46 AM EST: A user merged this pull request with Graphite.

@devjiwonchoi devjiwonchoi deleted the 02-15-_dev-overlay_change_dev_indicator_position_by_next_config_option branch February 17, 2025 11:46
devjiwonchoi added a commit that referenced this pull request Feb 17, 2025
@devjiwonchoi
test: replace explicit padding-bottom with devIndicators: false f…
f0887fa 
…or scroll position testing (#76105)

### Why?

There were explicit `paddingBottom: 60` set to fixtures due to the dev
indicator blocking the scroll position. Since an option to disable it
was added at #76079, this PR
replaced them with `devIndicators: false`.

x-ref:
#76077 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaojude gaojude gaojude approved these changes

@ztanner ztanner Awaiting requested review from ztanner

@huozhi huozhi Awaiting requested review from huozhi

@eps1lon eps1lon Awaiting requested review from eps1lon

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

Successfully merging this pull request may close these issues.
3 participants
@devjiwonchoi @ijjk @gaojude