Skip to content

Commit

Permalink
report: re-order manual audits and expand when audits pass (#15310)
Browse files Browse the repository at this point in the history
  • Loading branch information
jazyan authored Jul 28, 2023
1 parent 31913c7 commit 113bd76
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 54 deletions.
14 changes: 7 additions & 7 deletions core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ const UIStrings = {
diagnosticsGroupDescription: 'More information about the performance of your application. These numbers don\'t [directly affect](https://developer.chrome.com/docs/lighthouse/performance/performance-scoring/) the Performance score.',
/** Title of the Accessibility category of audits. This section contains audits focused on making web content accessible to all users. Also used as a label of a score gauge; try to limit to 20 characters. */
a11yCategoryTitle: 'Accessibility',
/** Description of the Accessibility category. This is displayed at the top of a list of audits focused on making web content accessible to all users. No character length limits. 'improve the accessibility of your web app' becomes link text to additional documentation. */
a11yCategoryDescription: 'These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.',
/** Description of the Accessibility category. This is displayed at the top of a list of audits focused on making web content accessible to all users. No character length limits. 'improve the accessibility of your web app' and 'manual testing' become link texts to additional documentation. */
a11yCategoryDescription: 'These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Automatic detection can only detect a subset of issues and does not guarantee the accessibility of your web app, so [manual testing](https://web.dev/how-to-review/) is also encouraged.',
/** Description of the Accessibility manual checks category. This description is displayed above a list of accessibility audits that currently have no automated test and so must be verified manually by the user. No character length limits. 'conducting an accessibility review' becomes link text to additional documentation. */
a11yCategoryManualDescription: 'These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://web.dev/how-to-review/).',
/** Title of the best practices section of the Accessibility category. Within this section are audits with descriptive titles that highlight common accessibility best practices. */
Expand Down Expand Up @@ -571,16 +571,16 @@ const defaultConfig = {
{id: 'valid-lang', weight: 7, group: 'a11y-language'},
{id: 'video-caption', weight: 10, group: 'a11y-audio-video'},
// Manual audits
{id: 'logical-tab-order', weight: 0},
{id: 'focusable-controls', weight: 0},
{id: 'interactive-element-affordance', weight: 0},
{id: 'managed-focus', weight: 0},
{id: 'logical-tab-order', weight: 0},
{id: 'visual-order-follows-dom', weight: 0},
{id: 'focus-traps', weight: 0},
{id: 'managed-focus', weight: 0},
{id: 'use-landmarks', weight: 0},
{id: 'offscreen-content-hidden', weight: 0},
{id: 'custom-controls-labels', weight: 0},
{id: 'custom-controls-roles', weight: 0},
{id: 'visual-order-follows-dom', weight: 0},
{id: 'offscreen-content-hidden', weight: 0},
{id: 'use-landmarks', weight: 0},
// Hidden audits
{id: 'empty-heading', weight: 0, group: 'hidden'},
{id: 'identical-links-same-purpose', weight: 0, group: 'hidden'},
Expand Down
60 changes: 30 additions & 30 deletions core/test/fixtures/user-flows/reports/sample-flow-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -4006,7 +4006,7 @@
},
"accessibility": {
"title": "Accessibility",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Automatic detection can only detect a subset of issues and does not guarantee the accessibility of your web app, so [manual testing](https://web.dev/how-to-review/) is also encouraged.",
"manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://web.dev/how-to-review/).",
"supportedModes": [
"navigation",
Expand Down Expand Up @@ -4299,43 +4299,43 @@
"group": "a11y-audio-video"
},
{
"id": "logical-tab-order",
"id": "focusable-controls",
"weight": 0
},
{
"id": "focusable-controls",
"id": "interactive-element-affordance",
"weight": 0
},
{
"id": "interactive-element-affordance",
"id": "logical-tab-order",
"weight": 0
},
{
"id": "managed-focus",
"id": "visual-order-follows-dom",
"weight": 0
},
{
"id": "focus-traps",
"weight": 0
},
{
"id": "custom-controls-labels",
"id": "managed-focus",
"weight": 0
},
{
"id": "custom-controls-roles",
"id": "use-landmarks",
"weight": 0
},
{
"id": "visual-order-follows-dom",
"id": "offscreen-content-hidden",
"weight": 0
},
{
"id": "offscreen-content-hidden",
"id": "custom-controls-labels",
"weight": 0
},
{
"id": "use-landmarks",
"id": "custom-controls-roles",
"weight": 0
},
{
Expand Down Expand Up @@ -14855,7 +14855,7 @@
},
"accessibility": {
"title": "Accessibility",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Automatic detection can only detect a subset of issues and does not guarantee the accessibility of your web app, so [manual testing](https://web.dev/how-to-review/) is also encouraged.",
"manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://web.dev/how-to-review/).",
"supportedModes": [
"navigation",
Expand Down Expand Up @@ -15148,43 +15148,43 @@
"group": "a11y-audio-video"
},
{
"id": "logical-tab-order",
"id": "focusable-controls",
"weight": 0
},
{
"id": "focusable-controls",
"id": "interactive-element-affordance",
"weight": 0
},
{
"id": "interactive-element-affordance",
"id": "logical-tab-order",
"weight": 0
},
{
"id": "managed-focus",
"id": "visual-order-follows-dom",
"weight": 0
},
{
"id": "focus-traps",
"weight": 0
},
{
"id": "custom-controls-labels",
"id": "managed-focus",
"weight": 0
},
{
"id": "custom-controls-roles",
"id": "use-landmarks",
"weight": 0
},
{
"id": "visual-order-follows-dom",
"id": "offscreen-content-hidden",
"weight": 0
},
{
"id": "offscreen-content-hidden",
"id": "custom-controls-labels",
"weight": 0
},
{
"id": "use-landmarks",
"id": "custom-controls-roles",
"weight": 0
},
{
Expand Down Expand Up @@ -21634,7 +21634,7 @@
},
"accessibility": {
"title": "Accessibility",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Automatic detection can only detect a subset of issues and does not guarantee the accessibility of your web app, so [manual testing](https://web.dev/how-to-review/) is also encouraged.",
"manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://web.dev/how-to-review/).",
"supportedModes": [
"navigation",
Expand Down Expand Up @@ -21927,43 +21927,43 @@
"group": "a11y-audio-video"
},
{
"id": "logical-tab-order",
"id": "focusable-controls",
"weight": 0
},
{
"id": "focusable-controls",
"id": "interactive-element-affordance",
"weight": 0
},
{
"id": "interactive-element-affordance",
"id": "logical-tab-order",
"weight": 0
},
{
"id": "managed-focus",
"id": "visual-order-follows-dom",
"weight": 0
},
{
"id": "focus-traps",
"weight": 0
},
{
"id": "custom-controls-labels",
"id": "managed-focus",
"weight": 0
},
{
"id": "custom-controls-roles",
"id": "use-landmarks",
"weight": 0
},
{
"id": "visual-order-follows-dom",
"id": "offscreen-content-hidden",
"weight": 0
},
{
"id": "offscreen-content-hidden",
"id": "custom-controls-labels",
"weight": 0
},
{
"id": "use-landmarks",
"id": "custom-controls-roles",
"weight": 0
},
{
Expand Down
20 changes: 10 additions & 10 deletions core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -6103,7 +6103,7 @@
},
"accessibility": {
"title": "Accessibility",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.",
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developer.chrome.com/docs/lighthouse/accessibility/). Automatic detection can only detect a subset of issues and does not guarantee the accessibility of your web app, so [manual testing](https://web.dev/how-to-review/) is also encouraged.",
"manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://web.dev/how-to-review/).",
"supportedModes": [
"navigation",
Expand Down Expand Up @@ -6396,43 +6396,43 @@
"group": "a11y-audio-video"
},
{
"id": "logical-tab-order",
"id": "focusable-controls",
"weight": 0
},
{
"id": "focusable-controls",
"id": "interactive-element-affordance",
"weight": 0
},
{
"id": "interactive-element-affordance",
"id": "logical-tab-order",
"weight": 0
},
{
"id": "managed-focus",
"id": "visual-order-follows-dom",
"weight": 0
},
{
"id": "focus-traps",
"weight": 0
},
{
"id": "custom-controls-labels",
"id": "managed-focus",
"weight": 0
},
{
"id": "custom-controls-roles",
"id": "use-landmarks",
"weight": 0
},
{
"id": "visual-order-follows-dom",
"id": "offscreen-content-hidden",
"weight": 0
},
{
"id": "offscreen-content-hidden",
"id": "custom-controls-labels",
"weight": 0
},
{
"id": "use-landmarks",
"id": "custom-controls-roles",
"weight": 0
},
{
Expand Down
12 changes: 8 additions & 4 deletions report/renderer/category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,14 @@ export class CategoryRenderer {
* Take a set of audits and render in a top-level, expandable clump that starts
* in a collapsed state.
* @param {Exclude<TopLevelClumpId, 'failed'>} clumpId
* @param {{auditRefs: Array<LH.ReportResult.AuditRef>, description?: string}} clumpOpts
* @param {{auditRefs: Array<LH.ReportResult.AuditRef>, description?: string, openByDefault?: boolean}} clumpOpts
* @return {!Element}
*/
renderClump(clumpId, {auditRefs, description}) {
renderClump(clumpId, {auditRefs, description, openByDefault}) {
const clumpComponent = this.dom.createComponent('clump');
const clumpElement = this.dom.find('.lh-clump', clumpComponent);

if (clumpId === 'warning') {
if (openByDefault) {
clumpElement.setAttribute('open', '');
}

Expand Down Expand Up @@ -559,6 +559,7 @@ export class CategoryRenderer {
});
}

const numFailingAudits = clumps.get('failed')?.length;
// Render each clump.
for (const [clumpId, auditRefs] of clumps) {
if (auditRefs.length === 0) continue;
Expand All @@ -571,7 +572,10 @@ export class CategoryRenderer {
}

const description = clumpId === 'manual' ? category.manualDescription : undefined;
const clumpElem = this.renderClump(clumpId, {auditRefs, description});
// Expand on warning, or manual audits when there are no failing audits.
const openByDefault =
clumpId === 'warning' || (clumpId === 'manual' && numFailingAudits === 0);
const clumpElem = this.renderClump(clumpId, {auditRefs, description, openByDefault});
element.append(clumpElem);
}

Expand Down
2 changes: 1 addition & 1 deletion report/renderer/pwa-category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class PwaCategoryRenderer extends CategoryRenderer {
// Manual audits are still in a manual clump.
const manualAuditRefs = auditRefs.filter(ref => ref.result.scoreDisplayMode === 'manual');
const manualElem = this.renderClump('manual',
{auditRefs: manualAuditRefs, description: category.manualDescription});
{auditRefs: manualAuditRefs, description: category.manualDescription, openByDefault: true});
categoryElem.append(manualElem);

return categoryElem;
Expand Down
12 changes: 12 additions & 0 deletions report/test/renderer/category-renderer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,18 @@ describe('CategoryRenderer', () => {
assert.ok(isExpanded, 'Warning audit group should be expanded by default');
});

it('expands the manual audit group if there are 0 failing audits', () => {
const category = sampleResults.categories.accessibility;
const categoryClone = JSON.parse(JSON.stringify(category));
categoryClone.auditRefs.filter(audit => audit.result.scoreDisplayMode === 'binary')
.forEach(audit => audit.result.score = 1);

const auditDOM = renderer.render(categoryClone, sampleResults.categoryGroups);
const manualClumpEl = auditDOM.querySelector('.lh-clump--manual');
const isExpanded = manualClumpEl.hasAttribute('open');
assert.ok(isExpanded, 'Manual audit group should be expanded if there are 0 failing audits');
});

it('only passing audits with warnings show in warnings section', () => {
const failingWarning = 'Failed and warned';
const passingWarning = 'A passing warning';
Expand Down
2 changes: 1 addition & 1 deletion shared/localization/locales/en-US.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion shared/localization/locales/en-XL.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 113bd76

Please sign in to comment.