test(layout): don't flag scroll-reachable fields as off-viewport
The shared assert_layout_sane degenerate check flagged an interactive element as 'off-viewport' when it sat below the fold at rest but was reachable by scrolling an overflow-y:auto ancestor (tall dialog on a short viewport). This produced a CI-environment-sensitive false positive on the Kanban board settings modal at 480x320 German (the field is inside the .kanban-modal-overlay which is overflow-y:auto), passing locally but failing on the CI runner's font render. Add reachableByScrollableAncestor(): an off-viewport edge absorbed by a scrollable (overflow auto/scroll, content-overflowing) ancestor is reachable and not a defect. Genuinely-unreachable elements (no scroll escape, e.g. inside overflow:hidden) are still flagged. +2 unit tests covering both directions.
This commit is contained in:
1 parent
728ec10e66
commit
903ace3f49
3 files changed
+133
-2
No files matched your search
@@ -3,6 +3,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Layout test helper no longer flags scroll-reachable fields as off-viewport.** The shared `assert_layout_sane` degenerate check reported a false "interactive element is off-viewport" for an input that sits below the fold at rest but is reachable by scrolling an `overflow-y:auto` ancestor (e.g. a tall dialog on a short landscape viewport). It now exempts elements whose off-viewport edge is absorbed by a scrollable ancestor, while still flagging genuinely-unreachable elements (no scroll escape). This removes a CI-environment-sensitive flake on the Kanban board settings modal at 480×320. (internal test infra)
|
||||
|
||||
### Changed
|
||||
|
||||
- **Transparent Stream copy buttons now confirm success in place.** The tool-event and thinking copy controls flash a check for ~1.5s after a successful copy — the same feedback the normal chat copy buttons already give — then revert to the copy glyph. The confirmation is now clearly visible without hover (important on touch), stays correct across live-row reconciliation, cleans up if its row is torn down mid-feedback, and is never serialized into a restored session snapshot. Thanks @Stacey2911. (#6037)
|
||||
|
||||
@@ -137,6 +137,32 @@ function collectRenderViolations(scopeSelector, options) {
|
||||
return null
|
||||
}
|
||||
|
||||
// An interactive element that currently sits outside the viewport is NOT a
|
||||
// "degenerate/off-viewport" defect when it lives inside a scrollable ancestor
|
||||
// (overflow-y/x auto|scroll whose content actually overflows): it is reachable
|
||||
// by scrolling that container. Flagging it produces a false positive for a
|
||||
// valid pattern — e.g. a tall dialog with overflow-y:auto on a short viewport,
|
||||
// where a lower field is momentarily below the fold but scrolls into view.
|
||||
// Only report off-viewport when the element is genuinely unreachable (no
|
||||
// scrollable ancestor can bring the off-axis edge back on-screen).
|
||||
function reachableByScrollableAncestor(node, offLeft, offRight, offTop, offBottom) {
|
||||
var cur = node.parentElement
|
||||
while (cur && cur !== root.parentElement) {
|
||||
var cs = getComputed(cur)
|
||||
var scrollsY = (cs.overflowY === "auto" || cs.overflowY === "scroll") &&
|
||||
cur.scrollHeight > cur.clientHeight + 1
|
||||
var scrollsX = (cs.overflowX === "auto" || cs.overflowX === "scroll") &&
|
||||
cur.scrollWidth > cur.clientWidth + 1
|
||||
// A vertical off-viewport (top past the fold, or bottom above it) is
|
||||
// absorbed by a Y-scrollable ancestor; likewise X off-viewport by an
|
||||
// X-scrollable ancestor.
|
||||
if ((offTop || offBottom) && scrollsY) return true
|
||||
if ((offLeft || offRight) && scrollsX) return true
|
||||
cur = cur.parentElement
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
function walk(node) {
|
||||
if (node.nodeType !== 1) return
|
||||
var cs = getComputed(node)
|
||||
@@ -226,7 +252,15 @@ function collectRenderViolations(scopeSelector, options) {
|
||||
} else if (isVisibleForDegenerate(child)) {
|
||||
var dr = child.getBoundingClientRect()
|
||||
var zeroSize = dr.width === 0 || dr.height === 0
|
||||
var offViewport = dr.right < 0 || dr.bottom < 0 || dr.left > win.innerWidth || dr.top > win.innerHeight
|
||||
var offLeft = dr.right < 0
|
||||
var offRight = dr.left > win.innerWidth
|
||||
var offTop = dr.bottom < 0
|
||||
var offBottom = dr.top > win.innerHeight
|
||||
var offViewport = offLeft || offRight || offTop || offBottom
|
||||
// Reachable-by-scroll inside a scrollable ancestor is not a defect.
|
||||
if (offViewport && reachableByScrollableAncestor(child, offLeft, offRight, offTop, offBottom)) {
|
||||
offViewport = false
|
||||
}
|
||||
if (zeroSize || offViewport) {
|
||||
violations.push({
|
||||
type: "degenerate",
|
||||
@@ -302,7 +336,14 @@ function collectRenderViolations(scopeSelector, options) {
|
||||
if (dNode.tagName === "INPUT" && dNode.getAttribute("type") === "hidden") continue
|
||||
var dr2 = dNode.getBoundingClientRect()
|
||||
var zs = dr2.width === 0 || dr2.height === 0
|
||||
var ov = dr2.right < 0 || dr2.bottom < 0 || dr2.left > win.innerWidth || dr2.top > win.innerHeight
|
||||
var off2Left = dr2.right < 0
|
||||
var off2Right = dr2.left > win.innerWidth
|
||||
var off2Top = dr2.bottom < 0
|
||||
var off2Bottom = dr2.top > win.innerHeight
|
||||
var ov = off2Left || off2Right || off2Top || off2Bottom
|
||||
if (ov && reachableByScrollableAncestor(dNode, off2Left, off2Right, off2Top, off2Bottom)) {
|
||||
ov = false
|
||||
}
|
||||
if (zs || ov) {
|
||||
violations.push({
|
||||
type: "degenerate",
|
||||
|
||||
@@ -137,3 +137,89 @@ def test_raw_i18n_key_detected():
|
||||
assert_no_raw_i18n_keys(page)
|
||||
finally:
|
||||
browser.close()
|
||||
|
||||
|
||||
# A tall dialog with overflow-y:auto on a short viewport: the lower input is
|
||||
# below the fold at rest but reachable by scrolling the dialog. It must NOT be
|
||||
# flagged as an off-viewport degenerate (that is a false positive for a valid
|
||||
# scroll-to-reach pattern). Its sibling case (no scrollable ancestor) MUST still
|
||||
# be flagged.
|
||||
_SCROLLABLE_OFFVIEWPORT_HTML = """\
|
||||
<!doctype html>
|
||||
<html><head><meta charset="utf-8" />
|
||||
<style>
|
||||
body { margin: 0; }
|
||||
.dialog { height: 900px; overflow-y: auto; }
|
||||
.spacer { height: 1000px; }
|
||||
input { display: block; }
|
||||
</style></head>
|
||||
<body>
|
||||
<div class="dialog" id="dlg">
|
||||
<div class="spacer"></div>
|
||||
<input id="lowerField" type="text" />
|
||||
</div>
|
||||
</body></html>"""
|
||||
|
||||
|
||||
_UNREACHABLE_OFFVIEWPORT_HTML = """\
|
||||
<!doctype html>
|
||||
<html><head><meta charset="utf-8" />
|
||||
<style>
|
||||
body { margin: 0; overflow: hidden; }
|
||||
.fixedbox { height: 900px; overflow: hidden; }
|
||||
.spacer { height: 1000px; }
|
||||
input { display: block; }
|
||||
</style></head>
|
||||
<body>
|
||||
<div class="fixedbox" id="box">
|
||||
<div class="spacer"></div>
|
||||
<input id="lostField" type="text" />
|
||||
</div>
|
||||
</body></html>"""
|
||||
|
||||
|
||||
def test_offviewport_interactive_reachable_by_scroll_is_not_flagged():
|
||||
"""An interactive input below the fold but inside an overflow-y:auto ancestor
|
||||
is reachable by scrolling and must NOT be reported off-viewport."""
|
||||
sp = _require_playwright()
|
||||
with sp() as pw:
|
||||
browser = pw.chromium.launch(headless=True, args=_BROWSER_ARGS)
|
||||
try:
|
||||
# 320px tall viewport; the dialog is 900px so the field sits well
|
||||
# below the fold at rest — but the dialog scrolls.
|
||||
page = browser.new_page(viewport={"width": 480, "height": 320})
|
||||
page.set_content(_SCROLLABLE_OFFVIEWPORT_HTML)
|
||||
violations = collect_layout_violations(page, checks=["degenerate"])
|
||||
degenerate = [
|
||||
v for v in violations
|
||||
if v["type"] == "degenerate" and "lowerField" in " ".join(v["elements"])
|
||||
]
|
||||
assert not degenerate, (
|
||||
"a field reachable by scrolling an overflow-y:auto ancestor must not "
|
||||
f"be flagged off-viewport; got {degenerate}"
|
||||
)
|
||||
finally:
|
||||
browser.close()
|
||||
|
||||
|
||||
def test_offviewport_interactive_with_no_scroll_escape_is_still_flagged():
|
||||
"""Positive control: an off-viewport input whose only ancestor is
|
||||
overflow:hidden (no scroll escape) is genuinely unreachable and MUST still
|
||||
be flagged — the scrollable-ancestor exemption must not create false negatives."""
|
||||
sp = _require_playwright()
|
||||
with sp() as pw:
|
||||
browser = pw.chromium.launch(headless=True, args=_BROWSER_ARGS)
|
||||
try:
|
||||
page = browser.new_page(viewport={"width": 480, "height": 320})
|
||||
page.set_content(_UNREACHABLE_OFFVIEWPORT_HTML)
|
||||
violations = collect_layout_violations(page, checks=["degenerate"])
|
||||
degenerate = [
|
||||
v for v in violations
|
||||
if v["type"] == "degenerate" and "lostField" in " ".join(v["elements"])
|
||||
]
|
||||
assert degenerate, (
|
||||
"an off-viewport field with no scrollable ancestor is unreachable "
|
||||
"and must still be flagged off-viewport"
|
||||
)
|
||||
finally:
|
||||
browser.close()
|
||||
Reference in new issue
Block a user