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

Fix scrolling when invisible element is targeted #48874

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,26 @@ function findDOMNode(
return ReactDOM.findDOMNode(instance)
}

const rectProperties = [
'bottom',
'height',
'left',
'right',
'top',
'width',
'x',
'y',
] as const
/**
* Check if a HTMLElement is hidden.
*/
function elementCanScroll(element: HTMLElement) {
// Uses `getBoundingClientRect` to check if the element is hidden instead of `offsetParent`
// because `offsetParent` doesn't consider document/body
const rect = element.getBoundingClientRect()
return rectProperties.every((item) => rect[item] === 0)
}

/**
* Check if the top corner of the HTMLElement is in the viewport.
*/
Expand Down Expand Up @@ -172,11 +192,23 @@ class ScrollAndFocusHandler extends React.Component<ScrollAndFocusHandlerProps>
domNode = findDOMNode(this)
}

// If there is no DOMNode this layout-router level is skipped. It'll be handled higher-up in the tree.
if (!(domNode instanceof HTMLElement)) {
// TODO-APP: Handle the case where we couldn't select any DOM node, even higher up in the layout-router above the current segmentPath.
// If there is no DOM node this layout-router level is skipped. It'll be handled higher-up in the tree.
if (!(domNode instanceof Element)) {
return
}

// Verify if the element is a HTMLElement and if it's visible on screen (e.g. not display: none).
// If the element is not a HTMLElement or not visible we try to select the next sibling and try again.
while (!(domNode instanceof HTMLElement) || elementCanScroll(domNode)) {
// TODO-APP: Handle the case where we couldn't select any DOM node, even higher up in the layout-router above the current segmentPath.
// No siblings found that are visible so we handle scroll higher up in the tree instead.
if (domNode.nextElementSibling === null) {
return
}
domNode = domNode.nextElementSibling
}

// State is mutated to ensure that the focus and scroll is applied only once.
focusAndScrollRef.apply = false

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export default function InvisibleFirstElementPage() {
return (
<>
<div style={{ display: 'none' }}>Content that is hidden.</div>
<div id="content-that-is-visible">Content which is not hidden.</div>
{
// Repeat 500 elements
Array.from({ length: 500 }, (_, i) => (
<div key={i}>{i}</div>
))
}
</>
)
}
17 changes: 17 additions & 0 deletions test/e2e/app-dir/router-autoscroll/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Link from 'next/link'

export default function Page() {
return (
<>
{
// Repeat 500 elements
Array.from({ length: 500 }, (_, i) => (
<div key={i}>{i}</div>
))
}
<Link href="/invisible-first-element" id="to-invisible-first-element">
To invisible first element
</Link>
</>
)
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/router-autoscroll/router-autoscroll.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,5 +159,17 @@ createNextDescribe(
}
)
})

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)
})
})
}
)