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

Remove 'Coming Soon' results from report #1637

Merged
merged 2 commits into from
Feb 7, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 0 additions & 3 deletions lighthouse-cli/printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ function createOutput(results: Results, outputMode: OutputMode): string {
auditResult = subitem as AuditResult;
}

if (auditResult.comingSoon === true)
return;

const formattedScore = auditResult.error ? `${log.redify('‽')}` :
`${formatAggregationResultItem(auditResult.score)}`;
let lineItem = ` ${log.doubleLightHorizontal} ${formattedScore} ${auditResult.description}`;
Expand Down
60 changes: 2 additions & 58 deletions lighthouse-cli/test/fixtures/sample.json
Original file line number Diff line number Diff line change
Expand Up @@ -470,28 +470,7 @@
"first-meaningful-paint",
"speed-index-metric",
"estimated-input-latency",
"time-to-interactive",
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Content scrolls at 60fps",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Touch input gets a response in < 150ms",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "App is interactive without jank after the first meaningful paint",
"comingSoon": true
}
"time-to-interactive"
]
},
{
Expand Down Expand Up @@ -572,42 +551,7 @@
"image-alt",
"label",
"manifest-short-name-length",
"manifest-display",
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Service worker makes use of push notifications, if appropriate",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Tap targets are appropriately sized for touch",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Payment forms marked up with [autocomplete] attributes",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Login forms marked up with [autocomplete] attributes",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Input fields use appropriate [type] attributes for custom keyboards",
"comingSoon": true
}
"manifest-display"
]
}
]
Expand Down
1 change: 0 additions & 1 deletion lighthouse-cli/types/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
interface AuditResult {
displayValue: string;
debugString: string;
comingSoon?: boolean;
score: number;
error?: boolean;
description: string;
Expand Down
14 changes: 0 additions & 14 deletions lighthouse-core/aggregator/aggregate.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,20 +168,6 @@ class Aggregate {
// Step through each item in the expected results, and add them
// to the overall score and add each to the subItems list.
expectedNames.forEach(e => {
/* istanbul ignore if */
// TODO(paullewis): Remove once coming soon audits have landed
if (item.audits[e].comingSoon) {
subItems.push({
score: '¯\\_(ツ)_/¯',
name: 'coming-soon',
category: item.audits[e].category,
description: item.audits[e].description,
comingSoon: true
});

return;
}

if (!filteredAndRemappedResults[e]) {
throw new Error(`aggregations: expected audit results not found under audit name ${e}`);
}
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/closure/typedefs/Aggregation.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ AggregationCriterion.prototype.rawValue;
/** @type {number} */
AggregationCriterion.prototype.weight;

/** @type {boolean|undefined} */
AggregationCriterion.prototype.comingSoon;

/** @type {string|undefined} */
AggregationCriterion.prototype.category;

Expand Down
56 changes: 0 additions & 56 deletions lighthouse-core/config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -138,27 +138,6 @@
"time-to-interactive": {
"expectedValue": 100,
"weight": 1
},
"scrolling-60fps": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Content scrolls at 60fps",
"category": "UX"
},
"touch-150ms": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Touch input gets a response in < 150ms",
"category": "UX"
},
"fmp-no-jank": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "App is interactive without jank after the first meaningful paint",
"category": "UX"
}
}
}, {
Expand Down Expand Up @@ -389,41 +368,6 @@
"manifest-display": {
"expectedValue": true,
"weight": 1
},
"serviceworker-push": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Service worker makes use of push notifications, if appropriate",
"category": "UX"
},
"tap-targets": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Tap targets are appropriately sized for touch",
"category": "UX"
},
"payments-autocomplete": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Payment forms marked up with `autocomplete` attributes",
"category": "UX"
},
"login-autocomplete": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Login forms marked up with `autocomplete` attributes",
"category": "UX"
},
"input-type": {
"expectedValue": true,
"weight": 0,
"comingSoon": true,
"description": "Input fields use appropriate `type` attributes for custom keyboards",
"category": "UX"
}
}
}]
Expand Down
12 changes: 0 additions & 12 deletions lighthouse-core/report/styles/report.css
Original file line number Diff line number Diff line change
Expand Up @@ -620,14 +620,6 @@ body {
color: #999;
}

.coming-soon, .coming-soon * {
color: #aaa;
}

.coming-soon .report-section__item-value {
font-size: 70%;
}

.devtabs {
flex: 0 1 auto;
background: right 0 / auto 27px no-repeat url(tabs_right.png),
Expand Down Expand Up @@ -732,10 +724,6 @@ body {
margin-top: calc(var(--subitem-line-height) / 2);
}

.subitem.--coming-soon {
color: var(--secondary-text-color);
}

.subitem strong {
font-weight: 700;
}
Expand Down
10 changes: 2 additions & 8 deletions lighthouse-core/report/templates/report-template.html
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ <h2>{{ aggregation.name }}</h2>

<ul class="subitems">
{{#each aggregation.subItems as |subItem| }}
<li class="subitem {{#if subItem.comingSoon}}--coming-soon{{/if}} {{#if (shouldShowHelpText subItem.score)}}--show-help{{/if}}">
<li class="subitem {{#if (shouldShowHelpText subItem.score)}}--show-help{{/if}}">

<p class="subitem__desc">
<!--{{#unless ../../scored }}
Expand All @@ -167,10 +167,6 @@ <h2>{{ aggregation.name }}</h2>
<small>(target: {{ subItem.optimalValue }})</small>
{{/if}}

{{#if subItem.comingSoon}}
<small class="subitem__tease">(Coming soon)</small>
{{/if}}

{{#if subItem.helpText }}
<input type="checkbox" class="subitem__help-toggle" title="Toggle help text">
<label class="subitem__help-toggle-label"></label>
Expand All @@ -187,9 +183,7 @@ <h2>{{ aggregation.name }}</h2>
{{/if}}

<div class="subitem-result">
{{#if subItem.comingSoon}}
<span class="subitem-result__unknown score-unknown-bg">N/A</span>
{{else if subItem.error}}
{{#if subItem.error}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe remove .subitem-result__unknown. and .score-unknown-bg now that they're unused? are there other things we're missing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to run the css usage gatherer against report :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need the unknown error background image. a067d30#diff-181e527364f32304fefacb83683da1a7R193.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right :) so just score-unknown-bg?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nuked that rule.

also here's the unused CSS against our report:
image

haha

<span class="subitem-result__unknown score-warning-bg">N/A</span>
{{else}}
{{#if (is-bool subItem.score)}}
Expand Down
60 changes: 2 additions & 58 deletions lighthouse-core/test/results/sample.json
Original file line number Diff line number Diff line change
Expand Up @@ -858,28 +858,7 @@
"first-meaningful-paint",
"speed-index-metric",
"estimated-input-latency",
"time-to-interactive",
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Content scrolls at 60fps",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Touch input gets a response in < 150ms",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "App is interactive without jank after the first meaningful paint",
"comingSoon": true
}
"time-to-interactive"
]
},
{
Expand Down Expand Up @@ -1011,42 +990,7 @@
"name": "Other",
"subItems": [
"manifest-short-name-length",
"manifest-display",
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Service worker makes use of push notifications, if appropriate",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Tap targets are appropriately sized for touch",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Payment forms marked up with [autocomplete] attributes",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Login forms marked up with [autocomplete] attributes",
"comingSoon": true
},
{
"score": "¯\\_(ツ)_/¯",
"name": "coming-soon",
"category": "UX",
"description": "Input fields use appropriate [type] attributes for custom keyboards",
"comingSoon": true
}
"manifest-display"
]
}
]
Expand Down