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 logic to compute nested frame elements #1

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion integration/__snapshots__/body.test.js.snap
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ Array [
Object {
"el": "html",
"left": 0,
"top": 750,
"top": 500,
},
]
`;
Original file line number Diff line number Diff line change
@@ -5,7 +5,12 @@ Array [
Object {
"el": "html",
"left": 0,
"top": 366,
"top": 300,
},
Object {
"el": "html",
"left": 0,
"top": 124,
},
]
`;
6 changes: 4 additions & 2 deletions integration/iframe_outside_viewport.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
<!DOCTYPE html>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<script src="../umd/compute-scroll-into-view.js"></script>
<script src="../umd/compute-scroll-into-view.min.js"></script>
<script src="./utils.js"></script>

<div style="height: 100vh"></div>

<iframe srcdoc="
<iframe style="heihgt: 100px; width: 100px; max-height: 100px; max-width: 100px;" srcdoc="
<style>html, body { margin: 0 }</style>
<div style='height: 300px;'></div>
<div class='target' style='background: crimson; height: 100px; width: 100px;'></div>
">
</iframe>
5 changes: 3 additions & 2 deletions integration/iframe_outside_viewport.test.js
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ beforeAll(async () => {

describe('target in iframe is outside viewport', () => {
test('should scroll top window', async () => {
expect.assertions(3)
expect.assertions(4)
const actual = await page.evaluate(() => {
const iframe = document.querySelector('iframe')
const target = iframe.contentDocument.querySelector('.target')
@@ -14,8 +14,9 @@ describe('target in iframe is outside viewport', () => {
})
.map(window.mapActions)
})
expect(actual).toHaveLength(1)
expect(actual).toHaveLength(2)
expect(actual[0]).toMatchObject({ el: 'html' })
expect(actual[1]).toMatchObject({ el: 'html' }, { el: 'html' })
expect(actual).toMatchSnapshot()
})
})
304 changes: 127 additions & 177 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -34,14 +34,14 @@
"jest": "26.0.1",
"jest-junit": "10.0.0",
"jest-puppeteer": "4.4.0",
"lint-staged": "10.2.6",
"lint-staged": "10.2.10",
"microbundle": "^0.12.0",
"prettier": "2.0.5",
"prettier-package-json": "2.1.3",
"puppeteer": "3.1.0",
"puppeteer": "3.3.0",
"rimraf": "3.0.2",
"serve": "11.3.1",
"typescript": "3.9.3"
"serve": "11.3.2",
"typescript": "3.9.5"
},
"keywords": [
"if-needed",
114 changes: 60 additions & 54 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -49,14 +49,10 @@ interface CustomScrollAction {
}

// @TODO better shadowdom test, 11 = document fragment
function isElement(el: any): el is Element {
function isElement(el: any) {
return el != null && typeof el === 'object' && el.nodeType === 1
}

function isDocument(el: any): el is Document {
return el != null && typeof el === 'object' && el.nodeType === 9
}

function canOverflow(
overflow: string | null,
skipOverflowHiddenElements?: boolean
@@ -245,35 +241,14 @@ function alignNearest(
return 0
}

/**
* Get rect considering iframe
*/
function getRect(target: Element) {
const clientRect = target.getBoundingClientRect()
const rect = {
height: clientRect.height,
width: clientRect.width,
top: clientRect.top,
left: clientRect.left,
bottom: clientRect.bottom,
right: clientRect.right,
}
function computeScrollIntoOwnerDocumentDefaultView(target: Element, options: Options): CustomScrollAction[] {
const doc = target.ownerDocument
let childWindow = doc?.defaultView

while (childWindow && window.top !== childWindow) {
const frameRect = childWindow.frameElement.getBoundingClientRect()
rect.top += frameRect.top
rect.bottom += frameRect.top
rect.left += frameRect.left
rect.right += frameRect.left
childWindow = childWindow.parent
}
const win = doc.defaultView

return rect
}
if (!doc || !win) {
return []
}

export default (target: Element, options: Options): CustomScrollAction[] => {
const {
scrollMode,
block,
@@ -292,19 +267,14 @@ export default (target: Element, options: Options): CustomScrollAction[] => {
}

// Used to handle the top most element that can be scrolled
const scrollingElement = document.scrollingElement || document.documentElement
const scrollingElement = doc.scrollingElement || doc.documentElement

// Collect all the scrolling boxes, as defined in the spec: https://drafts.csswg.org/cssom-view/#scrolling-box
const frames: Element[] = []
let cursor = target
while ((isElement(cursor) || isDocument(cursor)) && checkBoundary(cursor)) {
if (isDocument(cursor)) {
const document = cursor
cursor = document.defaultView?.frameElement as Element
} else {
// Move cursor to parent
cursor = cursor.parentNode as Element
}
while (isElement(cursor) && checkBoundary(cursor)) {
// Move cursor to parent
cursor = cursor.parentNode as Element

// Stop when we reach the viewport
if (cursor === scrollingElement) {
@@ -314,9 +284,9 @@ export default (target: Element, options: Options): CustomScrollAction[] => {

// Skip document.body if it's not the scrollingElement and documentElement isn't independently scrollable
if (
cursor === document.body &&
cursor === doc.body &&
isScrollable(cursor) &&
!isScrollable(document.documentElement as HTMLElement)
!isScrollable(doc.documentElement as HTMLElement)
) {
continue
}
@@ -332,16 +302,16 @@ export default (target: Element, options: Options): CustomScrollAction[] => {
// and viewport dimensions on window.innerWidth/Height
// https://www.quirksmode.org/mobile/viewports2.html
// https://bokand.github.io/viewport/index.html
const viewportWidth = window.visualViewport
? visualViewport.width
: innerWidth
const viewportHeight = window.visualViewport
? visualViewport.height
: innerHeight
const viewportWidth = win.visualViewport
? win.visualViewport.width
: win.innerWidth
const viewportHeight = win.visualViewport
? win.visualViewport.height
: win.innerHeight

// Newer browsers supports scroll[X|Y], page[X|Y]Offset is
const viewportX = window.scrollX || pageXOffset
const viewportY = window.scrollY || pageYOffset
const viewportX = win.scrollX || win.pageXOffset
const viewportY = win.scrollY || win.pageYOffset

const {
height: targetHeight,
@@ -350,7 +320,7 @@ export default (target: Element, options: Options): CustomScrollAction[] => {
right: targetRight,
bottom: targetBottom,
left: targetLeft,
} = getRect(target)
} = target.getBoundingClientRect()

// These values mutate as we loop through and generate scroll coordinates
let targetBlock: number =
@@ -374,7 +344,14 @@ export default (target: Element, options: Options): CustomScrollAction[] => {

// @TODO add a shouldScroll hook here that allows userland code to take control

const { height, width, top, right, bottom, left } = getRect(frame)
const {
height,
width,
top,
right,
bottom,
left,
} = frame.getBoundingClientRect()

// If the element is already visible we can end it here
// @TODO targetBlock and targetInline should be taken into account to be compliant with https://github.com/w3c/csswg-drafts/pull/1805/files#diff-3c17f0e43c20f8ecf89419d49e7ef5e0R1333
@@ -464,8 +441,20 @@ export default (target: Element, options: Options): CustomScrollAction[] => {

// Apply scroll position offsets and ensure they are within bounds
// @TODO add more test cases to cover this 100%
blockScroll = Math.max(0, blockScroll + viewportY)
inlineScroll = Math.max(0, inlineScroll + viewportX)
blockScroll = Math.max(
0,
Math.min(
frame.scrollHeight - frame.clientHeight,
blockScroll + viewportY
)
)
inlineScroll = Math.max(
0,
Math.min(
frame.scrollWidth - frame.clientWidth,
inlineScroll + viewportX
)
)
} else {
// Handle each scrolling frame that might exist between the target and the viewport

@@ -536,3 +525,20 @@ export default (target: Element, options: Options): CustomScrollAction[] => {

return computations
}

export default (target: Element, options: Options): CustomScrollAction[] => {
const targets = []
let cursor: Element | undefined = target

while (cursor) {
targets.push(cursor)
if (cursor?.ownerDocument.defaultView === window) {
break
}
cursor = cursor.ownerDocument.defaultView?.frameElement
}

return targets.reduce((computations: CustomScrollAction[], target) => {
return computations.concat(computeScrollIntoOwnerDocumentDefaultView(target, options))
}, [])
}