Skip to content

Commit

Permalink
core(tap-targets): don't exclude visible position absolute elements f…
Browse files Browse the repository at this point in the history 
…rom audit (#7778)
  • Loading branch information
@mattzeunert @patrickhulce
mattzeunert authored and patrickhulce committed May 25, 2019
1 parent 4aeb0b3 commit 631bbe2
Show file tree
Hide file tree
Showing 4 changed files with 276 additions and 138 deletions.
101 changes: 77 additions & 24 deletions lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,19 @@
<!-- Big tap target, but it's invisible because it's behind the main content div -->
<a style="background: red; position: absolute; top: 0; bottom: 0; left: 0; right: 0; z-index: -1;"></a>

<a style="background: red; position: sticky; top: 0; left: 0; display: block; width: 100vw; height: 40px; z-index: 1;">sticky</a>
<a style="background: red; position: sticky; top: 0; left: 0; display: block; width: 100vw; height: 30px; z-index: 1;">sticky</a>

<div style="background: white; padding: 20px;">

<div style="height: 300px">
<a
data-gathered-target="large-link-at-bottom-of-page"
style="background: #afa; position: absolute; top: 0; display: block; width: calc(100% - 40px); height: 300px;">
This link is intentionally placed at the top of the page, so that the sticky header is hard to tap on.
It should not fail though because overlap with a sticky element depends on the scroll position.
</a>
</div>

<h1>SEO Tap targets</h1>

<!-- Invisible nodes don't cause failures -->
Expand All @@ -47,14 +57,14 @@ <h1>SEO Tap targets</h1>
width 0 and overflow x hidden
</a>
<!-- Visible target should not fail because nothing overlaps it -->
<a>visible target</a>
<a data-gathered-target="visible-target">visible target</a>
</div>
<br/><br/>


<div style="overflow: hidden; position: relative">
<!-- Should be counted as visible although part of it is outside the scroll container -->
<a>
<a data-gathered-target="target-with-client-rect-outside-scroll-container">
<div style="position: absolute;top: -100px">invisible</div>
visible
</a>
Expand All @@ -64,18 +74,18 @@ <h1>SEO Tap targets</h1>

<!-- Link contains large inline block element - no failure because finger
should be placed in the center of the whole link area -->
<a style="background: red;">
<a style="background: red;" data-gathered-target="link-containing-large-inline-block-element">
<div style="display: inline-block; height: 30px; width: 30px; background: orange;"></div>
Link
</a>
<br/>
<a style="display: block; padding: 30px; background: #ddf;">
<a style="display: block; padding: 30px; background: #ddf;" data-gathered-target="link-next-to-link-containing-large-inline-block-element">
Link that the top one would overlap with, if it weren't for the inline-block child.
</a>

<br/><br/>

<div role="button">
<div role="button" data-gathered-target="tap-target-containing-other-tap-targets">
Tap target with children that are also tap targets should not fail.
(Children should not be counted as independent tap targets that appear
in the list.)
Expand All @@ -85,29 +95,78 @@ <h1>SEO Tap targets</h1>

<br/><br/>

<div style="position: relative">
<a style="display: block; position: absolute; top:0; height: 40px; width: 40px;">inner</a>
<a style="display: block; height: 100px; width: 100px; background: yellowgreen">outer</a>
<!-- The left tap target has a large client rect that would normally overlap with the right tap target,
but the left link is overflow hidden, so there should be no failure. We should however
detect both tap targets (and not say the left one is invisible because the center of the big client rect is invisible) -->
<div style="overflow: hidden">
<a
data-gathered-target="child-client-rect-hidden-by-overflow-hidden"
style="display: block; height: 40px; width: 40px; background: #fca; float: left; overflow: hidden; position: relative;">
<div style="background: #afa; height: 40px; width: 80px; position: absolute; left: 30px">
left
</div>
</a>
<a
data-gathered-target="tap-target-next-to-child-client-rect-hidden-by-overflow-hidden"
style="display: block; height: 40px; width: 60px; background: yellowgreen; float: left">right</a>
</div>

<br/><br/>

<!-- Only target that's being overlapped should fail, the overlapping one shouldn't -->
<div>
<a style="display: block; width: 100px; height: 30px;background: #ddd;">
too small target
</a>
<a style="display: block; width: 100px; height: 100px;background: #aaa;">
big enough target
<!--Similar to previous test, except that the position absolute child isn't hidden by "overflow: hidden" on the parent. -->
<div style="overflow: hidden"></div>
<a
data-gathered-target="child-client-rect-overlapping-other-target"
style="display: block; height: 40px; width: 40px; background: #fca; float: left; position: relative;">
<div style="background: #afa; height: 40px; width: 30px; position: absolute; left: 30px">
left
</div>
</a>
<a
data-gathered-target="tap-target-overlapped-by-other-targets-position-absolute-child-rect"
style="display: block; height: 40px; width: 60px; background: yellowgreen; float: left">right</a>
</div>

<br/><br/>

<div style="position: relative">
<a
data-gathered-target="position-absolute-tap-target-fully-contained-in-other-target"
style="display: block; position: absolute; top:0; height: 40px; width: 40px;">inner</a>
<a
data-gathered-target="tap-target-fully-containing-position-absolute-target"
style="display: block; height: 100px; width: 100px; background: yellowgreen">outer</a>
</div>

<br/><br/>

<!-- position: absolute wrapper shouldn't stop not prevent the check (fixed/sticky would) -->
<div style="position: relative; height: 200px;">
<div style="position: absolute;">
<!-- Only target that's being overlapped should fail, the overlapping one shouldn't -->
<a
data-gathered-target="too-small-failing-tap-target"
style="display: block; width: 100px; height: 30px;background: #ddd;">
too small target
</a>
<a
data-gathered-target="large-enough-tap-target-next-to-too-small-tap-target"
style="display: block; width: 100px; height: 100px;background: #aaa;">
big enough target
</a>
</div>
</div>

<br/><br/>

<!-- Should not fail if the two links have the same link target -->
<div>
<a style="display: block; width: 10px; height: 10px;" href="../seo/"></a>
<a style="display: block; width: 10px; height: 10px;" href="./"></a>
<a
data-gathered-target="links-with-same-link-target-1"
style="display: block; width: 10px; height: 10px;" href="../seo/"></a>
<a
data-gathered-target="links-with-same-link-target-2"
style="display: block; width: 10px; height: 10px;" href="./"></a>
</div>

<!-- Links in text blocks are exempted from size/overlap requirements and should not fail -->
Expand All @@ -118,12 +177,6 @@ <h1>SEO Tap targets</h1>
This is <a>a link in a text block.</a>
This is <a>a link in a text block.</a>
</p>

<a style="background: #afa; display: block; width: 100%; height: 800px;">
This link is intentionally placed at the bottom of the page, so that when we've scrolled
to the bottom of the page it overlaps with the sticky header. (Should not fail though
because the overlap depends on the current scroll position.)
</a>
</div>
</body>
</html>
98 changes: 81 additions & 17 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,60 @@ function headersParam(headers) {
return new URLSearchParams([['extra_header', headerString]]).toString();
}

const expectedGatheredTapTargets = [
{
snippet: /large-link-at-bottom-of-page/,
},
{
snippet: /visible-target/,
},
{
snippet: /target-with-client-rect-outside-scroll-container/,
},
{
snippet: /link-containing-large-inline-block-element/,
},
{
snippet: /link-next-to-link-containing-large-inline-block-element/,
},
{
snippet: /tap-target-containing-other-tap-targets/,
},
{
snippet: /child-client-rect-hidden-by-overflow-hidden/,
},
{
snippet: /tap-target-next-to-child-client-rect-hidden-by-overflow-hidden/,
},
{
snippet: /child-client-rect-overlapping-other-target/,
shouldFail: true,
},
{
snippet: /tap-target-overlapped-by-other-targets-position-absolute-child-rect/,
shouldFail: true,
},
{
snippet: /position-absolute-tap-target-fully-contained-in-other-target/,
},
{
snippet: /tap-target-fully-containing-position-absolute-target/,
},
{
snippet: /too-small-failing-tap-target/,
shouldFail: true,
},
{
snippet: /large-enough-tap-target-next-to-too-small-tap-target/,
},
{
snippet: /links-with-same-link-target-1/,
},
{
snippet: /links-with-same-link-target-2/,
},
];

const failureHeaders = headersParam([[
'x-robots-tag',
'none',
Expand Down Expand Up @@ -190,48 +244,58 @@ module.exports = [
audits: {
'tap-targets': {
score: (() => {
const PASSING_TAP_TARGETS = 11;
const TOTAL_TAP_TARGETS = 12;
const totalTapTargets = expectedGatheredTapTargets.length;
const passingTapTargets = expectedGatheredTapTargets.filter(t => !t.shouldFail).length;
const SCORE_FACTOR = 0.89;
return Math.floor(PASSING_TAP_TARGETS / TOTAL_TAP_TARGETS * SCORE_FACTOR * 100) / 100;
return Math.round(passingTapTargets / totalTapTargets * SCORE_FACTOR * 100) / 100;
})(),
details: {
items: [
{
'tapTarget': {
'type': 'node',
'snippet': '<a ' +
'style="display: block; width: 100px; height: 30px;background: #ddd;">' +
'\n too small target\n </a>',
'path': '2,HTML,1,BODY,3,DIV,21,DIV,0,A',
'path': '2,HTML,1,BODY,10,DIV,0,DIV,1,A',
'selector': 'body > div > div > a',
'nodeLabel': 'too small target',
},
'overlappingTarget': {
'type': 'node',
'snippet': '<a ' +
'style="display: block; width: 100px; height: 100px;background: #aaa;">' +
'\n big enough target\n </a>',
'path': '2,HTML,1,BODY,3,DIV,21,DIV,1,A',
'path': '2,HTML,1,BODY,10,DIV,0,DIV,2,A',
'selector': 'body > div > div > a',
'nodeLabel': 'big enough target',
},
'size': '100x30',
'width': 100,
'height': 30,
'tapTargetScore': 1440,
'overlappingTargetScore': 432,
'overlapScoreRatio': 0.3,
'size': '100x30',
'width': 100,
'height': 30,
},
{
'tapTarget': {
'type': 'node',
'path': '2,HTML,1,BODY,3,DIV,24,A',
'selector': 'body > div > a',
},
'overlappingTarget': {
'type': 'node',
'path': '2,HTML,1,BODY,3,DIV,25,A',
'selector': 'body > div > a',
},
'tapTargetScore': 1920,
'overlappingTargetScore': 560,
'overlapScoreRatio': 0.2916666666666667,
'size': '40x40',
'width': 40,
'height': 40,
},
],
},
},
},
},
artifacts: {
TapTargets: {
length: 11,
},
TapTargets: expectedGatheredTapTargets.map(({snippet}) => ({snippet})),
},
},
];
Loading

0 comments on commit 631bbe2

Please sign in to comment.