rsnext/test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

240 lines
8.7 KiB
TypeScript
Raw Normal View History

import webdriver from 'next-webdriver'
import { createNextDescribe } from 'e2e-utils'
2023-01-26 07:55:49 +01:00
import { check } from 'next-test-utils'
createNextDescribe(
'router autoscrolling on navigation',
{
files: __dirname,
},
Fix scroll bailout logic when targeting fixed/sticky elements (#53873) ### What? When navigating to a new page with fixed or sticky positioned element as the first element, we were bailing on scroll to top behavior, which often isn't expected. ### Why? Currently, we decide to bail on scroll to top behavior on navigation if the content that is swapped into view is visible within the viewport. Since fixed/sticky positioned elements are often intended to be relative to the current viewport, it's most likely not the case that you'd want it to be considered in this heuristic. For example, if you were scrolled far down on a page, and you navigated to a page that makes use of a sticky header, you would not be scrolled to the top of the page because that sticky header is technically visible within the viewport. ### How? I've updated the previous implementation that was intended to skip targeting invisible elements to also skip over fixed or sticky elements. This should help by falling back to the next level of the layout tree to determine which element to scroll to. I've deleted the `// TODO-APP` comments as I couldn't think of a scenario in which we'd need a global scrollTop handler -- if we've bailed on every element up the tree, it's likely the page wasn't scrollable. Some additional considerations: - Is the warning helpful or annoying? - Is the parallel route trade-off an acceptable one? (ie, a parallel modal slot might not be considered in the content visibility check unless if it’s fixed positioned) Closes NEXT-1393 Fixes #47475
2023-08-15 15:31:39 +02:00
({ next, isNextDev }) => {
type BrowserInterface = Awaited<ReturnType<typeof webdriver>>
const getTopScroll = async (browser: BrowserInterface) =>
await browser.eval('document.documentElement.scrollTop')
const getLeftScroll = async (browser: BrowserInterface) =>
await browser.eval('document.documentElement.scrollLeft')
const waitForScrollToComplete = (
browser,
options: { x: number; y: number }
) =>
check(async () => {
const top = await getTopScroll(browser)
const left = await getLeftScroll(browser)
return top === options.y && left === options.x
? 'success'
: JSON.stringify({ top, left })
}, 'success')
const scrollTo = async (
browser: BrowserInterface,
options: { x: number; y: number }
) => {
await browser.eval(`window.scrollTo(${options.x}, ${options.y})`)
await waitForScrollToComplete(browser, options)
}
describe('vertical scroll', () => {
it('should scroll to top of document when navigating between to pages without layout', async () => {
const browser = await webdriver(next.url, '/0/0/100/10000/page1')
await scrollTo(browser, { x: 0, y: 1000 })
expect(await getTopScroll(browser)).toBe(1000)
await browser.eval(`window.router.push("/0/0/100/10000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 0 })
})
it("should scroll to top of page when scrolling to phe top of the document wouldn't have the page in the viewport", async () => {
const browser = await webdriver(next.url, '/0/1000/100/1000/page1')
await scrollTo(browser, { x: 0, y: 1500 })
expect(await getTopScroll(browser)).toBe(1500)
await browser.eval(`window.router.push("/0/1000/100/1000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 1000 })
})
it("should scroll down to the navigated page when it's below viewort", async () => {
const browser = await webdriver(next.url, '/0/1000/100/1000/page1')
expect(await getTopScroll(browser)).toBe(0)
await browser.eval(`window.router.push("/0/1000/100/1000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 1000 })
})
it('should not scroll when the top of the page is in the viewport', async () => {
const browser = await webdriver(next.url, '/10/1000/100/1000/page1')
await scrollTo(browser, { x: 0, y: 800 })
expect(await getTopScroll(browser)).toBe(800)
await browser.eval(`window.router.push("/10/1000/100/1000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 800 })
})
it('should not scroll to top of document if page in viewport', async () => {
const browser = await webdriver(next.url, '/10/100/100/1000/page1')
await scrollTo(browser, { x: 0, y: 50 })
expect(await getTopScroll(browser)).toBe(50)
await browser.eval(`window.router.push("/10/100/100/1000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 50 })
})
it('should scroll to top of document if possible while giving focus to page', async () => {
const browser = await webdriver(next.url, '/10/100/100/1000/page1')
await scrollTo(browser, { x: 0, y: 200 })
expect(await getTopScroll(browser)).toBe(200)
await browser.eval(`window.router.push("/10/100/100/1000/page2")`)
await waitForScrollToComplete(browser, { x: 0, y: 0 })
})
})
describe('horizontal scroll', () => {
it("should't scroll horizontally", async () => {
const browser = await webdriver(next.url, '/0/0/10000/10000/page1')
await scrollTo(browser, { x: 1000, y: 1000 })
expect(await getLeftScroll(browser)).toBe(1000)
expect(await getTopScroll(browser)).toBe(1000)
await browser.eval(`window.router.push("/0/0/10000/10000/page2")`)
await waitForScrollToComplete(browser, { x: 1000, y: 0 })
})
})
describe('router.refresh()', () => {
it('should not scroll when called alone', async () => {
const browser = await webdriver(next.url, '/10/10000/100/1000/page1')
await scrollTo(browser, { x: 0, y: 12000 })
expect(await getTopScroll(browser)).toBe(12000)
await browser.eval(`window.router.refresh()`)
await waitForScrollToComplete(browser, { x: 0, y: 12000 })
})
it('should not stop router.push() from scrolling', async () => {
const browser = await webdriver(next.url, '/10/10000/100/1000/page1')
await scrollTo(browser, { x: 0, y: 12000 })
expect(await getTopScroll(browser)).toBe(12000)
await browser.eval(`
window.React.startTransition(() => {
window.router.push('/10/10000/100/1000/page2')
window.router.refresh()
})
`)
await waitForScrollToComplete(browser, { x: 0, y: 10000 })
browser.close()
})
// Test hot reloading only in development
chore(test): fix false-positive tests (#63242) <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2798
2024-03-21 01:24:33 +01:00
;(isNextDev ? it : it.skip)(
'should not scroll the page when we hot reload',
async () => {
const browser = await webdriver(next.url, '/10/10000/100/1000/page1')
await scrollTo(browser, { x: 0, y: 12000 })
const pagePath =
'app/[layoutPaddingWidth]/[layoutPaddingHeight]/[pageWidth]/[pageHeight]/[param]/page.tsx'
await browser.eval(`window.router.refresh()`)
chore(test): fix false-positive tests (#63242) <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2798
2024-03-21 01:24:33 +01:00
let originalContent: string
await next.patchFile(pagePath, (content) => {
originalContent = content
return (
content +
`
\\\\ Add this meaningless comment to force refresh
`
chore(test): fix false-positive tests (#63242) <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2798
2024-03-21 01:24:33 +01:00
)
})
await waitForScrollToComplete(browser, { x: 0, y: 12000 })
chore(test): fix false-positive tests (#63242) <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-2798
2024-03-21 01:24:33 +01:00
await next.patchFile(pagePath, originalContent)
}
)
})
Fix scrolling when invisible element is targeted (#48874) ### What? Mux reported they're experiencing a specific case where scroll wasn't applied. I've found a bug when the first element in a layout or page, the one that React will return from `findDOMNode`, is display: hidden. At that point the rect is is `0` `0` `0` `0` and the current logic assumes that means it's in the viewport as it's top `0`. In order to fix this I've looked at a few ways: - Scrolling to top when the element is not visible - Scrolling to the parent element that is visible - **Scrolling to the closest sibling (nextSibling) of the element that is visible** Eventually I landed on the third option after looking at the Mux case, my reproduction, and the way vercel.com's pages leveraging App Router are structured. ### How? Used a while loop to check if the domNode is visible, if not we continue to the next sibling until one that is visible is found. If none are found we bail on resolving further. While looking into this it highlighted that we should add a global scroll handler in app-router too for the case where none of the layout-routers apply scroll. With this fix that is less urgent though so I've added a todo. Fixes NEXT-1056 <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation or adding/fixing Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # -->
2023-04-27 10:00:51 +02:00
describe('bugs', () => {
it('Should scroll to the top of the layout when the first child is display none', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.scrollTo(0, 500)')
await browser
.elementByCss('#to-invisible-first-element')
.click()
.waitForElementByCss('#content-that-is-visible')
await check(() => browser.eval('window.scrollY'), 0)
})
Fix scroll bailout logic when targeting fixed/sticky elements (#53873) ### What? When navigating to a new page with fixed or sticky positioned element as the first element, we were bailing on scroll to top behavior, which often isn't expected. ### Why? Currently, we decide to bail on scroll to top behavior on navigation if the content that is swapped into view is visible within the viewport. Since fixed/sticky positioned elements are often intended to be relative to the current viewport, it's most likely not the case that you'd want it to be considered in this heuristic. For example, if you were scrolled far down on a page, and you navigated to a page that makes use of a sticky header, you would not be scrolled to the top of the page because that sticky header is technically visible within the viewport. ### How? I've updated the previous implementation that was intended to skip targeting invisible elements to also skip over fixed or sticky elements. This should help by falling back to the next level of the layout tree to determine which element to scroll to. I've deleted the `// TODO-APP` comments as I couldn't think of a scenario in which we'd need a global scrollTop handler -- if we've bailed on every element up the tree, it's likely the page wasn't scrollable. Some additional considerations: - Is the warning helpful or annoying? - Is the parallel route trade-off an acceptable one? (ie, a parallel modal slot might not be considered in the content visibility check unless if it’s fixed positioned) Closes NEXT-1393 Fixes #47475
2023-08-15 15:31:39 +02:00
it('Should scroll to the top of the layout when the first child is position fixed', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.scrollTo(0, 500)')
await browser
.elementByCss('#to-fixed-first-element')
.click()
.waitForElementByCss('#content-that-is-visible')
await check(() => browser.eval('window.scrollY'), 0)
if (isNextDev) {
// Check that we've logged a warning
await check(async () => {
const logs = await browser.log()
return logs.some((log) =>
log.message.includes(
'Skipping auto-scroll behavior due to `position: sticky` or `position: fixed` on element:'
)
)
? 'success'
: undefined
}, 'success')
}
})
it('Should scroll to the top of the layout when the first child is position sticky', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.scrollTo(0, 500)')
await browser
.elementByCss('#to-sticky-first-element')
.click()
.waitForElementByCss('#content-that-is-visible')
await check(() => browser.eval('window.scrollY'), 0)
if (isNextDev) {
// Check that we've logged a warning
await check(async () => {
const logs = await browser.log()
return logs.some((log) =>
log.message.includes(
'Skipping auto-scroll behavior due to `position: sticky` or `position: fixed` on element:'
)
)
? 'success'
: undefined
}, 'success')
}
})
it('Should apply scroll when loading.js is used', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.scrollTo(0, 500)')
await browser
.elementByCss('#to-loading-scroll')
.click()
.waitForElementByCss('#loading-component')
await check(() => browser.eval('window.scrollY'), 0)
await browser.waitForElementByCss('#content-that-is-visible')
await check(() => browser.eval('window.scrollY'), 0)
})
Fix scrolling when invisible element is targeted (#48874) ### What? Mux reported they're experiencing a specific case where scroll wasn't applied. I've found a bug when the first element in a layout or page, the one that React will return from `findDOMNode`, is display: hidden. At that point the rect is is `0` `0` `0` `0` and the current logic assumes that means it's in the viewport as it's top `0`. In order to fix this I've looked at a few ways: - Scrolling to top when the element is not visible - Scrolling to the parent element that is visible - **Scrolling to the closest sibling (nextSibling) of the element that is visible** Eventually I landed on the third option after looking at the Mux case, my reproduction, and the way vercel.com's pages leveraging App Router are structured. ### How? Used a while loop to check if the domNode is visible, if not we continue to the next sibling until one that is visible is found. If none are found we bail on resolving further. While looking into this it highlighted that we should add a global scroll handler in app-router too for the case where none of the layout-routers apply scroll. With this fix that is less urgent though so I've added a todo. Fixes NEXT-1056 <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation or adding/fixing Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # -->
2023-04-27 10:00:51 +02:00
})
}
)